mirror of
https://github.com/kvcache-ai/Mooncake.git
synced 2026-11-05 01:17:55 +00:00
[GH-ISSUE #2885] [RFC]: LOCAL_DISK data migration during drain — Options (a) and (c) #748
Labels
No labels
ascend
auto-closed
bug
bug
CI/Build
Common
documentation
enhancement
feature request
good first issue
help wanted
high priority
MetaX
nightly-failure
pull-request
PyTorch Backend
question
RFC
Roadmap
stale
Store
Tests
Transfer Engine
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
kvcache-ai/Mooncake#748
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @liangxu2000 on GitHub (Jul 14, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2885
Changes proposed
Background
Issue #2817 identified that
ScheduleDrainJobTaskssilently ignores LOCAL_DISK replicas becauseReplica::get_segment_names()returns an empty vector forLOCAL_DISK. PR #2834 implements a fail-closed safety net (Step 1 in he-yufeng's two-step plan), but the actual migration capability — Step 2 — remains unimplemented.This RFC follows up on #2817 and the fail-closed fix in #2834, proposing a concrete plan for Step 2 — actual LOCAL_DISK migration during drain.
It evaluates two viable approaches:
BatchLoad→TransferWrite→BatchOffloadNoFSegment-style allocator-managed resourceOption (b) (drop LOCAL_DISK and re-offload on target) is excluded because LOCAL_DISK data is not recomputable for some use cases.
Current State: LOCAL_DISK in the Drain Flow
Both the scheduling and execution paths structurally exclude LOCAL_DISK replicas.
Option (a): Targeted File Transfer
Approach
Three building blocks already exist in the codebase:
BatchLoad: Reads data from local SSD files into staging buffers (all four storage backends).TransferWrite(TransferEngine): RDMA/TCP transfer from a local buffer to a remote MEMORY replica.BatchOffload: Writes data from memory buffers to local SSD on the target node.The Promotion path (
ProcessPromotionTasks) already proves the first two steps work together:AllocateBatch() → BatchLoad() → TransferWrite().What Already Exists
storage_backend.cpptransfer_task.cpp:981storage_backend.cppfile_storage.cpp:728What Needs to Be Built
Client::ExecuteReplicaTransfer()— lift theis_memory_replica()restriction (client_service.cpp:3182). Add a LOCAL_DISK path:MasterService::ScheduleDrainJobTasks()— recognize LOCAL_DISK replicas as drainable (currently excluded viaget_segment_names()returning empty). Generate move units for keys that have only LOCAL_DISK replicas on the draining segment.Staging buffer management — reuse the existing
client_buffer_allocator_(same pool used by Promotion). Withlocal_buffer_sizeas hard cap, failures trigger GC retry, then skip. Concurrency is naturally bounded bymax_concurrency(default 4).Source LOCAL_DISK cleanup — after successful migration, delete the source
.bucket/.metaentries. This requires a new cleanup path (MEMORY replicas usediscarded_replicas_with delayed release; LOCAL_DISK has no equivalent).Deduplication — a key may have both MEMORY and LOCAL_DISK replicas on the draining node. When both need to move, transferring both wastes bandwidth (LOCAL_DISK data is a superset of MEMORY data). Optimization: if a MEMORY replica exists for the same key on the same node, use the existing zero-copy TransferWrite path and skip the BatchLoad for LOCAL_DISK.
Considerations
discarded_replicas_(10-min delayed release). LOCAL_DISK cleanup is simpler — just delete the files.max_concurrency=4per drain job + 500ms scheduling interval provides natural throttling.Estimation
client_service.cpp(new LOCAL_DISK branch inExecuteReplicaTransfer, following the Promotion pattern)master_service.cpp(recognize LOCAL_DISK replicas)Option (c): Model LOCAL_DISK as NoFSegment
Approach
Treat LOCAL_DISK as a block-addressable resource managed by a
BufferAllocator, similar to how NoF (NVMe-oF) segments work. This would allow LOCAL_DISK replicas to participate in the existingREPLICA_MOVEprimitive (allocator-to-allocator copy).Why This Requires Architectural Changes
1. Storage model mismatch — files vs blocks
NoF uses block-level LBA addressing via SPDK. LOCAL_DISK uses file-level path addressing. Bridging these requires a file-backed block abstraction layer.
2. Replica data structure incompatibility
NoFReplicaData)LocalDiskReplicaData)unique_ptr<AllocatedBuffer>(offset + endpoint)UUID client_id+transport_endpointTo model LOCAL_DISK as NoF,
LocalDiskReplicaDatamust carry anAllocatedBuffer, which means a new allocator type that manages file offsets rather than memory offsets.3. Requires a new
FileBackedBufferAllocatorExisting allocators (
CachelibBufferAllocator,OffsetBufferAllocator) manage memory regions. LOCAL_DISK data is in files managed byBucketStorageBackendwhere multiple keys share a single bucket file. The allocator must:(file_path, file_offset)pairsThis is effectively building a file-system-backed block allocator — a significant subsystem.
4. Transfer path incompatibility
NoF's
submitSpdkNofOperationissues NVMe commands directly. LOCAL_DISK reads must go through file I/O. To unify them, LOCAL_DISK would need an SPDK backend that emulates NVMe on top of files — unrealistic.5. Drain infrastructure doesn't support NoF segments
ValidateDrainRequestsegment_manager_nof_segment_manager_MaybeCompleteDrainJobsegment_manager_statusExecuteReplicaTransferis_memory_replica()onlyCreateMoveTask/MoveStartEstimation
Comparison
ScheduleDrainJobTasksclient_buffer_allocator_Open Questions
Target replica type: After moving LOCAL_DISK data, should the target store it as MEMORY (simpler, handles drain immediately) or trigger an offload to target's LOCAL_DISK (preserves storage tier semantics)?
Partial drain semantics: If some LOCAL_DISK objects are successfully moved and others fail, should the drain report PARTIAL rather than FAILED? This provides better visibility than the current all-or-nothing approach.
Staging buffer sizing: Should LOCAL_DISK drain use the same
local_buffer_sizepool as Promotion, or have a separate configurable limit?Before submitting a new issue...
@github-actions[bot] commented on GitHub (Jul 14, 2026):
Thanks for opening this issue, @liangxu2000!
258133064A maintainer will triage this when possible. To help us respond faster, please include:
Useful links: Documentation · Contributing guide
@liangxu2000 commented on GitHub (Jul 14, 2026):
@he-yufeng This RFC proposes Step 2 for #2817 — LOCAL_DISK migration during drain. Would appreciate your thoughts when you get a chance.
@liangxu2000 commented on GitHub (Jul 16, 2026):
Option (a) Design Analysis: LOCAL_DISK Drain Migration
After analyzing the codebase, I'd like to share a detailed design for Option (a) — extending the existing Move flow to handle LOCAL_DISK sources. The good news: this approach requires no new infrastructure. All three building blocks (BatchLoad, TransferWrite, client_buffer_allocator_) already exist and are proven by the Promotion path.
Data Flow
The staging buffer is a structural necessity, not an optimization:
read()cannot write directly into remote memory. The staging buffer is the mandatory hop between local disk I/O and RDMA transfer.client_buffer_allocator_(AlignedClientBufferAllocator) does.Key Design Points
1. Staging buffer:
client_buffer_allocator_, not mem segmentThe staging buffer comes from
client_buffer_allocator_(file_storage.cpp:183), a dedicated pool of sizelocal_buffer_size, completely independent from the source client's mem segment.MountSegment.RegisterLocalMemory()for RDMA access.file_storage.cpp:1022).ProcessPromotionTasksandBatchGet.This means: even if the source client's memory is 100% full (which is likely why data was offloaded to SSD in the first place), LOCAL_DISK drain migration can still proceed — the staging pool is separate.
2. Target allocation: MoveStart pre-allocates MEMORY replica
MoveStart (
master_service.cpp:4284) callsallocation_strategy_->AllocateFrom()to pre-allocate the target MEMORY replica before any data transfer. If the target has no space,AllocateFromfails and the move is retried with a different target.This means "target memory full" is caught at MoveStart time, not during TransferWrite. The allocated slot is refcnt-protected for the duration of the move.
3. Deduplication: MEMORY + LOCAL_DISK on same client
After offload completes (
master_service.cpp:5084-5090), the source MEMORY replica's refcnt is decremented but the replica is not immediately removed — eviction happens asynchronously. During this transient window (seconds), the same key can have both MEMORY and LOCAL_DISK replicas on the same client.If drain starts during this window, both replicas would be discovered as separate drain units, causing duplicate migration. Dedup is required, not optional: when generating drain plans, if a key has both MEMORY and LOCAL_DISK on the same source client, only generate one unit (prefer MEMORY — no BatchLoad needed).
4. Priority: MEMORY first, LOCAL_DISK second
Drain plan should process MEMORY units before LOCAL_DISK units. Rationale:
ScheduleDrainJobTasks, not a hard dependency (the staging pool is independent of mem segment capacity).5. Source SSD cleanup
After LOCAL_DISK migration completes, the source's SSD data file is not automatically deleted.
MoveEndruns on the master side and only removes the replica metadata — it does not trigger source-side disk I/O.Options for cleanup:
MoveEnd.For the initial implementation, residual SSD data is harmless (correctness is unaffected, only wasted space). This can be addressed in a follow-up.
Implementation Outline
Change 1:
replica.h— LOCAL_DISK returns segment name (~5 lines)Replica::get_segment_names()currently returns empty for LOCAL_DISK, makingGetReplicaBySegmentNameunable to find it. Add a LOCAL_DISK branch returningtransport_endpointfromLocalDiskReplicaData.Change 2:
master_service.cpp—MoveStartallows LOCAL_DISK source (~10 lines)The
has_invalid_mem_handle()check (master_service.cpp:4256-4257) blocks LOCAL_DISK (which has no mem handle). Skip this check for LOCAL_DISK replicas:Change 3:
client_service.cpp—ExecuteReplicaTransferLOCAL_DISK branch (~70 lines) [core]Add a new branch following the
ProcessPromotionTaskspattern:Change 4:
master_service.cpp—MoveEndhandles LOCAL_DISK (~15 lines)Same
has_invalid_mem_handle()fix as Change 2, applied to theMoveEndsource validation path (master_service.cpp:4370).Change 5:
master_service.cpp—ScheduleDrainJobTasksrecognizes LOCAL_DISK (~20 lines)After PR #2834 (drain job LOCAL_DISK check) merges, remove the LOCAL_DISK block and let LOCAL_DISK keys enter the drain plan normally. Generate drain units with LOCAL_DISK sources, ordered after MEMORY units.
Summary
ProcessPromotionTasks(file_storage.cpp:728)Option (a) is the lowest-risk path: it reuses the Move flow's scheduling/retry/accounting infrastructure and the Promotion path's SSD→RAM→transfer pattern. No new RPCs, no new master state, no new client components.
@zchuango commented on GitHub (Jul 20, 2026):
Thanks for raising this issue! May I ask whether there has been any progress? I’d be glad to see a follow-up PR when it’s ready.
@liangxu2000 commented on GitHub (Jul 23, 2026):
Option (a) implementation is ready — PR #3085
I've implemented Option (a) (Targeted File Transfer) as described in this RFC. The PR is up: #3085.
How it maps to this RFC
is_memory_replica()restriction inExecuteReplicaTransferLOCAL_DISKbranch:LoadBatchFromLocalDisk(AllocateBatch + BatchLoad with O_DIRECT align_down/align_up) →TransferWrite→ target MEMORY replicaScheduleDrainJobTasksrecognize LOCAL_DISKget_segment_names()now returnstransport_endpointfor LOCAL_DISK; scheduler scans them with MEMORY-first dedupclient_buffer_allocator_(same pool as Promotion); naturally bounded bymax_concurrencyMoveEnddrops LOCAL_DISK master metadata whenis_drain=true. Physical SSD file intentionally retained as safety net (no destructive deletion in this PR)Design decisions (addressing the RFC's Open Questions)
Target replica type: MEMORY. The target receives data as a MEMORY replica (simpler, completes the drain immediately). Triggering a subsequent offload to the target's LOCAL_DISK is possible but out of scope for this PR — the drain's goal is to evacuate data from the decommissioning node, and MEMORY achieves that.
Partial drain semantics: The existing
kMaxDrainUnitRetries(3) +terminal_failed_unit_keysmechanism already provides per-key granularity. If some keys succeed and others exhaust retries,MaybeCompleteDrainJobmarks the jobFAILEDwithsucceeded_units > 0, failed_units > 0— effectively a partial result. The segment staysDRAINING(notDRAINED), so the operator can inspect and retry. I did not add a separatePARTIALstatus; the existing fields already convey the information.Staging buffer sizing: Uses the same
local_buffer_sizepool as Promotion. No separate limit introduced —max_concurrency=1(drain default) means at most one staging buffer is in flight per drain job, well within the pool.Additional mechanism not in the RFC:
is_drainflagDuring implementation I found that the RFC's step 4 (source cleanup) has a subtle safety issue: the public
create_move_taskAPI (used for load-balancing) goes through the sameMove → MoveEndpath. Without a distinguishing flag, MoveEnd would unconditionally drop the source's LOCAL_DISK metadata after any MEMORY move — silently discarding a durable SSD backup on a still-live node.To prevent this, I threaded a boolean
is_drainthrough:Manual moves default to
is_drain=false, preserving the source's LOCAL_DISK replica.Test coverage (5 e2e tests, all pass)
DrainLocalDiskReplicasToPeerManualMovePreservesLocalDiskBackupis_draingating: manual move does NOT drop LD; subsequent drain uses LOCAL_DISK transfer path with non-aligned O_DIRECT reads ({4096, 7001, 12000, 5000} bytes)DrainFailsGracefullyWhenLocalDiskDataMissingDrainAfterEviction_OffloadOnEvictoffload_on_evict=true): mixed MEMORY + LOCAL_DISK paths in one drainDrainAfterEviction_ImmediateOffloadoffload_on_evict=falseScope
@github-actions[bot] commented on GitHub (Oct 22, 2026):
This issue has had no activity for 90 days and will be closed in 7 days if there is no further activity. Please comment or react if it should stay open.
@github-actions[bot] commented on GitHub (Oct 29, 2026):
Closing due to 3 months of inactivity. If this is still relevant, please comment and we can reopen.