owncloud / owncloud/ocis

userlog/activitylog: cross-instance lost updates on event-list writes (fix direction?)

Open
#12,475 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
2.1k
Forks
274
Avg merge
2d 1h
Merged PRs (30d)
103

Description

## Summary

`userlog` and `activitylog` both keep a per-entity list in the persistent store and update it with a non-atomic **read-list → append → write-list**. With more than one service instance this races and silently loses notifications/activities. PRs #12415 (userlog) and #12417 (activitylog) added a local `sync.Mutex`, but as @kobergj correctly pointed out that only serializes a single process and does nothing across instances. I'd like to agree a direction before reworking, since the real fix lives one layer below these two services.

## Root cause

1. **Events are group-load-balanced.** Both services call `events.Consume(stream, "", …)`, so with N instances a given user's/resource's events are spread across different instances.
2. **The write is a non-atomic read-modify-write**, and the go-micro `store.Store` interface exposes only `Read`/`Write`/`Delete` — `WriteOptions` carries no revision / if-match / CAS. So two instances can each `Read` the list, append, and `Write`, and the later write wins (lost update). A per-process mutex cannot prevent this.

For #12417 specifically, the debounce makes it slightly worse: buffering widens the read-modify-write window, so the cross-instance lost-update probability *increases* versus writing immediately.

## Options

**A. Partition the event stream by entity key** (userid / resourceID) so each entity's events always land on one instance (subject partitioning + per-partition durable consumers). Then a per-process lock is sufficient and reads stay a single fast `Read`.
- *Pro:* architecturally correct for an event-sourced system; benefits every event consumer; no store change.
- *Con:* touches the shared `reva` events layer (publisher + consumer wiring).

**B. Add optimistic concurrency (CAS) to the store.** nats-js-kv's underlying `nats.KeyValue` already supports revisioned `Update(key,val,rev)` + `Get().Revision()`; the go-micro plugin just hides it. Expose a conditional-store interface (memory: in-proc version map; redis: `WATCH`/Lua; noop: passthrough) and do a `read-rev → modify → write-if-rev → retry` loop.
- *Pro:* most reusable — fixes read-modify-write for any oCIS service; keeps single-key reads.
- *Con:* store-layer change (ideally upstreamed to go-micro-plugins, or oCIS opens its own nats-KV handle for these writes).

**C. Append-only, one key per event** (`/`), read with `ReadPrefix()`.
- *Pro:* self-contained in the two services, no upstream change, removes contention by construction.
- *Con:* nats-js-kv's prefix read does `bucket.Keys()` over the whole bucket then filters, so reads scale with total keys, not per-entity — fine for bounded notification lists, risky for high-volume activitylog.

## Recommendation

I lean toward **A** as the correct long-term fix and **B** as the highest-leverage one (it removes this whole class of bug across oCIS). I'd keep the per-process mutex out of the final solution rather than ship a single-binary-only guard.

## Decision needed

@kobergj @DeepDiver1975 — which direction do you prefer? I'm happy to implement A, B, or C (or split: C for `userlog`, A/B for `activitylog`). I'll rebase #12415/#12417 onto whatever we settle on.

Contributor guide

Open the contributing guide

Research direction

Read the event consumers in userlog and activitylog alongside PRs #12415 and #12417; then inspect the shared events and store interfaces described in the issue. Done means the maintainers choose A, B, or C and the selected approach demonstrates that concurrent writes no longer lose events.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, redis
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.