[GH-ISSUE #3402] [Bug]: MoveEnd leaks source replica refcount when the target disappears #869

New issue

Closed

opened 2026-10-29 21:20:22 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-29 21:20:22 +00:00

Owner

Originally created by @Aionw on GitHub (Aug 12, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3402

Bug Report

While reviewing #2877, I found a pre-existing reference-count leak in the Move error path.

MoveStart() increments the COMPLETE source replica's refcount to protect it from eviction. If the target segment is unmounted before MoveEnd(), the target is removed as an invalid handle. MoveEnd() then enters its missing/invalid-target branch, aborts the reserved quota, and erases the replication task, but it does not decrement the source refcount.

Because the task has already been erased, neither MoveRevoke() nor the task timeout can release that reference later. The surviving source remains permanently busy and cannot be overwritten or evicted.

Reproduction

Mount source_segment and target_segment.
PutStart + PutEnd a key on source_segment.
Call MoveStart(client, key, ..., "source_segment", "target_segment").
Unmount target_segment before calling MoveEnd().
Call MoveEnd(); it returns REPLICA_IS_GONE.
Call same-size UpsertStart() for the surviving key.
Optionally call MoveRevoke().

Expected externally visible results for a regression test:

ASSERT_TRUE(service.MoveStart(client, key, tenant, src, dst).has_value());
ASSERT_TRUE(service.UnmountSegment(dst_id, dst_owner).has_value());

auto move_end = service.MoveEnd(client, key, tenant);
ASSERT_FALSE(move_end.has_value());
EXPECT_EQ(ErrorCode::REPLICA_IS_GONE, move_end.error());

auto upsert = service.UpsertStart(client, key, tenant, object_size, config);
EXPECT_TRUE(upsert.has_value());  // Currently OBJECT_REPLICA_BUSY.

auto revoke = service.MoveRevoke(client, key, tenant);
EXPECT_EQ(ErrorCode::OBJECT_NO_REPLICATION_TASK, revoke.error());

The same condition can be asserted directly in MasterServiceTest: the source refcount is 1 after the failed MoveEnd() even though no replication task remains.

Expected behavior

The invalid-target error path should balance the reference acquired by MoveStart() before erasing the task. The surviving source should remain readable and become eligible for upsert/eviction normally.

Relevant code

In MasterService::MoveEnd(), the invalid-target branch currently calls:

AbortTenantQuota(...);
accessor.EraseReplicationTask();
return tl::make_unexpected(ErrorCode::REPLICA_IS_GONE);

It needs to release the source reference (and keep target cleanup/accounting balanced) before the task becomes unreachable.

This issue predates #2877 and should be fixed independently from the asynchronous-unmount change.

Originally created by @Aionw on GitHub (Aug 12, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/3402 ## Bug Report While reviewing #2877, I found a pre-existing reference-count leak in the Move error path. `MoveStart()` increments the COMPLETE source replica's refcount to protect it from eviction. If the target segment is unmounted before `MoveEnd()`, the target is removed as an invalid handle. `MoveEnd()` then enters its missing/invalid-target branch, aborts the reserved quota, and erases the replication task, but it does not decrement the source refcount. Because the task has already been erased, neither `MoveRevoke()` nor the task timeout can release that reference later. The surviving source remains permanently busy and cannot be overwritten or evicted. ### Reproduction 1. Mount `source_segment` and `target_segment`. 2. `PutStart` + `PutEnd` a key on `source_segment`. 3. Call `MoveStart(client, key, ..., "source_segment", "target_segment")`. 4. Unmount `target_segment` before calling `MoveEnd()`. 5. Call `MoveEnd()`; it returns `REPLICA_IS_GONE`. 6. Call same-size `UpsertStart()` for the surviving key. 7. Optionally call `MoveRevoke()`. Expected externally visible results for a regression test: ```cpp ASSERT_TRUE(service.MoveStart(client, key, tenant, src, dst).has_value()); ASSERT_TRUE(service.UnmountSegment(dst_id, dst_owner).has_value()); auto move_end = service.MoveEnd(client, key, tenant); ASSERT_FALSE(move_end.has_value()); EXPECT_EQ(ErrorCode::REPLICA_IS_GONE, move_end.error()); auto upsert = service.UpsertStart(client, key, tenant, object_size, config); EXPECT_TRUE(upsert.has_value()); // Currently OBJECT_REPLICA_BUSY. auto revoke = service.MoveRevoke(client, key, tenant); EXPECT_EQ(ErrorCode::OBJECT_NO_REPLICATION_TASK, revoke.error()); ``` The same condition can be asserted directly in `MasterServiceTest`: the source refcount is 1 after the failed `MoveEnd()` even though no replication task remains. ### Expected behavior The invalid-target error path should balance the reference acquired by `MoveStart()` before erasing the task. The surviving source should remain readable and become eligible for upsert/eviction normally. ### Relevant code In `MasterService::MoveEnd()`, the invalid-target branch currently calls: ```cpp AbortTenantQuota(...); accessor.EraseReplicationTask(); return tl::make_unexpected(ErrorCode::REPLICA_IS_GONE); ``` It needs to release the source reference (and keep target cleanup/accounting balanced) before the task becomes unreachable. This issue predates #2877 and should be fixed independently from the asynchronous-unmount change.

bsakaguchi

2026-10-29 21:20:22 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-29 21:20:23 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Aug 12, 2026):

Thanks for opening this issue, @Aionw!

Field	Value
Issue	#3402
GitHub user ID	`41376987`
Reporter	@Aionw

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 (Aug 12, 2026): Thanks for opening this issue, @Aionw! | Field | Value | |-------|-------| | **Issue** | #3402 | | **GitHub user ID** | `41376987` | | **Reporter** | @Aionw | 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 referenced this issue

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

[PR #869] [MERGED] Fix nvlink_transport bug: revert #683 #1875

bsakaguchi referenced this issue

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

[PR #870] [MERGED] chore: bump version to 0.3.6.post1 in pyproject.toml #1878