[Store] Support LOCAL_DISK replica migration during drain job#3085
Open
liangxu2000 wants to merge 3 commits into
Open
[Store] Support LOCAL_DISK replica migration during drain job#3085liangxu2000 wants to merge 3 commits into
liangxu2000 wants to merge 3 commits into
Conversation
Enable drain jobs to migrate LOCAL_DISK replicas by reusing the existing Move flow with a new LOCAL_DISK transfer path: - replica.h: get_segment_names() returns transport_endpoint for LOCAL_DISK - client_service: ExecuteReplicaTransfer adds LOCAL_DISK branch (SSD -> staging buffer -> TransferWrite -> target MEMORY) - file_storage: expose LoadBatchFromLocalDisk (AllocateBatch + BatchLoad) for client access - master_service: CreateMoveTask falls back to LOCAL_DISK client lookup; ScheduleDrainJobTasks scans LOCAL_DISK replicas with MEMORY-first dedup; MaybeCompleteDrainJob checks LOCAL_DISK residuals Reference pattern: ProcessPromotionTasks. No new RPCs, no new master state.
…moves, add e2e tests Completes the LOCAL_DISK drain migration (Option A) on top of the core implementation in 60e6d33: - MoveEnd drops redundant LOCAL_DISK metadata for drain moves only, gated by a new is_drain flag threaded through the move task payload (ReplicaMovePayload in task_manager.h), the client (Client::Move / ExecuteTask), the MasterClient::MoveEnd RPC and the rpc_service / master_service handler chain. Manual create_move_task defaults to is_drain=false so it no longer discards a live node's LOCAL_DISK backup. - Scope-narrowing fix for an EDEADLK on segment_mutex_ in MoveEnd (release the unique_lock before taking the shared_lock on the same shared_mutex). - Adds a replica_move_local_disk_transfer_success log in the LOCAL_DISK transfer branch of ExecuteReplicaTransfer, making the path observable and testable (file_storage.h: signature reformatting only). - mountLocalDiskSegment / getOffloadRpcAddr test helpers (real_client) and an offload_on_evict knob in the in-proc master test config. - Adds the local_disk_drain_test e2e suite (5 tests): baseline MEMORY-first dedup, is_drain gating regression (non-aligned sizes + position-dependent pattern), LOCAL_DISK load-failure graceful handling, and eviction-driven offload under both offload_on_evict modes.
liangxu2000
requested review from
XucSh,
YiXR,
stmatengss and
ykwd
as code owners
July 24, 2026 01:50
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
1 task
liangxu2000
force-pushed
the
feature/drainjob-local-disk
branch
2 times, most recently
from
July 24, 2026 07:09
c17e137 to
eec70e6
Compare
liangxu2000
force-pushed
the
feature/drainjob-local-disk
branch
from
July 24, 2026 08:08
eec70e6 to
381183c
Compare
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
When a Mooncake Store node is drained (decommission / scale-in), the drain
scheduler (
ScheduleDrainJobTasks) migrates all replicas to a target node.However,
LOCAL_DISKreplicas (client-side SSD offload files written byFileStorage) were silently skipped becauseReplica::get_segment_names()returned an empty vector for the
LOCAL_DISKtype. Any data that had beenevicted from memory and offloaded to the local SSD was left behind on the
decommissioned node and effectively lost to the cluster.
Related issues / PRs (chronological):
the problem: drain jobs do not migrate
LOCAL_DISKdata.fail-closed safety net that prevents a segment from being marked
DRAINEDwhile
LOCAL_DISKresiduals exist (merged).analyzing implementation options for Step 2 (Option (a): targeted file
transfer vs Option (c): model LOCAL_DISK as NoF-style block resource).
Concluded Option (a) is preferred: low architectural risk, ~400–500
lines, reuses existing
BatchLoad/TransferWriteprimitives.[RFC]: LOCAL_DISK data migration during drain — Options (a) and (c) #2885: actual
LOCAL_DISKmigration during drain, so residuals are movedrather than merely blocking completion.
Solution — Targeted File Transfer (reuse the existing Move flow)
Core insight.
A
LOCAL_DISKreplica is just a file on the source client'slocal SSD. To migrate it, we need to (a) read that file into a memory buffer,
then (b) RDMA-write the buffer into a
MEMORYsegment on the target node.Mooncake already has battle-tested primitives for both steps:
BatchLoad(O_DIRECT file → aligned memory) andTransferWrite(registeredmemory → remote segment). The promotion path (
ProcessPromotionTasks) usesexactly this pattern within a single node; we simply redirect the
TransferWritedestination to a remote target allocated by the drainscheduler.
Why reuse the Move flow instead of a new mechanism?
The existing drain
infrastructure (
ScheduleDrainJobTasks→CreateMoveTask→ client executestransfer →
MoveEndfinalizes metadata) already provides:kMaxDrainUnitRetriesmax_concurrency)MaybeCompleteDrainJob)Building a separate “LOCAL_DISK drain” path would duplicate all of this. By
plugging into the existing Move flow, we get it for free with minimal blast
radius.
Data flow (per key):
Implementation steps:
Visibility.
Replica::get_segment_names()now returns thetransport_endpointforLOCAL_DISKreplicas. Previously it returned anempty vector, making them invisible to the scheduler.
Scheduling with MEMORY-first dedup.
ScheduleDrainJobTasksscans allreplicas on the draining segment. If a key has both a
MEMORYand aLOCAL_DISKreplica on the same client, only theMEMORYreplica isscheduled for migration (it is already in RAM — cheaper to transfer). The
redundant
LOCAL_DISKmetadata is cleaned up inMoveEnd(see step 5).Only keys that are
LOCAL_DISK-only (memory was already evicted) gothrough the file-transfer path.
File transfer branch.
ExecuteReplicaTransfergains aLOCAL_DISKsource branch:
LoadBatchFromLocalDiskcallsAllocateBatch(allocates anO_DIRECT-aligned staging buffer in registered memory) +
BatchLoad(reads the SSD file via
align_down/align_upoffset correction).TransferWriteRDMA-writes the staging buffer into the target's freshlyallocated
MEMORYreplica.replica_move_local_disk_transfer_successlog is emitted on success,making the path observable and testable.
is_drainflag (see Design notes). A boolean is threaded from thescheduler through
ReplicaMovePayload→Client::Move→MoveEndRPC.Conditional metadata cleanup. When a drain move relocates a
MEMORYreplica, the same key's
LOCAL_DISKreplica on the source client is nowredundant (it is a disk backup of the memory data we just moved, and the
target already holds that data).
MoveEnddrops theLOCAL_DISKmastermetadata only when
is_drain=true. The physical SSD file isintentionally left on disk as a safety net (no destructive deletion);
physical cleanup is a separate future step.
Completion guard.
MaybeCompleteDrainJobchecks forLOCAL_DISKresiduals before marking a segment
DRAINED. If anyLOCAL_DISK-onlykeys remain (e.g. because they failed), the segment stays in
DRAININGand the job is marked
FAILED.What is NOT introduced: no new RPC, no new master state machine, no new
task type. The entire feature plugs into the existing Move pipeline.
Testing
5 end-to-end tests drive real eviction-driven offload, manual moves, and
failure injection, verifying the full per-key data route with byte-for-byte
comparison and direct log evidence that the
LOCAL_DISKtransfer path(
LoadBatchFromLocalDisk→ staging buffer →TransferWrite) is exercised.Module
mooncake-transfer-engine)mooncake-store)mooncake-ep)mooncake-pg)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-common)mooncake-rl)Type of Change
Changes
include/replica.hget_segment_names()returns thetransport_endpointforLOCAL_DISKso the drain scheduler can locate them.src/client_service.cppExecuteReplicaTransfer: newLOCAL_DISKsource branch (SSD -> staging buffer viaLoadBatchFromLocalDisk->TransferWrite-> target MEMORY). Adds a..._local_disk_transfer_successlog so the path is observable/testable.src/file_storage.cppLoadBatchFromLocalDisk(AllocateBatch+BatchLoad) exposed for the client.src/master_service.cppCreateMoveTask: client fallback forLOCAL_DISKsources, plus anis_drainflag (defaultfalse; drain sets ittrue).ScheduleDrainJobTasks: scansLOCAL_DISKreplicas with MEMORY-first dedup.MaybeCompleteDrainJob: checksLOCAL_DISKresidue before marking a segmentDRAINED.MoveEnd: only for drain moves (is_drain=true) drops the redundantLOCAL_DISKmetadata after aMEMORYmove, with a scope-narrowing fix for anEDEADLKonsegment_mutex_(unique -> shared on the samestd::shared_mutex).include/task_manager.h,include/client_service.h/src/client_service.cpp,include/master_client.h/src/master_client.cpp,include/rpc_service.h/src/rpc_service.cppis_drainflag from the drain scheduler through the move task payload (ReplicaMovePayload), the client (Client::Move/ExecuteTask), and theMoveEndRPC chain, soMoveEndcan distinguish drain moves from the publiccreate_move_taskAPI. Manual moves default tois_drain=false.include/real_client.h/src/real_client.cppmountLocalDiskSegment()+getOffloadRpcAddr()(used by the e2e test to register aLOCAL_DISKsegment).include/master_config.h/tests/test_server_helpers.hInProcMasterConfig/ builder gainoffload_on_evict, passed through toWrappedMasterServiceConfig(test infra).tests/e2e/CMakeLists.txtlocal_disk_drain_test.tests/e2e/local_disk_drain_test.cppDesign notes
is_drainflag — distinguishing drain moves from manual moves. Thepublic
create_move_taskAPI (used for load-balancing) and the internaldrain scheduler both go through the same
Move→MoveEndcode path.Without a distinguishing flag,
MoveEndwould unconditionally drop thesource client's
LOCAL_DISKmetadata after any MEMORY move — silentlydiscarding a durable SSD backup on a still-live node during a manual move.
To prevent this, a boolean
is_drainis threaded through:ScheduleDrainJobTaskssets ittrueinReplicaMovePayload→task_manager.hcarries it in the task →Client::Move/ExecuteTaskforwards it →
MasterClient::MoveEndpasses it in the RPC →MasterService::MoveEndgates theLOCAL_DISKmetadata removal onis_drain == true. Manual moves default tois_drain=false, preservingthe source's
LOCAL_DISKreplica.MEMORYand a
LOCAL_DISKreplica on the draining client, only theMEMORYreplicais migrated; the
LOCAL_DISKmetadata is dropped inMoveEnd(the SSD fileis intentionally retained on disk as a safety net — physical cleanup is
future work).
get_global_mem_used_ratio()isallocated / capacitysummed across all mounted segments. Mounting alarge target segment therefore dilutes the ratio and can stop eviction. The
test mounts the drain target only after eviction has freed memory (see
test scenarios).
offload_on_evict(easy to confuse withenable_offload). It is gatedby
enable_offload:offload_on_evict_ = enable_offload_ && config.offload_on_evict, so it can only take effect when offload is enabled.It changes when offload happens, not whether:
offload_on_evict=false(default): every PutEnd offloads to SSD, so a keyalready has a
LOCAL_DISKreplica by the time eviction runs.offload_on_evict=true: PutEnd does not offload; a key is queued foroffload only when eviction picks it and finds no
LOCAL_DISKreplica yet,which yields a two-step eviction (queue offload + pin, then drop MEMORY in
the next cycle once the disk backup exists).
How Has This Been Tested?
Test commands:
Test scenarios (5 tests, all pass):
LocalDiskDrainTest.DrainLocalDiskReplicasToPeerPurpose: verify the baseline happy path — drain correctly migrates data
when every key has both MEMORY + LOCAL_DISK replicas, using the MEMORY-first
dedup policy.
Setup: 4 keys x 4 KB on client1 (MEMORY + LOCAL_DISK); drain client1 →
client2. Asserts: drain SUCCEEDED, MEMORY replica migrated (dedup),
LOCAL_DISK metadata dropped, all keys byte-for-byte readable on client2.
LocalDiskDrainTest.ManualMovePreservesLocalDiskBackupPurpose: regression guard for the
is_draingating — prove that amanual move (
is_drain=false) does NOT drop the source's LOCAL_DISKreplicas, and that a subsequent drain correctly uses the LOCAL_DISK transfer
path (including non-aligned O_DIRECT reads).
Setup: manually move each key's MEMORY replica client1 → client2 via the
public
create_move_taskAPI, then drain client1 → client3. Keys use a mixof 4096-aligned and deliberately non-aligned sizes ({4096, 7001, 12000,
5000}) filled with a position-dependent pattern. Asserts: all 4 keys
migrated via LOCAL_DISK path (
local_disk_transfer_successcount),byte-for-byte correct on client3 (guards the O_DIRECT aligned-read offset
fix in
BatchLoad). If the manual move had wrongly dropped LOCAL_DISKmetadata,
succeeded_unitswould be 0.LocalDiskDrainTest.DrainFailsGracefullyWhenLocalDiskDataMissingPurpose: verify graceful failure handling — when the source SSD files
are missing/corrupt, the drain job must reach a terminal FAILED state
(no hang), correctly account retries, and NOT corrupt surviving replicas.
Setup: put 4 keys on client1 (MEMORY + LOCAL_DISK), manually move MEMORY
to client2 (leaving client1 LOCAL_DISK-only), delete client1's SSD files,
then drain client1 → client3. Asserts: job FAILED (not stuck),
succeeded_units == 0,local_disk_load_failederror branch hit,source segment NOT marked DRAINED, client2's MEMORY copy byte-for-byte
intact. Exercises the retry / terminal-failure accounting
(
kMaxDrainUnitRetries = 3).LocalDiskDrainEvictTest.DrainAfterEviction_OffloadOnEvictPurpose: verify drain works end-to-end in a realistic eviction scenario
with
offload_on_evict=true(two-step eviction), where some keys areMEMORY-only and some are LOCAL_DISK-only at drain time.
Setup: 16 keys x 4 MB filling a 64 MB segment; eviction offloads to SSD
and frees MEMORY for ~8 keys. Asserts: drain migrates ~8 keys via MEMORY
path and ~8 keys via LOCAL_DISK path; all 16 keys byte-for-byte readable
on target.
LocalDiskDrainEvictTest.DrainAfterEviction_ImmediateOffloadPurpose: same as above but with
offload_on_evict=false(offload atPutEnd), verifying that both offload modes produce a correct drain outcome.
Setup: identical topology; eviction frees MEMORY for ~8 keys. Asserts:
drain again exercises both MEMORY and LOCAL_DISK paths successfully.
Test results:
local_disk_drain_test, 5/5 PASSED)Log evidence (what you see when running the tests):
The tests emit structured glog output (
GLOG_logtostderr=1) and use aCapturingLogSinkto programmatically assert that specific code paths weretaken. Here is what the key evidence looks like:
Test 2 — LOCAL_DISK transfer path exercised (success):
This proves the full SSD → staging buffer →
TransferWrite→ target MEMORYpath ran for every key, including non-aligned sizes (7001, 12000, 5000 bytes)
that exercise the O_DIRECT
align_down/align_upoffset correction inBatchLoad. The test asserts this count >= 4.Test 3 — graceful failure when SSD files are deleted (4 keys × 3 retries):
Each key fails exactly 3 times (
kMaxDrainUnitRetries = 3), producing 12local_disk_load_failedlines total. The test asserts:status_name == "FAILED"(job reached terminal state, no hang)succeeded_units == 0(nothing was migrated)DRAINEDdid not corrupt or drop the live replica)
Tests 4/5 — eviction-driven drain (final job query):
All 16 keys (8 via MEMORY path + 8 via LOCAL_DISK path) migrated successfully;
migrated_bytes = 16 × 4 MB = 64 MiBconfirms no data was silently dropped.Checklist
./scripts/code_format.shpre-commit run --all-filesand all hooks passAI Assistance Disclosure
AI coding assistants were used to help implement the feature, write the tests,
diagnose test behavior, and review the change. The human submitter has reviewed
the changes and is responsible for defending them end-to-end.