[GH-ISSUE #2885] [RFC]: LOCAL_DISK data migration during drain — Options (a) and (c) #748

Closed
opened 2026-10-29 21:19:04 +00:00 by bsakaguchi · 7 comments
Owner

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 ScheduleDrainJobTasks silently ignores LOCAL_DISK replicas because Replica::get_segment_names() returns an empty vector for LOCAL_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:

  • Option (a): Targeted file transfer — BatchLoad → TransferWrite → BatchOffload
  • Option (c): Model LOCAL_DISK as a NoFSegment-style allocator-managed resource

Option (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

Drain Job
  └─> ScheduleDrainJobTasks()
        └─> GetReplicaSegmentNames()
              └─> LOCAL_DISK → empty vector  ← structurally invisible
        └─> CreateMoveTask() — only for MEMORY replicas
  └─> Client::Move()
        └─> ExecuteReplicaTransfer()
              └─> if (!source.is_memory_replica()) → INVALID_PARAMS ← hard rejection

Both the scheduling and execution paths structurally exclude LOCAL_DISK replicas.


Option (a): Targeted File Transfer

Approach

Data:  source SSD  ────→  source RAM  ────→  target RAM  ────→  target SSD
Action:          BatchLoad        TransferWrite      BatchOffload

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

Component Location Status
BatchLoad (SSD → RAM) storage_backend.cpp ✅ 4 backends
TransferWrite (RDMA/TCP) transfer_task.cpp:981 ✅ generic buffer support
BatchOffload (RAM → SSD) storage_backend.cpp ✅ 3 backends
Promotion: BatchLoad → TransferWrite file_storage.cpp:728 ✅ proven pattern

What Needs to Be Built

  1. Client::ExecuteReplicaTransfer() — lift the is_memory_replica() restriction (client_service.cpp:3182). Add a LOCAL_DISK path:

    if (source.is_local_disk_replica()) {
        allocate staging buffer;
        BatchLoad(slices);                 // SSD → staging buffer
        TransferWrite(target, slices);     // staging buffer → target MEMORY replica
        release staging buffer;
    }
    
  2. MasterService::ScheduleDrainJobTasks() — recognize LOCAL_DISK replicas as drainable (currently excluded via get_segment_names() returning empty). Generate move units for keys that have only LOCAL_DISK replicas on the draining segment.

  3. Staging buffer management — reuse the existing client_buffer_allocator_ (same pool used by Promotion). With local_buffer_size as hard cap, failures trigger GC retry, then skip. Concurrency is naturally bounded by max_concurrency (default 4).

  4. Source LOCAL_DISK cleanup — after successful migration, delete the source .bucket/.meta entries. This requires a new cleanup path (MEMORY replicas use discarded_replicas_ with delayed release; LOCAL_DISK has no equivalent).

  5. 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

Concern Analysis
Staging buffer OOM Buffer pool has O(n) hard cap. GC + retry on failure. Same risk profile as Promotion.
Duplicate transfer Can be addressed by preferring MEMORY replica over LOCAL_DISK when both exist on same node.
Source cleanup New path required. MEMORY replicas use discarded_replicas_ (10-min delayed release). LOCAL_DISK cleanup is simpler — just delete the files.
Concurrent drain pressure max_concurrency=4 per drain job + 500ms scheduling interval provides natural throttling.

Estimation

  • Core change: ~200–300 lines in client_service.cpp (new LOCAL_DISK branch in ExecuteReplicaTransfer, following the Promotion pattern)
  • Drain scheduling: ~50–100 lines in master_service.cpp (recognize LOCAL_DISK replicas)
  • Cleanup: ~50–100 lines (delete LOCAL_DISK entries after successful move)
  • Total: moderate effort, low architectural risk

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 existing REPLICA_MOVE primitive (allocator-to-allocator copy).

Why This Requires Architectural Changes

1. Storage model mismatch — files vs blocks

LOCAL_DISK (current):             NoF (what option c requires):
┌─────────────────────┐           ┌─────────────────────┐
│ File system         │           │ NVMe namespace       │
│  bucket_1.bucket    │           │  base=0              │
│  bucket_2.bucket    │           │  offset=4096 (LBA 8) │
│  .meta              │           │  offset=8192         │
│ Addressing: path    │           │ Addressing: LBA      │
│ I/O: open/read/write│           │ I/O: SPDK NVMe-oF    │
└─────────────────────┘           └─────────────────────┘

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

Aspect NoF (NoFReplicaData) LOCAL_DISK (LocalDiskReplicaData)
Core field unique_ptr<AllocatedBuffer> (offset + endpoint) UUID client_id + transport_endpoint
Allocator CachelibBufferAllocator / OffsetBufferAllocator None (client manages files)
get_segment_names() Returns segment name Returns empty vector

To model LOCAL_DISK as NoF, LocalDiskReplicaData must carry an AllocatedBuffer, which means a new allocator type that manages file offsets rather than memory offsets.

3. Requires a new FileBackedBufferAllocator

Existing allocators (CachelibBufferAllocator, OffsetBufferAllocator) manage memory regions. LOCAL_DISK data is in files managed by BucketStorageBackend where multiple keys share a single bucket file. The allocator must:

  • Track available space across bucket files
  • Handle bucket fragmentation (keys of different sizes packed together)
  • Coordinate with the bucket's LRU eviction policy
  • Present a flat offset space that maps to (file_path, file_offset) pairs

This is effectively building a file-system-backed block allocator — a significant subsystem.

4. Transfer path incompatibility

NoF transfer path:                LOCAL_DISK transfer path:
SPDK → NVMe-oF → remote NVMe      open/read → user buffer → TransferEngine → remote

NoF's submitSpdkNofOperation issues 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

Drain step NoF support Needs
ValidateDrainRequest ❌ Only queries segment_manager_ Add nof_segment_manager_
MaybeCompleteDrainJob ❌ Only sets segment_manager_ status Add NoF segment status handling
ExecuteReplicaTransfer ❌ Hardcoded is_memory_replica() only Add NoF source support
CreateMoveTask / MoveStart ❌ Only allocates MEMORY replicas Support NoF→NoF or NoF→MEMORY

Estimation

  • New allocator: major new subsystem (thousands of lines)
  • SPDK/file bridge: large effort, architecturally questionable
  • Drain full-chain NoF support: hundreds of lines across validation, scheduling, and execution
  • Total: high effort, high architectural risk, uncertain benefit

Comparison

Option (a): File Transfer Option (c): Model as NoF
Reuses existing code ✅ BatchLoad, TransferWrite, BatchOffload all exist ❌ Requires new allocator, new transfer path
New code required ~400–500 lines Several thousand lines
Architectural risk Low — pipelining existing primitives High — abstracting files as blocks
Drain scheduling change Recognize LOCAL_DISK in ScheduleDrainJobTasks Full NoF drain support chain
Staging buffer Reuse existing client_buffer_allocator_ N/A (block-level access)
Transfer protocol File I/O + RDMA/TCP (existing) Must bridge file I/O to SPDK NVMe-oF
Source cleanup New delete path (simple) Via allocator free (needs to be built)
Deduplication Solvable — prefer MEMORY over LOCAL_DISK Same
Fits architecture ✅ Composes existing layers ❌ Forces square peg into round hole

Open Questions

  1. 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)?

  2. 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.

  3. Staging buffer sizing: Should LOCAL_DISK drain use the same local_buffer_size pool as Promotion, or have a separate configurable limit?

Before submitting a new issue...

  • Make sure you already searched for relevant issues and read the documentation
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](https://github.com/kvcache-ai/Mooncake/issues/2817) identified that `ScheduleDrainJobTasks` silently ignores LOCAL_DISK replicas because `Replica::get_segment_names()` returns an empty vector for `LOCAL_DISK`. PR [#2834](https://github.com/kvcache-ai/Mooncake/pull/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](https://github.com/kvcache-ai/Mooncake/issues/2817) and the fail-closed fix in [#2834](https://github.com/kvcache-ai/Mooncake/pull/2834), proposing a concrete plan for Step 2 — actual LOCAL_DISK migration during drain. It evaluates two viable approaches: - **Option (a)**: Targeted file transfer — `BatchLoad` → `TransferWrite` → `BatchOffload` - **Option (c)**: Model LOCAL_DISK as a `NoFSegment`-style allocator-managed resource Option (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 ``` Drain Job └─> ScheduleDrainJobTasks() └─> GetReplicaSegmentNames() └─> LOCAL_DISK → empty vector ← structurally invisible └─> CreateMoveTask() — only for MEMORY replicas └─> Client::Move() └─> ExecuteReplicaTransfer() └─> if (!source.is_memory_replica()) → INVALID_PARAMS ← hard rejection ``` Both the scheduling and execution paths structurally exclude LOCAL_DISK replicas. --- ## Option (a): Targeted File Transfer ### Approach ``` Data: source SSD ────→ source RAM ────→ target RAM ────→ target SSD Action: BatchLoad TransferWrite BatchOffload ``` 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 | Component | Location | Status | | --- | --- | --- | | BatchLoad (SSD → RAM) | `storage_backend.cpp` | ✅ 4 backends | | TransferWrite (RDMA/TCP) | `transfer_task.cpp:981` | ✅ generic buffer support | | BatchOffload (RAM → SSD) | `storage_backend.cpp` | ✅ 3 backends | | Promotion: BatchLoad → TransferWrite | `file_storage.cpp:728` | ✅ proven pattern | ### What Needs to Be Built 1. **`Client::ExecuteReplicaTransfer()`** — lift the `is_memory_replica()` restriction (client_service.cpp:3182). Add a LOCAL_DISK path: ``` if (source.is_local_disk_replica()) { allocate staging buffer; BatchLoad(slices); // SSD → staging buffer TransferWrite(target, slices); // staging buffer → target MEMORY replica release staging buffer; } ``` 2. **`MasterService::ScheduleDrainJobTasks()`** — recognize LOCAL_DISK replicas as drainable (currently excluded via `get_segment_names()` returning empty). Generate move units for keys that have only LOCAL_DISK replicas on the draining segment. 3. **Staging buffer management** — reuse the existing `client_buffer_allocator_` (same pool used by Promotion). With `local_buffer_size` as hard cap, failures trigger GC retry, then skip. Concurrency is naturally bounded by `max_concurrency` (default 4). 4. **Source LOCAL_DISK cleanup** — after successful migration, delete the source `.bucket`/`.meta` entries. This requires a new cleanup path (MEMORY replicas use `discarded_replicas_` with delayed release; LOCAL_DISK has no equivalent). 5. **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 | Concern | Analysis | | --- | --- | | **Staging buffer OOM** | Buffer pool has O(n) hard cap. GC + retry on failure. Same risk profile as Promotion. | | **Duplicate transfer** | Can be addressed by preferring MEMORY replica over LOCAL_DISK when both exist on same node. | | **Source cleanup** | New path required. MEMORY replicas use `discarded_replicas_` (10-min delayed release). LOCAL_DISK cleanup is simpler — just delete the files. | | **Concurrent drain pressure** | `max_concurrency=4` per drain job + 500ms scheduling interval provides natural throttling. | ### Estimation - **Core change**: ~200–300 lines in `client_service.cpp` (new LOCAL_DISK branch in `ExecuteReplicaTransfer`, following the Promotion pattern) - **Drain scheduling**: ~50–100 lines in `master_service.cpp` (recognize LOCAL_DISK replicas) - **Cleanup**: ~50–100 lines (delete LOCAL_DISK entries after successful move) - **Total**: moderate effort, low architectural risk --- ## 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 existing `REPLICA_MOVE` primitive (allocator-to-allocator copy). ### Why This Requires Architectural Changes **1. Storage model mismatch — files vs blocks** ``` LOCAL_DISK (current): NoF (what option c requires): ┌─────────────────────┐ ┌─────────────────────┐ │ File system │ │ NVMe namespace │ │ bucket_1.bucket │ │ base=0 │ │ bucket_2.bucket │ │ offset=4096 (LBA 8) │ │ .meta │ │ offset=8192 │ │ Addressing: path │ │ Addressing: LBA │ │ I/O: open/read/write│ │ I/O: SPDK NVMe-oF │ └─────────────────────┘ └─────────────────────┘ ``` 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** | Aspect | NoF (`NoFReplicaData`) | LOCAL_DISK (`LocalDiskReplicaData`) | | --- | --- | --- | | Core field | `unique_ptr<AllocatedBuffer>` (offset + endpoint) | `UUID client_id` + `transport_endpoint` | | Allocator | CachelibBufferAllocator / OffsetBufferAllocator | None (client manages files) | | get_segment_names() | Returns segment name | Returns empty vector | To model LOCAL_DISK as NoF, `LocalDiskReplicaData` must carry an `AllocatedBuffer`, which means a new allocator type that manages file offsets rather than memory offsets. **3. Requires a new `FileBackedBufferAllocator`** Existing allocators (`CachelibBufferAllocator`, `OffsetBufferAllocator`) manage memory regions. LOCAL_DISK data is in files managed by `BucketStorageBackend` where multiple keys share a single bucket file. The allocator must: - Track available space across bucket files - Handle bucket fragmentation (keys of different sizes packed together) - Coordinate with the bucket's LRU eviction policy - Present a flat offset space that maps to `(file_path, file_offset)` pairs This is effectively building a **file-system-backed block allocator** — a significant subsystem. **4. Transfer path incompatibility** ``` NoF transfer path: LOCAL_DISK transfer path: SPDK → NVMe-oF → remote NVMe open/read → user buffer → TransferEngine → remote ``` NoF's `submitSpdkNofOperation` issues 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** | Drain step | NoF support | Needs | | --- | --- | --- | | `ValidateDrainRequest` | ❌ Only queries `segment_manager_` | Add `nof_segment_manager_` | | `MaybeCompleteDrainJob` | ❌ Only sets `segment_manager_` status | Add NoF segment status handling | | `ExecuteReplicaTransfer` | ❌ Hardcoded `is_memory_replica()` only | Add NoF source support | | `CreateMoveTask` / `MoveStart` | ❌ Only allocates MEMORY replicas | Support NoF→NoF or NoF→MEMORY | ### Estimation - **New allocator**: major new subsystem (thousands of lines) - **SPDK/file bridge**: large effort, architecturally questionable - **Drain full-chain NoF support**: hundreds of lines across validation, scheduling, and execution - **Total**: high effort, high architectural risk, uncertain benefit --- ## Comparison | | Option (a): File Transfer | Option (c): Model as NoF | | --- | --- | --- | | **Reuses existing code** | ✅ BatchLoad, TransferWrite, BatchOffload all exist | ❌ Requires new allocator, new transfer path | | **New code required** | ~400–500 lines | Several thousand lines | | **Architectural risk** | Low — pipelining existing primitives | High — abstracting files as blocks | | **Drain scheduling change** | Recognize LOCAL_DISK in `ScheduleDrainJobTasks` | Full NoF drain support chain | | **Staging buffer** | Reuse existing `client_buffer_allocator_` | N/A (block-level access) | | **Transfer protocol** | File I/O + RDMA/TCP (existing) | Must bridge file I/O to SPDK NVMe-oF | | **Source cleanup** | New delete path (simple) | Via allocator free (needs to be built) | | **Deduplication** | Solvable — prefer MEMORY over LOCAL_DISK | Same | | **Fits architecture** | ✅ Composes existing layers | ❌ Forces square peg into round hole | --- ## Open Questions 1. **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)? 2. **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. 3. **Staging buffer sizing**: Should LOCAL_DISK drain use the same `local_buffer_size` pool as Promotion, or have a separate configurable limit? ### Before submitting a new issue... - [ ] Make sure you already searched for relevant issues and read the [documentation](https://kvcache-ai.github.io/Mooncake/)
bsakaguchi 2026-10-29 21:19:04 +00:00
Author
Owner

@github-actions[bot] commented on GitHub (Jul 14, 2026):

Thanks for opening this issue, @liangxu2000!

Field Value
Issue #2885
GitHub user ID 258133064
Reporter @liangxu2000

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.

<!-- gh-comment-id:4965253134 --> @github-actions[bot] commented on GitHub (Jul 14, 2026): Thanks for opening this issue, @liangxu2000! | Field | Value | |-------|-------| | **Issue** | #2885 | | **GitHub user ID** | `258133064` | | **Reporter** | @liangxu2000 | 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.
Author
Owner

@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.

<!-- gh-comment-id:4966018632 --> @liangxu2000 commented on GitHub (Jul 14, 2026): @he-yufeng This RFC proposes Step 2 for [#2817](https://github.com/kvcache-ai/Mooncake/issues/2817) — LOCAL_DISK migration during drain. Would appreciate your thoughts when you get a chance.
Author
Owner

@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

LOCAL_DISK replica (source SSD)
  → BatchLoad → client_buffer_allocator_ (staging buffer, O_DIRECT aligned)
  → TransferWrite → target MEMORY replica (pre-allocated by MoveStart in target's mem segment)

The staging buffer is a structural necessity, not an optimization:

  • Cross-node (drain migration): The source SSD and target memory are on different machines. A local read() cannot write directly into remote memory. The staging buffer is the mandatory hop between local disk I/O and RDMA transfer.
  • O_DIRECT alignment: Even for same-node (Promotion), the storage backend uses O_DIRECT which requires 4096-byte aligned buffers. The DRAM segment allocator does not guarantee this alignment; client_buffer_allocator_ (AlignedClientBufferAllocator) does.

Key Design Points

1. Staging buffer: client_buffer_allocator_, not mem segment

The staging buffer comes from client_buffer_allocator_ (file_storage.cpp:183), a dedicated pool of size local_buffer_size, completely independent from the source client's mem segment.

  • It is not managed by the master — no segment registration, no replica metadata, no MountSegment.
  • It is registered with the transfer engine via RegisterLocalMemory() for RDMA access.
  • It has its own GC (lease timeout, file_storage.cpp:1022).
  • It is already used by ProcessPromotionTasks and BatchGet.

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) calls allocation_strategy_->AllocateFrom() to pre-allocate the target MEMORY replica before any data transfer. If the target has no space, AllocateFrom fails 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:

  • MEMORY migration is faster (no BatchLoad, direct TransferWrite).
  • If LOCAL_DISK migrations run first, they consume target memory, potentially leaving no room for MEMORY migrations.
  • This is a scheduling concern in 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. MoveEnd runs on the master side and only removes the replica metadata — it does not trigger source-side disk I/O.

Options for cleanup:

  • (a) Push an offload-cleanup task to the source client's local_disk_segment in MoveEnd.
  • (b) Batch-cleanup all SSD files for the drained segment when the drain job completes.

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, making GetReplicaBySegmentName unable to find it. Add a LOCAL_DISK branch returning transport_endpoint from LocalDiskReplicaData.

Change 2: master_service.cpp — MoveStart allows 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:

if (source == nullptr || !source->is_completed())
    return REPLICA_NOT_FOUND;
if (!source->is_local_disk_replica() && source->has_invalid_mem_handle())
    return REPLICA_NOT_FOUND;

Change 3: client_service.cpp — ExecuteReplicaTransfer LOCAL_DISK branch (~70 lines) [core]

Add a new branch following the ProcessPromotionTasks pattern:

if (source.is_local_disk_replica()) {
    // 1. AllocateBatch → staging buffer from client_buffer_allocator_
    // 2. BatchLoad → SSD data into staging buffer
    // 3. split_into_slices(staging buffer)
    // 4. TransferWrite → staging → target MEMORY replica (for each target)
    // 5. Release staging buffer (RAII)
}

Change 4: master_service.cpp — MoveEnd handles LOCAL_DISK (~15 lines)

Same has_invalid_mem_handle() fix as Change 2, applied to the MoveEnd source validation path (master_service.cpp:4370).

Change 5: master_service.cpp — ScheduleDrainJobTasks recognizes 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

Aspect Status
New infrastructure needed None — BatchLoad, TransferWrite, client_buffer_allocator_ all exist
Proven reference pattern ProcessPromotionTasks (file_storage.cpp:728)
Estimated code changes ~120-170 lines across 3 files
Architecture risk Low — all three building blocks are battle-tested
Blocking dependency PR #2834 (Step 1: drain job LOCAL_DISK check)

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.

<!-- gh-comment-id:4989641936 --> @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 ``` LOCAL_DISK replica (source SSD) → BatchLoad → client_buffer_allocator_ (staging buffer, O_DIRECT aligned) → TransferWrite → target MEMORY replica (pre-allocated by MoveStart in target's mem segment) ``` The staging buffer is a **structural necessity**, not an optimization: - **Cross-node (drain migration)**: The source SSD and target memory are on different machines. A local `read()` cannot write directly into remote memory. The staging buffer is the mandatory hop between local disk I/O and RDMA transfer. - **O_DIRECT alignment**: Even for same-node (Promotion), the storage backend uses O_DIRECT which requires 4096-byte aligned buffers. The DRAM segment allocator does not guarantee this alignment; `client_buffer_allocator_` (`AlignedClientBufferAllocator`) does. --- ## Key Design Points ### 1. Staging buffer: `client_buffer_allocator_`, not mem segment The staging buffer comes from `client_buffer_allocator_` (`file_storage.cpp:183`), a **dedicated pool** of size `local_buffer_size`, completely independent from the source client's mem segment. - It is **not** managed by the master — no segment registration, no replica metadata, no `MountSegment`. - It is registered with the transfer engine via `RegisterLocalMemory()` for RDMA access. - It has its own GC (lease timeout, `file_storage.cpp:1022`). - It is already used by `ProcessPromotionTasks` and `BatchGet`. 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`) calls `allocation_strategy_->AllocateFrom()` to pre-allocate the target MEMORY replica **before any data transfer**. If the target has no space, `AllocateFrom` fails 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: - MEMORY migration is faster (no BatchLoad, direct TransferWrite). - If LOCAL_DISK migrations run first, they consume target memory, potentially leaving no room for MEMORY migrations. - This is a scheduling concern in `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**. `MoveEnd` runs on the master side and only removes the replica metadata — it does not trigger source-side disk I/O. Options for cleanup: - (a) Push an offload-cleanup task to the source client's local_disk_segment in `MoveEnd`. - (b) Batch-cleanup all SSD files for the drained segment when the drain job completes. 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, making `GetReplicaBySegmentName` unable to find it. Add a LOCAL_DISK branch returning `transport_endpoint` from `LocalDiskReplicaData`. ### Change 2: `master_service.cpp` — `MoveStart` allows 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: ``` if (source == nullptr || !source->is_completed()) return REPLICA_NOT_FOUND; if (!source->is_local_disk_replica() && source->has_invalid_mem_handle()) return REPLICA_NOT_FOUND; ``` ### Change 3: `client_service.cpp` — `ExecuteReplicaTransfer` LOCAL_DISK branch (~70 lines) **[core]** Add a new branch following the `ProcessPromotionTasks` pattern: ``` if (source.is_local_disk_replica()) { // 1. AllocateBatch → staging buffer from client_buffer_allocator_ // 2. BatchLoad → SSD data into staging buffer // 3. split_into_slices(staging buffer) // 4. TransferWrite → staging → target MEMORY replica (for each target) // 5. Release staging buffer (RAII) } ``` ### Change 4: `master_service.cpp` — `MoveEnd` handles LOCAL_DISK (~15 lines) Same `has_invalid_mem_handle()` fix as Change 2, applied to the `MoveEnd` source validation path (`master_service.cpp:4370`). ### Change 5: `master_service.cpp` — `ScheduleDrainJobTasks` recognizes 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 | Aspect | Status | |--------|--------| | New infrastructure needed | **None** — BatchLoad, TransferWrite, client_buffer_allocator_ all exist | | Proven reference pattern | `ProcessPromotionTasks` (file_storage.cpp:728) | | Estimated code changes | ~120-170 lines across 3 files | | Architecture risk | **Low** — all three building blocks are battle-tested | | Blocking dependency | PR #2834 (Step 1: drain job LOCAL_DISK check) | 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.
Author
Owner

@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.

<!-- gh-comment-id:5020842358 --> @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.
Author
Owner

@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

RFC "What Needs to Be Built" Implementation
1. Lift is_memory_replica() restriction in ExecuteReplicaTransfer New LOCAL_DISK branch: LoadBatchFromLocalDisk (AllocateBatch + BatchLoad with O_DIRECT align_down/align_up) → TransferWrite → target MEMORY replica
2. ScheduleDrainJobTasks recognize LOCAL_DISK get_segment_names() now returns transport_endpoint for LOCAL_DISK; scheduler scans them with MEMORY-first dedup
3. Staging buffer management Reuses existing client_buffer_allocator_ (same pool as Promotion); naturally bounded by max_concurrency
4. Source LOCAL_DISK cleanup MoveEnd drops LOCAL_DISK master metadata when is_drain=true. Physical SSD file intentionally retained as safety net (no destructive deletion in this PR)
5. Deduplication MEMORY-first: if a key has both MEMORY + LOCAL_DISK on the same client, only MEMORY is migrated; LOCAL_DISK metadata is cleaned up in MoveEnd

Design decisions (addressing the RFC's Open Questions)

  1. 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.

  2. Partial drain semantics: The existing kMaxDrainUnitRetries (3) + terminal_failed_unit_keys mechanism already provides per-key granularity. If some keys succeed and others exhaust retries, MaybeCompleteDrainJob marks the job FAILED with succeeded_units > 0, failed_units > 0 — effectively a partial result. The segment stays DRAINING (not DRAINED), so the operator can inspect and retry. I did not add a separate PARTIAL status; the existing fields already convey the information.

  3. Staging buffer sizing: Uses the same local_buffer_size pool 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_drain flag

During implementation I found that the RFC's step 4 (source cleanup) has a subtle safety issue: the public create_move_task API (used for load-balancing) goes through the same Move → MoveEnd path. 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_drain through:

ScheduleDrainJobTasks (sets true) → ReplicaMovePayload → Client::Move → MasterClient::MoveEnd RPC → MasterService::MoveEnd (gates cleanup on is_drain==true)

Manual moves default to is_drain=false, preserving the source's LOCAL_DISK replica.

Test coverage (5 e2e tests, all pass)

Test What it proves
DrainLocalDiskReplicasToPeer Baseline: MEMORY-first dedup works, LOCAL_DISK metadata dropped
ManualMovePreservesLocalDiskBackup is_drain gating: manual move does NOT drop LD; subsequent drain uses LOCAL_DISK transfer path with non-aligned O_DIRECT reads ({4096, 7001, 12000, 5000} bytes)
DrainFailsGracefullyWhenLocalDiskDataMissing Failure: SSD files deleted → job reaches FAILED (no hang), retries exactly 3×per key, surviving replicas intact
DrainAfterEviction_OffloadOnEvict Realistic eviction (offload_on_evict=true): mixed MEMORY + LOCAL_DISK paths in one drain
DrainAfterEviction_ImmediateOffload Same with offload_on_evict=false

Scope

  • ~200 lines non-test source change (client_service, master_service, headers for is_drain threading)
  • ~1250 lines test code
  • No new RPC, no new master state, no new task type
<!-- gh-comment-id:5057096104 --> @liangxu2000 commented on GitHub (Jul 23, 2026): ## Option (a) implementation is ready — PR [#3085](https://github.com/kvcache-ai/Mooncake/pull/3085) I've implemented Option (a) (Targeted File Transfer) as described in this RFC. The PR is up: [#3085](https://github.com/kvcache-ai/Mooncake/pull/3085). ### How it maps to this RFC | RFC "What Needs to Be Built" | Implementation | | --- | --- | | 1. Lift `is_memory_replica()` restriction in `ExecuteReplicaTransfer` | New `LOCAL_DISK` branch: `LoadBatchFromLocalDisk` (AllocateBatch + BatchLoad with O_DIRECT align_down/align_up) → `TransferWrite` → target MEMORY replica | | 2. `ScheduleDrainJobTasks` recognize LOCAL_DISK | `get_segment_names()` now returns `transport_endpoint` for LOCAL_DISK; scheduler scans them with **MEMORY-first dedup** | | 3. Staging buffer management | Reuses existing `client_buffer_allocator_` (same pool as Promotion); naturally bounded by `max_concurrency` | | 4. Source LOCAL_DISK cleanup | `MoveEnd` drops LOCAL_DISK **master metadata** when `is_drain=true`. Physical SSD file intentionally retained as safety net (no destructive deletion in this PR) | | 5. Deduplication | MEMORY-first: if a key has both MEMORY + LOCAL_DISK on the same client, only MEMORY is migrated; LOCAL_DISK metadata is cleaned up in MoveEnd | ### Design decisions (addressing the RFC's Open Questions) 1. **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. 2. **Partial drain semantics:** The existing `kMaxDrainUnitRetries` (3) + `terminal_failed_unit_keys` mechanism already provides per-key granularity. If some keys succeed and others exhaust retries, `MaybeCompleteDrainJob` marks the job `FAILED` with `succeeded_units > 0, failed_units > 0` — effectively a partial result. The segment stays `DRAINING` (not `DRAINED`), so the operator can inspect and retry. I did not add a separate `PARTIAL` status; the existing fields already convey the information. 3. **Staging buffer sizing:** Uses the same `local_buffer_size` pool 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_drain` flag During implementation I found that the RFC's step 4 (source cleanup) has a subtle safety issue: the public `create_move_task` API (used for load-balancing) goes through the same `Move → MoveEnd` path. 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_drain` through: ``` ScheduleDrainJobTasks (sets true) → ReplicaMovePayload → Client::Move → MasterClient::MoveEnd RPC → MasterService::MoveEnd (gates cleanup on is_drain==true) ``` Manual moves default to `is_drain=false`, preserving the source's LOCAL_DISK replica. ### Test coverage (5 e2e tests, all pass) | Test | What it proves | | --- | --- | | `DrainLocalDiskReplicasToPeer` | Baseline: MEMORY-first dedup works, LOCAL_DISK metadata dropped | | `ManualMovePreservesLocalDiskBackup` | `is_drain` gating: manual move does NOT drop LD; subsequent drain uses LOCAL_DISK transfer path with non-aligned O_DIRECT reads ({4096, 7001, 12000, 5000} bytes) | | `DrainFailsGracefullyWhenLocalDiskDataMissing` | Failure: SSD files deleted → job reaches FAILED (no hang), retries exactly 3×per key, surviving replicas intact | | `DrainAfterEviction_OffloadOnEvict` | Realistic eviction (`offload_on_evict=true`): mixed MEMORY + LOCAL_DISK paths in one drain | | `DrainAfterEviction_ImmediateOffload` | Same with `offload_on_evict=false` | ### Scope - ~200 lines non-test source change (client_service, master_service, headers for is_drain threading) - ~1250 lines test code - No new RPC, no new master state, no new task type
Author
Owner

@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.

<!-- gh-comment-id:6331824849 --> @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.
Author
Owner

@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.

<!-- gh-comment-id:6487047437 --> @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.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
kvcache-ai/Mooncake#748
No description provided.