Skip to content

[Store] Prevent drain success with local disk replicas#2834

Open
feichai0017 wants to merge 1 commit into
kvcache-ai:mainfrom
feichai0017:fix/drain-local-disk-replicas
Open

[Store] Prevent drain success with local disk replicas#2834
feichai0017 wants to merge 1 commit into
kvcache-ai:mainfrom
feichai0017:fix/drain-local-disk-replicas

Conversation

@feichai0017

@feichai0017 feichai0017 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #2817.

Prevent drain jobs from reporting success while LOCAL_DISK replicas remain on clients that own source segments:

  • Capture source segment ownership when the drain job is created.
  • Treat unsupported local-disk moves as terminal failures, restore source segments to OK, and return an explicit error.
  • Keep existing memory-replica migration behavior unchanged.
  • Add regression coverage for a local-disk-only object.

This change fails closed; it does not add cross-node local-disk migration.

Module

  • Mooncake Store (mooncake-store)

Type of Change

  • Bug fix

How Has This Been Tested?

Test commands:

./scripts/code_format.sh --check -b upstream/main
SKIP=mooncake-code-format pre-commit run --files mooncake-store/include/master_service.h mooncake-store/src/master_service.cpp mooncake-store/tests/master_service_test.cpp
cmake --build build --target master_service_test
./build/mooncake-store/tests/master_service_test

Test results:

  • Unit tests pass (142/142 on Ubuntu)
  • Integration tests pass (not applicable)
  • Manual testing done

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 touched files and all applicable hooks pass
  • I have updated the documentation (not applicable; no user-facing API change)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue (not applicable; under 500 LOC)

AI Assistance Disclosure

  • AI tools were used: Codex assisted with review, formatting, and test execution.

Copilot AI review requested due to automatic review settings July 10, 2026 08:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces handling for unsupported LOCAL_DISK replicas during drain jobs. Since LOCAL_DISK replicas are client-scoped and cannot currently be migrated, the system now identifies them, associates them with the source segments owned by the client, and fails the drain job. A new unit test has been added to verify this failure behavior. I have no additional feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 93.47826% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mooncake-store/src/master_service.cpp 88.46% 6 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

[Feature Request]: Drain job silently skips LOCAL_DISK objects — potential data loss when source node is decommissioned

3 participants