Skip to content

[Store] Add snapshot-validated batch OpLog pruning - #4142

Merged
ykwd merged 1 commit into
kvcache-ai:mainfrom
Icedcoco:dev/oplog-ha-prs/N12
Sep 17, 2026
Merged

ykwd merged 1 commit into
kvcache-ai:mainfrom
Icedcoco:dev/oplog-ha-prs/N12

Conversation

@Icedcoco

@Icedcoco Icedcoco commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds the N12 batch OpLog pruning coordinator. After a successful snapshot publication, it independently revalidates latest/fallback descriptors and artifacts, atomically advances the reader-visible compaction floor under the maintenance lease, then invokes bounded batch deletion. GC and pruning failures remain best-effort and never invalidate an already published snapshot.

Module

  • Mooncake Store (mooncake-store)
  • Docs

Type of Change

  • New feature
  • Documentation update

How Has This Been Tested?

Test commands:

ctest --test-dir /tmp/mooncake-n12-build --output-on-failure -R '^(batch_oplog_pruning_coordinator_test|batch_oplog_snapshot_gc_test|batch_oplog_snapshot_publisher_test|oplog_batch_storage_test)$'
/tmp/mooncake-n12-build/mooncake-store/tests/batch_oplog_snapshot_coordinator_test
pre-commit run --files <changed files>

Test results:

  • Unit tests pass
  • Integration tests pass (real etcd test passed when explicitly configured)
  • Manual testing done (real etcd floor-before-delete and bounded deletion checks)

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using ./scripts/code_format.sh
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

AI Assistance Disclosure

  • AI tools were used (implementation and test assistance; human review required)

@github-actions github-actions Bot added documentation Improvements or additions to documentation run-ci Store labels Sep 15, 2026

@he-yufeng he-yufeng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mechanism review of the coordinator and its tests (static, line level; no local build, per my review scope):

The contract chain from #4112 is honored end to end. The floor advance precedes deletion, it is computed at the fallback snapshot's batch id (so the latest snapshot's own batches are never in scope), the monotonicity guard (candidate < floor returns OK) keeps concurrent or repeated passes from walking the floor backwards, and the advance itself is a compare-and-swap on the old floor value (or its absence) under the maintenance lease, so two coordinators cannot clobber each other. The corrupt-floor branch (parse must consume the full stored value) fails closed instead of treating garbage as a floor. Only after the CAS does DeleteBatchesThrough run, and its idempotency is what makes the acknowledged crash window (floor advanced, delete failed) recoverable by the next successful publication instead of a leak.

Two integration questions, neither blocking this PR:

  1. The floor is written here, but who reads it? A BuildBatchOpLogSnapshotCompactionFloorKey consumer on the standby restore/rebootstrap path is what makes the floor reader-visible in practice. If that reader lands separately, the rollout note in the docs (promoted standbys must understand the floor protocol before pruning starts) is doing real work and is worth keeping bold.
  2. The fallback-anchored floor means pruning always trails one publication behind. That is the safe choice and reads deliberately conservative; just confirming the first snapshot intentionally never prunes is the documented behavior, since an operator watching disk growth after publication one might otherwise read it as a bug.

Boundary note: no overlap with #3806 (standby index restore tolerance) or #3135 (re-offload idempotency) on my side; the layers are different.

@Icedcoco

Copy link
Copy Markdown
Collaborator Author

Two integration questions, neither blocking this PR:

  1. The floor is written here, but who reads it? A BuildBatchOpLogSnapshotCompactionFloorKey consumer on the standby restore/rebootstrap path is what makes the floor reader-visible in practice. If that reader lands separately, the rollout note in the docs (promoted standbys must understand the floor protocol before pruning starts) is doing real work and is worth keeping bold.
  2. The fallback-anchored floor means pruning always trails one publication behind. That is the safe choice and reads deliberately conservative; just confirming the first snapshot intentionally never prunes is the documented behavior, since an operator watching disk growth after publication one might otherwise read it as a bug.

Thanks for the review. To clarify the two integration points:

  1. The floor reader and rebootstrap path already landed in [Store] Rebootstrap standbys behind the compaction floor #4048. The batch reader returns REBOOTSTRAP_REQUIRED when its cursor falls behind the floor, and the standby restores from a snapshot covering that floor before resuming replay. The rollout requirement remains: every promotable standby must support this protocol before pruning is enabled.

  2. Yes, the first snapshot intentionally does not prune. Pruning requires both latest and fallback, and the cutoff is the fallback’s batch ID. This behavior is covered by the tests and deployment documentation.

@ykwd
ykwd merged commit 2191fcc into kvcache-ai:main Sep 17, 2026
44 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation run-ci Store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants