Skip to content

[Bugfix][Store] Preserve object checksums through HA promotion - #4889

Merged
Aionw merged 2 commits into
kvcache-ai:mainfrom
Icedcoco:fix-ha-001
Oct 26, 2026
Merged

Aionw merged 2 commits into
kvcache-ai:mainfrom
Icedcoco:fix-ha-001

Conversation

@Icedcoco

@Icedcoco Icedcoco commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Description

HA promotion currently preserves an object's replicas but drops its optional object checksum. This change carries ObjectPart::checksum through OpLog payloads, standby metadata and snapshots, then restores it when publishing recovered objects. It covers the existing checksum diagnostic feature; it does not change checksum calculation or repair historical entries that never stored a checksum.

The schema addition uses a compatible field so readers accept older payloads and snapshots. Older records restore with an absent checksum, and older writers still discard the new field, so all writers must be upgraded for checksum continuity.

Related: #3140 added optional object checksum diagnostics and explicitly left HA OpLog continuity out of scope. This PR addresses that HA gap.

Suggested Merge Priority

  • P0 — Urgent
  • P1 — High
  • P2 — Normal
  • P3 — Low

Module

  • Mooncake Store (mooncake-store)

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

/tmp/mooncake-build-fix-ha-001-default/mooncake-store/tests/object_part_test --gtest_color=no
/tmp/mooncake-build-fix-ha-001-default/mooncake-store/tests/session_tag_test --gtest_color=no
/tmp/mooncake-build-fix-ha-001-default/mooncake-store/tests/batch_oplog_snapshot_codec_test --gtest_color=no
/tmp/mooncake-build-fix-ha-001-default/mooncake-store/tests/master_service_ha_test --gtest_color=no

Test results:

  • Unit tests pass: 8/8, 10/10, and 4/4 respectively
  • HA tests pass: 131/131
  • Manual live-cluster failover test

Checklist

  • I have performed a self-review of my own code
  • I have formatted my code using clang-format 20 through the staged-file pre-commit hook
  • I have run pre-commit on the files changed in this PR and all hooks pass
  • I have updated the documentation (not applicable; no user-facing behavior or API changed)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue (not applicable)

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used: assisted with code investigation, implementation, tests, and PR preparation. The human submitter is responsible for reviewing and understanding every changed line.

The durable cleanup tests fill a 1024-byte tenant quota while the default 90% tenant watermark remains enabled. Background eviction can remove the object before the tested cleanup operation, releasing quota early and preventing the expected REMOVE record.

Disable tenant watermark eviction in the three affected tests, matching the existing UpsertStart stale-cleanup test. Keep all durability and quota assertions intact.

Validation: reproduced all three failures by forcing a tenant eviction pass; the same interleaving passes after the fix. All 131 master_service_ha_test cases and three related CTest targets pass. The affected tests also pass 30 repetitions each, and scoped pre-commit passes.
@Icedcoco
Icedcoco marked this pull request as ready for review October 26, 2026 03:19
@Aionw
Aionw merged commit 83f8f87 into kvcache-ai:main Oct 26, 2026
27 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.

2 participants