[MongoDB Storage V3] Force create new replication stream if old one is invalidated - #755
Conversation
🦋 Changeset detectedLatest commit: 1ddd04e The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15bbefa17e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
15bbefa to
3829de4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85ee728925
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This fixes some edge cases.
stevensJourney
left a comment
There was a problem hiding this comment.
The changes look solid and well explained to me. I could not think of any potential issues from this.
When a replication stream is invalidated, for example due to a lost Postgres replication slot, or a MongoDB resume token falling outside the oplog window, we call
restartReplicationto start again from scratch. The implementation works by creating a copy of the current sync config.The issue is that with incremental reprocessing on MongoDB storage v3, this is effectively a no-op. The fix is simple: Force creating a new replication stream, since there is no way we can recover within the existing replication stream. Unfortunately, there is no way to make incremental reprocessing work for us here.
The same applies to the two
/processAPIs., used by the dashboard "Defragment" action. In the future, we can investigate other ways to achieve the defragmenting goals.Additional fixes
The comment here highlighted a class of issues, some existing before this PR: When we have one replication stream with both PROCESSING and ACTIVE sync configs, we must not update both sync configs to ERRORED. ERRORED is specifically used to keep serving the last ACTIVE stream to clients until we have a new one, so we should not have multiple.
This refactors the process both for
restartReplicationand forupdateSyncRulesto correctly handle that case, and additionally use transactions to be safe against concurrent modifications.This changed the PR from a single
forceNewReplicationStreamflag to a much larger refactor.Not many of the existing code issues could be reached directly, due to the original bug here "hiding" those issues. But these became directly apparent once we forced creating a new stream, which would then not correctly stop the existing ones in many cases.
AI Usage
Issue detected by Codex. Fix implemented by Codex gpt-5.6. Checked manually and using Claude Opus 5.