Summary
The collab relay's dedup → sequence → apply → append critical section is guarded only by an in-process room.lock (an asyncio.Lock). Sequence allocation (next_seq) is atomic across workers via Redis INCR, but the seq_for_batch (dedup) → apply_batch → append steps are not atomic across workers.
backend/topix/api/router/collab.py (_handle_message, ~L368):
async with room.lock: # in-memory, per-process — NOT shared across workers
if batch_id:
seen_seq = await oplog.seq_for_batch(board_id, batch_id)
if seen_seq is not None: ...ack + return
seq = await oplog.next_seq(board_id)
await apply_batch(...)
await oplog.append(board_id, seq, batch)
...broadcast
Impact
Under a multi-worker deployment (e.g. multiple uvicorn workers), a client's op frame for batch B handled on worker A while a reconnect/replay of the same batch_id lands on worker B: both see seq_for_batch == None, both allocate distinct seqs, both apply_batch, both append → B stored twice at two seqs, broadcast twice, acked twice.
The code comment explicitly scopes the guarantee to "same-batch races on one worker," so horizontal scaling silently breaks it. (This also affects presence/room state, which lives in per-process memory.)
Suggested direction
Decide and document the deployment model. Options:
- Pin each board/room to a single worker (sticky routing), keeping
room.lock sufficient; or
- Move the dedup+append critical section behind a cross-process lock (e.g. a Redis lock keyed by
board_id) and make room/presence state shared, if multi-worker per room is required.
Context
Found in the second code-review pass of the offline-first PR (#154). Relay is flag-gated (v2); this is an architectural assumption to make explicit before scaling out.
Summary
The collab relay's dedup → sequence → apply → append critical section is guarded only by an in-process
room.lock(anasyncio.Lock). Sequence allocation (next_seq) is atomic across workers via RedisINCR, but theseq_for_batch(dedup) →apply_batch→appendsteps are not atomic across workers.backend/topix/api/router/collab.py(_handle_message, ~L368):Impact
Under a multi-worker deployment (e.g. multiple uvicorn workers), a client's op frame for batch
Bhandled on worker A while a reconnect/replay of the samebatch_idlands on worker B: both seeseq_for_batch == None, both allocate distinct seqs, bothapply_batch, bothappend→Bstored twice at two seqs, broadcast twice, acked twice.The code comment explicitly scopes the guarantee to "same-batch races on one worker," so horizontal scaling silently breaks it. (This also affects presence/room state, which lives in per-process memory.)
Suggested direction
Decide and document the deployment model. Options:
room.locksufficient; orboard_id) and make room/presence state shared, if multi-worker per room is required.Context
Found in the second code-review pass of the offline-first PR (#154). Relay is flag-gated (v2); this is an architectural assumption to make explicit before scaling out.