Skip to content

[Store] Add bounded standby promotion handoff - #3841

Merged
ykwd merged 3 commits into
kvcache-ai:mainfrom
Icedcoco:dev/oplog-ha-prs/N07
Sep 7, 2026
Merged

ykwd merged 3 commits into
kvcache-ai:mainfrom
Icedcoco:dev/oplog-ha-prs/N07

Conversation

@Icedcoco

@Icedcoco Icedcoco commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Description

Depends on #3811.

Implement PR-N07 bounded standby promotion for the batch-OpLog snapshot path:

  • move the complete StandbyMetadataStore into a move-only promotion context instead of exporting a second full object vector;
  • carry the applied batch/sequence cursor, producer view, segments, and maximum live ReplicaID through the handoff;
  • drain and install primary metadata in bounded object chunks while preserving cross-chunk validation and allocator accounting;
  • use batch_oplog_retry_timeout_sec as a no-progress timeout during final catch-up instead of a fixed 30-second total deadline;
  • keep the legacy snapshot vector path unchanged and keep RPC registration behind the complete restore gate.

GitHub cannot use dev/oplog-ha-prs/I00 as the upstream base because that branch exists only in the contributor fork. #3811 is now merged into main; this branch has been rebased onto the resulting main and is N07-only. The N07 commit is d3d418584.

#3806 also edits RestoreFromStandbySnapshot, but for different semantics: it tolerates and discards malformed objects. This PR bounds ownership and conversion memory while retaining the current R01 fail-closed behavior. If #3806 lands first, the shared restore implementation will need a semantic rebase rather than duplicating either path.

Module

  • Mooncake Store (mooncake-store)

Type of Change

  • New feature
  • Performance improvement

How Has This Been Tested?

Test commands:

cmake --build /tmp/mooncake-n07-build --target hot_standby_service_test hot_standby_snapshot_bootstrap_test standby_metadata_store_test batch_oplog_snapshot_provider_test batch_oplog_snapshot_promotion_test master_service_ha_test batch_oplog_promotion_bench -j8
ctest --test-dir /tmp/mooncake-n07-build --output-on-failure -R '(^hot_standby_service_test$|^hot_standby_snapshot_bootstrap_test$|^standby_metadata_store_test$|^batch_oplog_snapshot_provider_test$|^batch_oplog_snapshot_promotion_test$|^master_service_ha_test$)'
/tmp/mooncake-n07-build/mooncake-store/benchmarks/batch_oplog_promotion_bench --objects=10000 --chunk_objects=128

The build used STORE_USE_ETCD=ON so the final catch-up tests were compiled.

Test results:

  • Six related test targets passed, including the complete master_service_ha_test suite
  • Snapshot-only provider fallback and empty batch namespace promotion regressions passed
  • Eight PromotionCatchUpTest cases passed
  • Synthetic 10,000-object promotion completed in 108 ms with 70.9 MB peak RSS in this debug build
  • C++ formatting, whitespace, conflict, large-file, spelling, and CMake formatting hooks passed; unrelated pre-existing CMake reformatting was left out per AGENTS.md

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using the project formatter
  • I have run pre-commit on the files changed in this PR and all hooks pass (see the scoped CMake note above)
  • Documentation update is not applicable; N08 owns configuration and production wiring
  • I have added tests to prove my changes are effective
  • RFC [RFC]: Standby-Generated Snapshots and Bounded OpLog Retention #3167 covers this change

AI Assistance Disclosure

  • AI tools were used (Codex implemented the change, tests, benchmark, and local verification)

The human submitter must review every changed line and be able to defend the change end-to-end before merge.

@Icedcoco
Icedcoco force-pushed the dev/oplog-ha-prs/N07 branch from 9a508ea to d3d4185 Compare September 3, 2026 05:46
@catyans

catyans commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Nice direction—the move-only handoff removes the second full object vector, while restore keeps cross-chunk duplicate, range, and capacity validation, and RPC registration remains behind the completed restore. One non-blocking question: producer_view_version and applied_cursor.batch_id are carried through the handoff and supervisor, but RestoreFromBatchOpLogPromotion() currently consumes only last_seq and max_replica_id. IIUC the new primary already claims its own producer view and the writer reloads the durable prefix from storage, so are these two fields intentionally informational for N08? If so, documenting that contract (or asserting the expected relationship) would keep them from looking like unenforced safety state.

@Icedcoco

Icedcoco commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Yes, this is intentional. producer_view_version is diagnostic only: the new primary claims and uses its own leader view, so it must not reuse the standby’s view. applied_cursor.batch_id is retained for cursor identity and audit; the batch reader already validates batch/sequence continuity, while the new primary writer reloads the authoritative full DurablePrefix from the backend. Therefore the restore path only needs the sequence portion and max_replica_id; no runtime change is required. A short comment can make this contract explicit. @catyans

@Icedcoco
Icedcoco marked this pull request as ready for review September 3, 2026 12:55
@Icedcoco

Icedcoco commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed in 347f1a2c9:

  • Legacy RestoreFromStandbySnapshot() keeps the contiguous vector<pair<...>> + sort range validation path; only bounded batch promotion uses the cross-chunk std::map index.
  • HotStandbyService::Start() recreates a detached StandbyMetadataStore before constructing OpLogApplier, so a failed primary restore can restart standby and bootstrap again instead of passing a null store.
  • Added regressions for snapshot-only fallback, empty batch namespace zero cursor, and restart after batch detach.

Validation: scoped pre-commit passed; promotion tests 7/7, final catch-up tests 10/10, and restore-related HA tests 16/16 passed. The complete master_service_ha_test suite also passed in the same ETCD-enabled build.

@Aionw Aionw 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.

LGTM

@ykwd
ykwd merged commit f30e10a into kvcache-ai:main Sep 7, 2026
39 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants