Repository navigation
GO-7561 concurrent AddLog of the same log returns ErrLogExists instead of WriteConflict - #115
Merged
Merged
Conversation
WatchIds added the id to the stream's watched ids before AddStream and kept it when AddStream failed. The stream was never attached to the log, and a later WatchIds for the id was skipped as already watched, so it stayed dead. The id is now dropped before the error is sent; the error still ends that watch, as clients expect. A node watches a log right after creating it, and its lookup can share the ErrLogNotFound of a load another watch started before the log was committed, so a missing log is looked up once more.
When several nodes create the same log at once, the losing AddLog transactions failed with WriteConflict, which went to the client without an rpc code, instead of ErrLogExists. tx now runs a transaction again on TransientTransactionError, so a loser sees the committed log and gets ErrLogExists. Transactions return mongo errors as they are, so that tx can see their labels; AddLog and AddRecord replace any non-consensus error with ErrUnexpected afterwards, which keeps its rpc code. AddLog and AddRecord no longer clear the payloads of their input, which a retried transaction needs again.
Like AddLog and AddRecord, DeleteLog returned raw mongo errors, which reached the client without an rpc code.
This was referenced Oct 5, 2026
Coverage provided by https://github.com/seriousben/go-patch-cover-action |
consensusErr logged the original mongo error without the log id, while the rpc request log only has the generic ErrUnexpected; the two could only be matched by time.
cheggaaa
approved these changes
Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every sync node that stores a space calls
AddLogand thenWatchwhen it loads the space, usually at the same time as the other replicas. On stage (2026-09-30 to 10-05) nodes loggedfailed to add consensus record: (WriteConflict) … WiredTigerIdIndex::_insert: index: _id_, each followed within 1 ms byreceived consensus error: log not foundfor the same space.AddLogruns in a transaction. When several nodes create the same log at once, the losers fail withWriteConflictwhile the winner is still uncommitted. That error went out joined withErrUnexpected, and drpc cannot read a code fromerrors.Join, so it reached the client with code 0 and the raw mongo text.ErrLogNotFound. The client keeps its watcher registered and only logs the error, so the node got no ACL pushes for that space until it unloaded it or the watch stream reconnected.Fix
txruns a transaction again onTransientTransactionError(up to 10 attempts, 10 ms linear backoff), so a loser waits for the winner's commit and getsErrLogExists; its watch then finds the log. Transactions return mongo errors as they are, sotxcan see their labels;AddLog,AddRecordandDeleteLogthen replace any uncoded error withErrUnexpected(code 500), logging the original.AddLogandAddRecordno longer clear the payloads of their input, which a retried transaction needs again.ErrLogNotFound; ocache drops a failed load, so the second lookup reads the db again.WatchIdswithoutUnwatchIdswas skipped.consensusclientdoes not send that (UnwatchIdsalready removed the id), so this changes nothing for current clients; the error still ends the watch.Remaining gap: if
AddLogstill fails (every retry exhausted), the node's watch ends withErrLogNotFoundandnodeSpace.AddConsensusErroronly logs it, as before. The node keeps its ACL and gets new records through headsync, without the push. A re-watch on the node would close this; not part of this PR.An earlier version kept a watch on a missing log waiting for it. That was dropped: clients treat
ErrLogNotFoundas final, and the coordinator's acl watcher panicked on records after it.Tests
stream:TestStream_WatchMissingLogchecks the event order a client sees (second lookup, error ends the watch, re-watch, other errors); all four cases fail on main.db: concurrentAddLogof the same log (fails on main with the stage WriteConflict),txretry with injected labeled errors (retry, wrapped label, exhaustion, no retry of other errors),AddLogkeeps its input, error conversion keeps code 500.The db tests need MongoDB as a replica set; they were run against a single-node replica set.
Deploy
Requires anyproto/any-sync#805 in the coordinator, any-sync-filenode and any-sync-filenode2 first.
consensusclientrestores its watches after a reconnect outside its mutex; anUnWatch+Watchof the same log in that window, followed by the restore, sends the id twice on one stream. Before, the second request was skipped; now both getErrLogNotFound, which an unpatched acl watcher turns into a double close. The window is narrow, and the same panic is reachable on main without this PR. Order: coordinator and filenodes → consensusnode → nodes (anyproto/any-sync-node#251, optional).Known limits: retries are bounded by attempts, not time (a server-selection failure inside a transaction is also labeled transient); an unknown commit result is handled as before.
Part of GO-7561.