kvcache-ai/Mooncake

Fork 0

mirror of https://github.com/kvcache-ai/Mooncake.git synced 2026-11-05 01:17:55 +00:00

[GH-ISSUE #3826] [Bug]: [NoF] Remount after HA recovery may reallocate offsets referenced by restored replicas #979

New issue

Open

opened 2026-10-29 21:21:35 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-29 21:21:35 +00:00

Owner

Originally created by @cage-goat on GitHub (Sep 2, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3826

Bug Report

Problem Description

Based on the current main branch, Mooncake restores NoF replica descriptors during HA + OpLog recovery, including the endpoint, offset, size, and status.

The restored NoF replicas are associated with a DummyBufferAllocator. This dummy allocator preserves the descriptors and endpoint but does not maintain the actual allocation state.

When the same NoF namespace is subsequently re-registered or remounted, Mooncake creates a new real allocator without importing the restored NoF descriptors into it.

As a result, the new allocator may still consider offsets referenced by restored replicas to be free and allocate them to new objects.

Potential Impact

Mooncake indexes metadata by (tenant_id, user_key) and does not check whether descriptors belonging to different keys reference the same physical range.

Different keys may therefore reference the same (endpoint, offset range). A later write can overwrite the physical data of an older object while its metadata remains COMPLETE and readable. A subsequent Get may then return incorrect data, resulting in silent data corruption.

Questions for the Community

Are NoF replicas expected to remain readable after HA promotion?
When a NoF namespace is remounted, should its real allocator be rebuilt from the restored descriptors?
If NoF HA recovery is not currently supported, should restored replicas or the remounted segment remain unavailable to prevent a fresh allocator from overwriting existing data?

Before submitting...

Ensure you searched for relevant issues and read the [documentation]

Originally created by @cage-goat on GitHub (Sep 2, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3826 ### Bug Report ### Problem Description Based on the current main branch, Mooncake restores NoF replica descriptors during HA + OpLog recovery, including the endpoint, offset, size, and status. The restored NoF replicas are associated with a `DummyBufferAllocator`. This dummy allocator preserves the descriptors and endpoint but does not maintain the actual allocation state. When the same NoF namespace is subsequently re-registered or remounted, Mooncake creates a new real allocator without importing the restored NoF descriptors into it. As a result, the new allocator may still consider offsets referenced by restored replicas to be free and allocate them to new objects. ### Potential Impact Mooncake indexes metadata by `(tenant_id, user_key)` and does not check whether descriptors belonging to different keys reference the same physical range. Different keys may therefore reference the same `(endpoint, offset range)`. A later write can overwrite the physical data of an older object while its metadata remains `COMPLETE` and readable. A subsequent `Get` may then return incorrect data, resulting in silent data corruption. ### Questions for the Community 1. Are NoF replicas expected to remain readable after HA promotion? 2. When a NoF namespace is remounted, should its real allocator be rebuilt from the restored descriptors? 3. If NoF HA recovery is not currently supported, should restored replicas or the remounted segment remain unavailable to prevent a fresh allocator from overwriting existing data? ### Before submitting... - [x] Ensure you searched for relevant issues and read the [documentation]

bsakaguchi added the

bug

label

2026-10-29 21:21:35 +00:00

bsakaguchi commented

2026-10-29 21:21:36 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Sep 2, 2026):

Thanks for opening this issue, @cage-goat!

Field	Value
Issue	#3826
GitHub user ID	`318081269`
Reporter	@cage-goat

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Sep 2, 2026): Thanks for opening this issue, @cage-goat! | Field | Value | |-------|-------| | **Issue** | #3826 | | **GitHub user ID** | `318081269` | | **Reporter** | @cage-goat | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:21:36 +00:00

Author

Owner

@he-yufeng commented on GitHub (Sep 2, 2026):

Verified both halves of this against current main, and the report holds.

The restore side: RestoreFromStandbySnapshot rebuilds NoF replicas on a DummyBufferAllocator (master_service.cpp, the is_nof_replica() branch), which preserves endpoint/offset/size but no allocation state. The segment loop skips non-memory segments entirely (if (!seg.is_memory_segment) continue;), so a restored NoF endpoint never lands in invalid_replica_endpoints_. Since the dummy allocator is kept alive by the standby keepalive map, has_invalid_nof_handle() is false as well. Net: a restored NoF replica is IsReplicaReadable immediately after promotion, before any remount.

The remount side: ReMountNoFSegment -> ScopedNoFSegmentAccess::ReMountSegment -> MountSegment creates a fresh real allocator over the namespace range and imports nothing. There is no NoF counterpart to the memory-side remount flow (allocator replacement, descriptor rewrite, invalid-endpoint erase, lease grant). So offsets referenced by readable restored replicas look free to the new allocator, and the next write can land on them while the old key still serves a COMPLETE, readable replica. That is the silent-corruption sequence you describe.

On your questions, my read:

Readable-after-promotion is the de facto current behavior (nothing gates it), so the safe assumption is yes, they must be handled as live data.
I think the remount has to rebuild allocation state from the restored descriptors before the namespace accepts new writes. The open design bit is the mechanism: the NoF allocator needs a way to mark restored (offset, size) ranges occupied, or the master needs to rebuild that occupancy from the restored replica set of the remounted endpoint.
Until such an import exists, fail-closed is the only honest state: track restored NoF endpoints as invalid at restore time (same invalid_replica_endpoints_ treatment memory segments get) and only clear them on a remount that has imported the restored ranges.

I can take the fix. My current leaning is to do the fail-closed gate first (small, kills the corruption window), then the range import as a second step so restored NoF data actually becomes usable again. If you or the HA folks prefer a different split, say so and I will follow it. One coordination note: the SegmentPool work in #3705/#3707 moves segment lifecycle code around, so whichever lands second will need a rebase pass.

@he-yufeng commented on GitHub (Sep 2, 2026): Verified both halves of this against current main, and the report holds. The restore side: `RestoreFromStandbySnapshot` rebuilds NoF replicas on a `DummyBufferAllocator` (master_service.cpp, the `is_nof_replica()` branch), which preserves endpoint/offset/size but no allocation state. The segment loop skips non-memory segments entirely (`if (!seg.is_memory_segment) continue;`), so a restored NoF endpoint never lands in `invalid_replica_endpoints_`. Since the dummy allocator is kept alive by the standby keepalive map, `has_invalid_nof_handle()` is false as well. Net: a restored NoF replica is `IsReplicaReadable` immediately after promotion, before any remount. The remount side: `ReMountNoFSegment` -> `ScopedNoFSegmentAccess::ReMountSegment` -> `MountSegment` creates a fresh real allocator over the namespace range and imports nothing. There is no NoF counterpart to the memory-side remount flow (allocator replacement, descriptor rewrite, invalid-endpoint erase, lease grant). So offsets referenced by readable restored replicas look free to the new allocator, and the next write can land on them while the old key still serves a COMPLETE, readable replica. That is the silent-corruption sequence you describe. On your questions, my read: 1. Readable-after-promotion is the de facto current behavior (nothing gates it), so the safe assumption is yes, they must be handled as live data. 2. I think the remount has to rebuild allocation state from the restored descriptors before the namespace accepts new writes. The open design bit is the mechanism: the NoF allocator needs a way to mark restored (offset, size) ranges occupied, or the master needs to rebuild that occupancy from the restored replica set of the remounted endpoint. 3. Until such an import exists, fail-closed is the only honest state: track restored NoF endpoints as invalid at restore time (same `invalid_replica_endpoints_` treatment memory segments get) and only clear them on a remount that has imported the restored ranges. I can take the fix. My current leaning is to do the fail-closed gate first (small, kills the corruption window), then the range import as a second step so restored NoF data actually becomes usable again. If you or the HA folks prefer a different split, say so and I will follow it. One coordination note: the SegmentPool work in #3705/#3707 moves segment lifecycle code around, so whichever lands second will need a rebase pass.

bsakaguchi commented

2026-10-29 21:21:36 +00:00

Author

Owner

@cage-goat commented on GitHub (Sep 3, 2026):

@he-yufeng Thanks for the thorough investigation! I support the proposed two-step approach.

@cage-goat commented on GitHub (Sep 3, 2026): @he-yufeng Thanks for the thorough investigation! I support the proposed two-step approach.

bsakaguchi referenced this issue

2026-10-29 21:31:50 +00:00

[PR #1047] [CLOSED] feat(ci): add code coverage support with lcov and Codecov integration Closes #688 #1979

bsakaguchi referenced this issue

2026-10-29 21:33:11 +00:00

[PR #1315] [CLOSED] feat: add Python package version verification (closes #979) #2154

bsakaguchi referenced this issue

2026-10-29 21:33:12 +00:00

[PR #1317] [CLOSED] feat: Add Python package version verification #2157