GO-7373: reconnect the mongo change stream instead of dying silently#103
Open
requilence wants to merge 2 commits into
Open
GO-7373: reconnect the mongo change stream instead of dying silently#103requilence wants to merge 2 commits into
requilence wants to merge 2 commits into
Conversation
streamListener was a bare `for stream.Next(ctx) {}` loop. The driver
resumes a change stream only once per failure, and returns false with a
nil error when the server closes the cursor, so a mongo restart, a
non-resumable error or an invalidate event ended the loop and the
goroutine exited without a trace.
The listener is the only thing that refreshes the log cache, and the
cache never reloads from the db while a stream is attached to it, so a
dead listener froze the cache for good: every new subscriber was served
a stale snapshot and the coordinator kept rejecting valid acl records
with "incorrect prev id of a record".
Log the interruption and reopen the stream with a backoff. The updates
made while the stream was down are not replayed, so notify the reset
receiver on reconnect and resync the cached logs with the db, sending
the missed records to the subscribed streams.
- resync retries a log it failed to fetch: it runs right after mongo came back, so a fetch may well fail, and giving up would leave the log stale forever, since it is pinned in the cache by its streams and never reloaded - ResetReceiver takes a ctx, so a resync can't outlive the service: it runs on the listener goroutine, and Close waits for it before disconnecting - keep the backoff between the reconnects of a stream that dies right after being opened, otherwise the listener reopens it and resyncs in a tight loop - fail loudly if the listener ever stops while the service is running: without it the cache silently serves stale records Do not resume the stream from the last token: the pipeline filters out the invalidate event, so the token never moves past it and the resumed stream dies on it again and again.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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
db.streamListeneris a barefor stream.Next(ctx) {}loop. The driver resumes a change stream only once per failure, and returns false with a nil error when the server closes the cursor:So a mongo restart (resume hits
server selection timeout), a non-resumable error (ChangeStreamHistoryLost,CappedPositionLost) or aninvalidateends the loop and the goroutine exits without a trace. The node stays up and keeps serving.That listener is the only thing that refreshes the log cache, and the cache cannot heal on its own:
object.AddStreamserves the cached snapshot and never re-reads the db, whileobject.TryClosepins the object as long as any stream is attached — and a tree node keeps a permanentWatchon every space it holds.Net effect: a dead listener freezes the cache for good. Every new subscriber gets a stale snapshot, so the coordinator's acl head stays behind the db and it rejects the next correctly chained record forever — surfacing to users as
incorrect prev id of a recordwhen sharing a space.Evidence
Caught on a live consensus node:
LogWatchstreams served — zerostreamListenergoroutines, zeroChangeStreamframes$currentOp: zero change-stream cursors onconsensus.logFix
db.ResetReceiver, fired on reconnect: updates made while the stream was down are never replayed, so the consumer has to resyncstream.Service.resyncreloads every cached log from the db and sends the missed records to the subscribed streams, healing both the cache and the live subscribersRunone, and stop the listener before disconnecting the client inCloseTests
Both fail without the fix:
stream: a new subscriber gets a stale head after a missed updatedb: forces aninvalidate, asserts the listener reconnects and reports the resetFollow-ups (not here)
object.AddStreamcould reconcile with the db instead of serving a pinned snapshotobject.AddRecordsdiffs by slice length, so it cannot detect a gap in the chainany-sync/acl.AddRecordrelies entirely on the consensus broadcast to learn about its own accepted record