[GH-ISSUE #4955] [Track]: [Store] MasterService follow-ups after the tenant-first migration #1258

Open
opened 2026-10-29 21:24:32 +00:00 by bsakaguchi · 10 comments
Owner

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

#4557 moved MasterService onto tenant namespaces, per-key locks, the Evictor and TenantQuotaPolicy. This issue tracks what is left after it, in priority order. A PR that takes an item links back here.

Findings come from reading main at b7a50b39c, rechecked against main after #4736. Nothing below has been measured unless it says so. Performance items need a before/after number before they are changed.

How the architecture evolves

Now (after #4557)

The metadata core is per tenant and per key, but MasterService is still where every feature meets. It holds most features' state and threads, writes the OpLog from each call site, and runs every subsystem's cleanup when an object goes. The components already extracted call back into it.

flowchart TB
    RPC["WrappedMasterService (RPC)"] --> MS
    subgraph MS["MasterService: state, threads and call order of every feature"]
        REQ["Request flows<br/>Put, Get, Remove, Copy, Move, Revoke"]
        ERASE["EraseMetadata<br/>ten cleanup steps for nine subsystems"]
        OPL["34 OpLog call sites over 4 append helpers"]
        THR["7 hand-written threads<br/>eviction, client monitor, NoF heartbeat,<br/>drain, task cleanup, weights, HTTP cleanup"]
        HALF["State of half-extracted features<br/>promotion, dynamic replication, KV epochs, soft pin"]
    end
    subgraph COMP["Extracted components"]
        OFF["ClientOffboardingWorker"]
        WB["MasterStoreBackend"]
        EVR["Evictor"]
    end
    MS --> COMP
    OFF -- "MasterService*" --> MS
    WB -- "friend" --> MS
    EVR -- "StoreControl" --> MS
    subgraph CORE["Metadata core"]
        NS["Namespace<br/>one policy, eviction index, in-flight list"] --> OR["ObjectRoute stripes → KeySlot<br/>~1.1 KB, 7 heap blocks per object"]
    end
    REQ --> NS
    ERASE --> NS
    OPL --> OW["OrderedOpLogWriter"] --> SB["Standby"]

Target

Each component owns its loop: its state, threads and invariants. MasterService only assembles them. A component meets the rest of the master in two ways. Route hooks tell it when a key changes, and ports let it start work. The OpLog is one of the hooks, registered only when it is enabled.

flowchart TB
    RPC["WrappedMasterService (RPC)"] --> MS["MasterService<br/>builds and wires components,<br/>starts and stops them, forwards RPCs"]
    MS --> FEAT
    subgraph FEAT["Components: own state, threads and invariants; never reach MasterService or each other"]
        OBJ["Object read/write"]
        EV["Evictor"]
        PR["Promotion"]
        DR["Dynamic replication"]
        DJ["Drain scheduler"]
        CO["Client offboarding"]
        NH["NoF heartbeat"]
        HC["HTTP metadata cleanup"]
        WM["Weight management"]
        HA["HA: OpLog hook, restore"]
    end
    subgraph PORTS["Ports"]
        MP["Metadata port<br/>key access, ReadBatch, cursors, hook registration"]
        CR["ClientRegistry"]
        AL["Placement and allocation"]
        TM["ClientTaskManager"]
        LG["Lifecycle gate"]
    end
    FEAT -- "start work" --> PORTS
    subgraph CORE["Metadata core"]
        NS["Namespace hook chain<br/>WriteBegin, Grow, Replace, TearDown, WriteRelease"] --> OR["ObjectRoute → KeySlot<br/>key once, lease, part and replica inline"]
    end
    MP --> NS
    NS -. "a key changed" .-> FEAT
    HA --> OW["OrderedOpLogWriter"] --> SB["Standby"]

Order

Arrows are dependencies. A solid arrow means the later item needs the earlier one; a dashed arrow means it gets simpler. Items without arrows can start any time.

flowchart LR
    subgraph P0["P0 correctness"]
        HOOK["Route hook chain and WriteBegin"]
        OPH["OpLog as an optional route hook"]
        RST["Restore repair batch"]
        PRED["Shared eviction predicate"]
    end
    subgraph P1["P1 foreground"]
        RB["Batch reads by stripe"]
        BW["Batch writes: global locks once per batch"]
    end
    subgraph P2["P2 background"]
        RANK["Evictor ranks by evictable time"]
        SWEEP["One-pass in-flight sweep"]
        SCAN["No timed full scans"]
        WR["OpLog writer batching"]
    end
    subgraph P3["P3 memory"]
        MEM["Key once, inline lease, part and replica"]
    end
    subgraph P4["P4 structure"]
        MPT["Metadata port out of MasterService"]
        TD["Teardown and accounting as hooks"]
        EXT["Extract components"]
        SP["Soft-pin index on the key slot"]
        GRP["Group ownership"]
        DRS["Dynamic replication split"]
        DUP["Share Copy/Move and serializers"]
    end
    HOOK --> OPH
    HOOK --> TD
    OPH --> WR
    MPT --> EXT
    TD --> EXT
    OPH -.-> DUP
    OPH -.-> RST
    SP --> RANK
    GRP --> RANK
    EXT -.-> SCAN
    DRS -.-> MEM

The cheap part of "ranks by evictable time" does not need the soft-pin index or group ownership: refile a key the evictor skipped, and keep managed groups out of the index. Ranking by the real deadline does need them.

Target structure

Most extracted components are only half out. The decision is in the component, but its state, thread or execution steps stay in MasterService, so it calls back in:

  • ClientOffboardingWorker holds a MasterService*, and its jobs run in MasterService::ProcessClientOffboardingJob.
  • MasterStoreBackend is a friend that reads enable_oplog_ and ordered_oplog_writer_.
  • PromotionCandidateTracker holds only the retry index.

The clearest case is EraseMetadata. Every teardown through an accessor runs, in an order MasterService hard-codes, cleanup for:

  • KV events
  • the offload task and its LocalSSD mailbox entry
  • the replication task
  • the promotion task and candidate
  • dynamic replication
  • local disk usage
  • DFS replicas
  • cache total
  • the soft-pin index

A component is closed when:

  1. All of its feature lives in it: state, threads, config and metrics. MasterService keeps no member for it.
  2. It keeps its own invariants and offers whole operations. It does not hand out locks, indexes or transactions for the caller to compose.
  3. It does not hold MasterService and is not its friend. It is given the ports it uses when it is built.
  4. It does not call other components. Per-key events reach it through route hooks.
Route hooks Ports
Direction The route calls the component when a key changes The component starts work
Runs Under the key's lock; leaf locks only, no other key, no blocking On the component's own thread, or for an RPC
For State that must match each key: indexes, accounting, tasks on the object Scans, writes to keys, allocation

Components use them as follows:

  • Hooks only: the quota policy, the eviction index, the in-flight list, cache total, the soft-pin index, and KV events (the hook enqueues; the publisher thread sends).
  • Hooks and ports: the Evictor, promotion, dynamic replication, local disk and offload, DFS, and HA.
  • Ports only:
    • object read/write, which triggers the hooks;
    • drain, client offboarding, NoF heartbeat and weight management;
    • HTTP metadata cleanup, which follows ClientRegistry's client-down notice;
    • the task manager.

What stays in MasterService:

  • building the ports and components from the config;
  • registering hooks in order (the OpLog commit runs before the KV stored event);
  • starting them in order and stopping them in reverse;
  • forwarding each RPC.

P0: correctness of HA writes

Most of these come from MasterService writing the OpLog in four different ways (AppendOpLogVisibleBeforeDurable, AppendOpLogWithDurableFinalize, AppendReservedOpLogWithDurableFinalize, AppendOpLogBatchWithDurableFinalize, 34 call sites in all). Each call site chooses its own order of mutating, logging and handling failure.

  • OpLog as an optional route hook. The OpLog is optional, so it should not be a step every write path calls.
    • The hook. The route already reports writes to the namespace (RouteObserver::OnWriteRelease, NamespacePolicy), and the quota policy and the eviction index hang on that. The OpLog becomes one more hook, registered only when it is enabled:
      • OnWriteBegin runs under the key lock before the first change and may fail. It validates the entry and reserves a slot.
      • OnWriteRelease runs with the lock still held and cannot fail. It builds the entry from the post-state (COMPLETE replicas only) and commits it into that slot.
    • Why this order. Reserving under the key lock keeps the order per key; a slot reserved before the lock could put two writes of one key in the wrong order. A write that fails does so before anything changed, so the next three bugs go away as a class. Without the OpLog the hook is absent and a write costs what it does today.
    • The four contracts. Immediate is the default. Tombstone and deferred writes only say what goes: without the OpLog it goes at release; with it, it stays REMOVED until the durable finalize, whose own write the hook does not log again. This generalizes ImmediateReclaimer and OpLogReclaimer. Barrier users (restore, weight ops) exist only under HA and keep calling the writer.
    • What the hooks need:
      • a hook chain on Namespace, instead of one policy plus an eviction index and in-flight list wired in by hand;
      • OnWriteBegin, called on every write under the key lock and allowed to fail (AdmitWrite runs before the lock, and OnGrow only runs on growth);
      • the kind of change recorded on the guard (new write, replicas removed, key removed), so the hook picks PUT_END or REMOVE;
      • to check: that a commit into a held reservation can fail only when the writer is terminal.
    • PutEnd drops its OpLog entry and still succeeds. When the append fails, PutEnd logs a warning and returns OK after PublishKvStored. Reservations fail while the writer retries etcd and when more than 1024 slots are waiting, so the object never reaches the standby and is lost on failover. Mount, remount and unmount drop their entries the same way; a standby that misses a mount rejects that endpoint's objects on restore.
    • Removed replicas are not restored when the append fails. BatchReplicaClear, PutRevoke, EvictDiskReplicaImpl, MoveEnd and NoFBatchEvict mark replicas removed and then continue or return on failure. The replica stays REMOVED: its memory and quota are never released (MemoryCharge counts REMOVED replicas), and the object can no longer be removed. The writer always rejects an entry whose key is longer than 4096 bytes, which the master does not check, or whose payload is over 10 MiB. PutRevoke cannot simply call cancel_remove(), which restores COMPLETE rather than PROCESSING.
    • One rejected entry stops the eviction round. OpLogReclaimer reports any failure the same way, so a single oversized key stops the round as backpressure would. With the hook, a rejected entry skips that key, and only backpressure or a terminal writer stops the round.
  • Standby restore repair batch. It reserves one batch for all repair entries, which the writer rejects above oplog_batch_max_entries (default 1024), so a restore with more repairs fails every time. It also waits for the callback with no timeout while holding the client and state locks exclusively, and a terminal writer never calls back. AwaitDurable already returns an error in that case. #4736 added the restore state machine and did not change this.
  • DFS bucket eviction skips the managed-group check. The eviction-eligibility predicate is copied four times. The bucket DFS copy has no IsManagedGroup check, unlike shard DFS, NoF and drain, so a managed weight may be evicted from a DFS bucket. One shared predicate fixes it.
  • Smaller:
    • PutEnd and the restore repair serialize PROCESSING and REMOVED replicas into the OpLog.
    • EvictDiskReplicaImpl takes a key lock without master_state_mutex_, against the lock order.
    • BatchRemove runs stale-handle cleanup that a single Remove does not, so the two return different errors in HA mode.

P1: foreground latency

  • Batch reads by stripe. BatchGetReplicaList and BatchExistKey build one accessor per key: two hashes, the stripe lock, a handle refcount, the slot lock and a CAS. That is about six atomic RMWs per key, on cache lines all lookup threads share. Before #4557 a batch took each shard once. This is the likely cause of the 30000-key read p99 regression measured in #4844 (73–78 → 88–95 ms). An ObjectRoute::ReadBatch would group keys by stripe and lock each stripe once.

  • Batch writes take global locks per key. For each key, PutStart takes:

    • client_mutex_, two or three times;
    • master_state_mutex_, once;
    • the tenant registration mutex (one per shard of the quota map);
    • for a non-default tenant, the namespace table lock.

    These can move to the start of the batch.

  • PutStart and UpsertStart build the set of live clients for every key. It is only used for an existing key with a local-disk replica.

  • IsManagedGroup locks the weight store for ungrouped keys. Remove, BatchRemove, UpsertStart and NoF eviction call it without checking for an empty group id, under the key's write lock.

  • Smaller:

    • the key and tenant are copied twice per write (ObjectIdentity by value into the accessor);
    • AdmitWithDfsRecovery takes a std::function and allocates per put;
    • BuildStaleHandleCleanupPlan copies every replica descriptor when nothing is stale.

P2: background work that competes with requests

  • Keys the evictor always skips sit at the head of the index. The lease index ranks by lease expiry and excludes only hard pins. So these keys are write-locked and judged again every round, every 10 ms under pressure:

    • soft-pinned keys, when soft-pinned objects may not be evicted;
    • managed groups;
    • keys with an offload in flight.

    Rank by when a key can actually be evicted, or refile a skipped key later.

  • Expired in-flight work is swept with four lock passes. DiscardExpiredProcessingReplicas runs from both memory and NoF eviction each tick. It copies the in-flight key list and write-locks each in-flight key four times: for processing, replication, offload and promotion. These are the keys PutEnd is writing. It should be one pass, one lock per key, ordered by deadline.

  • Full scans on a timer.

    • Drain scheduling scans every object every 500 ms per job, and does not stop once its slots are filled.
    • Client offboarding scans twice per retry.
    • NoF eviction scans everything every 10 ms while nothing is evictable.
    • Unmount in HA mode runs ClearInvalidHandles on the RPC thread.

    Each scan holds a stripe's shared lock across its loop body, which may stall new keys in that stripe (unmeasured).

  • OpLog writer cost.

    • The payload is copied four to five times, twice under the writer's global lock.
    • Each key takes that lock twice, to reserve and to commit.
    • Every finalize closure heap-allocates.
    • One condition variable wakes the writer, the callback thread and retries on every commit.

P3: memory per object

About 1,136 bytes in seven heap blocks for an ungrouped object with one memory replica and a 64-byte key (sizes from sizeof, chunks from glibc rounding):

  • Drop ObjectMetadata::user_key (a second copy of the key) and ObjectMetadata::tenant_id (the namespace knows it).
  • Keep an ungrouped object's lease inline instead of in a shared_ptr.
  • Make AllocatedBuffer::protocol an enum and move the CXL-only segment name out (−64 B per memory replica).
  • Keep the first part and first replica inline (−2 heap blocks), and size Replica for the memory case.
  • Measure a smaller per-key lock than the 56-byte std::shared_mutex.

A hook that keeps a per-key index (the eviction index, the in-flight list, a soft-pin index on the slot) needs words or a list hook in SlotOwnerState on every key. Count that before adding one.

P4: structure

First:

  • Metadata port out of MasterService. MetadataAccessorRW/RO are nested friend classes that call back into CleanupInvalidMemoryReplicas and EraseMetadata, and the OpLog helpers are private to MasterService. So any component that writes a key has to go through MasterService. Key access, ReadBatch, cursors and hook registration should move into a port of their own. The port needs a new name, since metadata_store.h holds the standby structs.
  • Teardown and accounting as hooks. Each step of EraseMetadata and of replica removal moves to the component it belongs to, on OnTearDown, OnReplace or OnWriteRelease. That covers CleanupInvalidMemoryReplicas, CancelPromotionTaskForRemovedReplicas, the 26 *WithCacheTotalAccounting wrappers and the 9 SyncCacheTotalAccounting calls.

Then each component takes its members and its thread out of MasterService, and its periodic loop becomes a BackgroundWorker:

  • Evictor:
    • what moves in: MemoryEviction (policy, reclaimers, offload queue, KV observer), BatchEvict, NoFBatchEvict, RunDfsEviction, the pressure flags and watermarks, and PolicyStoreAccess;
    • the eviction thread then only evicts. Today it also sweeps expired writes and retries promotions.
  • Promotion: the in-flight counter, the sketch, the settings, TryPushPromotionQueue and the retry loop, plus hooks that cancel a task when its source goes.
  • Dynamic replication: the five execution steps still in MasterService. The plan from #4557 (lease and cooldown as one value on ObjectState, stateless admission and planning, expiry through ObjectRef instead of a pending index) still applies.
  • Drain scheduler: DrainJob, drain_jobs_, its validation and scheduling functions, and job_dispatch_thread_.
  • ClientRegistry: client_mutex_, the liveness records, ok_client_, host ids and client_monitor_thread_. It tells subscribers when a client comes up or goes down.
  • Client offboarding: ProcessClientOffboardingJob moves into the worker, and the back-pointer goes.
  • NoF heartbeat monitor and HTTP metadata cleaner. The cleaner acts on ClientRegistry's client-down notice.
  • KV events: tenant epochs, media snapshots and PublishKv*.
  • Soft-pin deadline index: one per namespace, kept by hooks on the key slot instead of a nested class of MasterService with two copies of each key.
  • Weight management: MasterStoreBackend stops reading MasterService members.
  • HA: the OpLog hook, the writer and serializers, and the standby restore state.
  • Lifecycle gate: components wait on it instead of taking master_state_mutex_. #4736 started this.

From #4557:

  • Eviction state on the key slot, and who owns a group.
    • The route keeps the shared lease and a member count.
    • Eviction membership lives only in the index, as typed entries.
    • Weight import commit checks a key list against the manifest.
  • Cleanups left by the cutover:
    • fold KvEventObserver into the immediate reclaimer;
    • keep one quota policy handle instead of policy_ and tenant_quota_policy_;
    • share the grow-and-allocate step of UpsertStart and AllocateAndInsertMetadata.
  • Eviction cursor: read a bucket's keys in batches, and name objects internally by slot instead of by key string.

New:

  • Remove dead and test-only code:
    • ValidateDrainRequest, which has no callers;
    • SerializeMetadataForOpLogWithoutMemReplicas;
    • the public SubmitReplicaActionProposal and the key-only BatchReplicaClear overload, used only by tests;
    • the test hooks kv_remove_all_tenant_hook_ and nof_probe_fn_.
  • Share duplicated code: CopyEnd/MoveEnd, CopyStart/MoveStart, and the three SerializeMetadataForOpLog*.

Open:

  • Do the request flows (Put, Get, Remove, Copy, Move) become an object read/write component, or stay as the one piece of orchestration MasterService keeps?
  • Once components stand alone, each gets its own unit tests instead of going through MasterServiceTestPeer.
Originally created by @Aionw on GitHub (Oct 26, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4955 #4557 moved `MasterService` onto tenant namespaces, per-key locks, the `Evictor` and `TenantQuotaPolicy`. This issue tracks what is left after it, in priority order. A PR that takes an item links back here. Findings come from reading `main` at b7a50b39c, rechecked against `main` after #4736. Nothing below has been measured unless it says so. Performance items need a before/after number before they are changed. ## How the architecture evolves ### Now (after #4557) The metadata core is per tenant and per key, but `MasterService` is still where every feature meets. It holds most features' state and threads, writes the OpLog from each call site, and runs every subsystem's cleanup when an object goes. The components already extracted call back into it. ```mermaid flowchart TB RPC["WrappedMasterService (RPC)"] --> MS subgraph MS["MasterService: state, threads and call order of every feature"] REQ["Request flows<br/>Put, Get, Remove, Copy, Move, Revoke"] ERASE["EraseMetadata<br/>ten cleanup steps for nine subsystems"] OPL["34 OpLog call sites over 4 append helpers"] THR["7 hand-written threads<br/>eviction, client monitor, NoF heartbeat,<br/>drain, task cleanup, weights, HTTP cleanup"] HALF["State of half-extracted features<br/>promotion, dynamic replication, KV epochs, soft pin"] end subgraph COMP["Extracted components"] OFF["ClientOffboardingWorker"] WB["MasterStoreBackend"] EVR["Evictor"] end MS --> COMP OFF -- "MasterService*" --> MS WB -- "friend" --> MS EVR -- "StoreControl" --> MS subgraph CORE["Metadata core"] NS["Namespace<br/>one policy, eviction index, in-flight list"] --> OR["ObjectRoute stripes → KeySlot<br/>~1.1 KB, 7 heap blocks per object"] end REQ --> NS ERASE --> NS OPL --> OW["OrderedOpLogWriter"] --> SB["Standby"] ``` ### Target Each component owns its loop: its state, threads and invariants. `MasterService` only assembles them. A component meets the rest of the master in two ways. Route hooks tell it when a key changes, and ports let it start work. The OpLog is one of the hooks, registered only when it is enabled. ```mermaid flowchart TB RPC["WrappedMasterService (RPC)"] --> MS["MasterService<br/>builds and wires components,<br/>starts and stops them, forwards RPCs"] MS --> FEAT subgraph FEAT["Components: own state, threads and invariants; never reach MasterService or each other"] OBJ["Object read/write"] EV["Evictor"] PR["Promotion"] DR["Dynamic replication"] DJ["Drain scheduler"] CO["Client offboarding"] NH["NoF heartbeat"] HC["HTTP metadata cleanup"] WM["Weight management"] HA["HA: OpLog hook, restore"] end subgraph PORTS["Ports"] MP["Metadata port<br/>key access, ReadBatch, cursors, hook registration"] CR["ClientRegistry"] AL["Placement and allocation"] TM["ClientTaskManager"] LG["Lifecycle gate"] end FEAT -- "start work" --> PORTS subgraph CORE["Metadata core"] NS["Namespace hook chain<br/>WriteBegin, Grow, Replace, TearDown, WriteRelease"] --> OR["ObjectRoute → KeySlot<br/>key once, lease, part and replica inline"] end MP --> NS NS -. "a key changed" .-> FEAT HA --> OW["OrderedOpLogWriter"] --> SB["Standby"] ``` ### Order Arrows are dependencies. A solid arrow means the later item needs the earlier one; a dashed arrow means it gets simpler. Items without arrows can start any time. ```mermaid flowchart LR subgraph P0["P0 correctness"] HOOK["Route hook chain and WriteBegin"] OPH["OpLog as an optional route hook"] RST["Restore repair batch"] PRED["Shared eviction predicate"] end subgraph P1["P1 foreground"] RB["Batch reads by stripe"] BW["Batch writes: global locks once per batch"] end subgraph P2["P2 background"] RANK["Evictor ranks by evictable time"] SWEEP["One-pass in-flight sweep"] SCAN["No timed full scans"] WR["OpLog writer batching"] end subgraph P3["P3 memory"] MEM["Key once, inline lease, part and replica"] end subgraph P4["P4 structure"] MPT["Metadata port out of MasterService"] TD["Teardown and accounting as hooks"] EXT["Extract components"] SP["Soft-pin index on the key slot"] GRP["Group ownership"] DRS["Dynamic replication split"] DUP["Share Copy/Move and serializers"] end HOOK --> OPH HOOK --> TD OPH --> WR MPT --> EXT TD --> EXT OPH -.-> DUP OPH -.-> RST SP --> RANK GRP --> RANK EXT -.-> SCAN DRS -.-> MEM ``` The cheap part of "ranks by evictable time" does not need the soft-pin index or group ownership: refile a key the evictor skipped, and keep managed groups out of the index. Ranking by the real deadline does need them. ## Target structure Most extracted components are only half out. The decision is in the component, but its state, thread or execution steps stay in `MasterService`, so it calls back in: - `ClientOffboardingWorker` holds a `MasterService*`, and its jobs run in `MasterService::ProcessClientOffboardingJob`. - `MasterStoreBackend` is a friend that reads `enable_oplog_` and `ordered_oplog_writer_`. - `PromotionCandidateTracker` holds only the retry index. The clearest case is `EraseMetadata`. Every teardown through an accessor runs, in an order `MasterService` hard-codes, cleanup for: - KV events - the offload task and its LocalSSD mailbox entry - the replication task - the promotion task and candidate - dynamic replication - local disk usage - DFS replicas - cache total - the soft-pin index A component is closed when: 1. All of its feature lives in it: state, threads, config and metrics. `MasterService` keeps no member for it. 2. It keeps its own invariants and offers whole operations. It does not hand out locks, indexes or transactions for the caller to compose. 3. It does not hold `MasterService` and is not its friend. It is given the ports it uses when it is built. 4. It does not call other components. Per-key events reach it through route hooks. | | Route hooks | Ports | |---|---|---| | Direction | The route calls the component when a key changes | The component starts work | | Runs | Under the key's lock; leaf locks only, no other key, no blocking | On the component's own thread, or for an RPC | | For | State that must match each key: indexes, accounting, tasks on the object | Scans, writes to keys, allocation | Components use them as follows: - **Hooks only:** the quota policy, the eviction index, the in-flight list, cache total, the soft-pin index, and KV events (the hook enqueues; the publisher thread sends). - **Hooks and ports:** the Evictor, promotion, dynamic replication, local disk and offload, DFS, and HA. - **Ports only:** - object read/write, which triggers the hooks; - drain, client offboarding, NoF heartbeat and weight management; - HTTP metadata cleanup, which follows `ClientRegistry`'s client-down notice; - the task manager. What stays in `MasterService`: - building the ports and components from the config; - registering hooks in order (the OpLog commit runs before the KV stored event); - starting them in order and stopping them in reverse; - forwarding each RPC. ## P0: correctness of HA writes Most of these come from `MasterService` writing the OpLog in four different ways (`AppendOpLogVisibleBeforeDurable`, `AppendOpLogWithDurableFinalize`, `AppendReservedOpLogWithDurableFinalize`, `AppendOpLogBatchWithDurableFinalize`, 34 call sites in all). Each call site chooses its own order of mutating, logging and handling failure. - [ ] **OpLog as an optional route hook.** The OpLog is optional, so it should not be a step every write path calls. - **The hook.** The route already reports writes to the namespace (`RouteObserver::OnWriteRelease`, `NamespacePolicy`), and the quota policy and the eviction index hang on that. The OpLog becomes one more hook, registered only when it is enabled: - `OnWriteBegin` runs under the key lock before the first change and may fail. It validates the entry and reserves a slot. - `OnWriteRelease` runs with the lock still held and cannot fail. It builds the entry from the post-state (COMPLETE replicas only) and commits it into that slot. - **Why this order.** Reserving under the key lock keeps the order per key; a slot reserved before the lock could put two writes of one key in the wrong order. A write that fails does so before anything changed, so the next three bugs go away as a class. Without the OpLog the hook is absent and a write costs what it does today. - **The four contracts.** *Immediate* is the default. *Tombstone* and *deferred* writes only say what goes: without the OpLog it goes at release; with it, it stays REMOVED until the durable finalize, whose own write the hook does not log again. This generalizes `ImmediateReclaimer` and `OpLogReclaimer`. *Barrier* users (restore, weight ops) exist only under HA and keep calling the writer. - **What the hooks need:** - a hook chain on `Namespace`, instead of one policy plus an eviction index and in-flight list wired in by hand; - `OnWriteBegin`, called on every write under the key lock and allowed to fail (`AdmitWrite` runs before the lock, and `OnGrow` only runs on growth); - the kind of change recorded on the guard (new write, replicas removed, key removed), so the hook picks PUT_END or REMOVE; - to check: that a commit into a held reservation can fail only when the writer is terminal. - **PutEnd drops its OpLog entry and still succeeds.** When the append fails, PutEnd logs a warning and returns OK after `PublishKvStored`. Reservations fail while the writer retries etcd and when more than 1024 slots are waiting, so the object never reaches the standby and is lost on failover. Mount, remount and unmount drop their entries the same way; a standby that misses a mount rejects that endpoint's objects on restore. - **Removed replicas are not restored when the append fails.** `BatchReplicaClear`, `PutRevoke`, `EvictDiskReplicaImpl`, `MoveEnd` and `NoFBatchEvict` mark replicas removed and then `continue` or return on failure. The replica stays REMOVED: its memory and quota are never released (`MemoryCharge` counts REMOVED replicas), and the object can no longer be removed. The writer always rejects an entry whose key is longer than 4096 bytes, which the master does not check, or whose payload is over 10 MiB. `PutRevoke` cannot simply call `cancel_remove()`, which restores COMPLETE rather than PROCESSING. - **One rejected entry stops the eviction round.** `OpLogReclaimer` reports any failure the same way, so a single oversized key stops the round as backpressure would. With the hook, a rejected entry skips that key, and only backpressure or a terminal writer stops the round. - [ ] **Standby restore repair batch.** It reserves one batch for all repair entries, which the writer rejects above `oplog_batch_max_entries` (default 1024), so a restore with more repairs fails every time. It also waits for the callback with no timeout while holding the client and state locks exclusively, and a terminal writer never calls back. `AwaitDurable` already returns an error in that case. #4736 added the restore state machine and did not change this. - [ ] **DFS bucket eviction skips the managed-group check.** The eviction-eligibility predicate is copied four times. The bucket DFS copy has no `IsManagedGroup` check, unlike shard DFS, NoF and drain, so a managed weight may be evicted from a DFS bucket. One shared predicate fixes it. - [ ] Smaller: - PutEnd and the restore repair serialize PROCESSING and REMOVED replicas into the OpLog. - `EvictDiskReplicaImpl` takes a key lock without `master_state_mutex_`, against the lock order. - `BatchRemove` runs stale-handle cleanup that a single `Remove` does not, so the two return different errors in HA mode. ## P1: foreground latency - [ ] **Batch reads by stripe.** `BatchGetReplicaList` and `BatchExistKey` build one accessor per key: two hashes, the stripe lock, a handle refcount, the slot lock and a CAS. That is about six atomic RMWs per key, on cache lines all lookup threads share. Before #4557 a batch took each shard once. This is the likely cause of the 30000-key read p99 regression measured in #4844 (73–78 → 88–95 ms). An `ObjectRoute::ReadBatch` would group keys by stripe and lock each stripe once. - [ ] **Batch writes take global locks per key.** For each key, PutStart takes: - `client_mutex_`, two or three times; - `master_state_mutex_`, once; - the tenant registration mutex (one per shard of the quota map); - for a non-default tenant, the namespace table lock. These can move to the start of the batch. - [ ] **PutStart and UpsertStart build the set of live clients for every key.** It is only used for an existing key with a local-disk replica. - [ ] **`IsManagedGroup` locks the weight store for ungrouped keys.** `Remove`, `BatchRemove`, `UpsertStart` and NoF eviction call it without checking for an empty group id, under the key's write lock. - [ ] Smaller: - the key and tenant are copied twice per write (`ObjectIdentity` by value into the accessor); - `AdmitWithDfsRecovery` takes a `std::function` and allocates per put; - `BuildStaleHandleCleanupPlan` copies every replica descriptor when nothing is stale. ## P2: background work that competes with requests - [ ] **Keys the evictor always skips sit at the head of the index.** The lease index ranks by lease expiry and excludes only hard pins. So these keys are write-locked and judged again every round, every 10 ms under pressure: - soft-pinned keys, when soft-pinned objects may not be evicted; - managed groups; - keys with an offload in flight. Rank by when a key can actually be evicted, or refile a skipped key later. - [ ] **Expired in-flight work is swept with four lock passes.** `DiscardExpiredProcessingReplicas` runs from both memory and NoF eviction each tick. It copies the in-flight key list and write-locks each in-flight key four times: for processing, replication, offload and promotion. These are the keys PutEnd is writing. It should be one pass, one lock per key, ordered by deadline. - [ ] **Full scans on a timer.** - Drain scheduling scans every object every 500 ms per job, and does not stop once its slots are filled. - Client offboarding scans twice per retry. - NoF eviction scans everything every 10 ms while nothing is evictable. - Unmount in HA mode runs `ClearInvalidHandles` on the RPC thread. Each scan holds a stripe's shared lock across its loop body, which may stall new keys in that stripe (unmeasured). - [ ] **OpLog writer cost.** - The payload is copied four to five times, twice under the writer's global lock. - Each key takes that lock twice, to reserve and to commit. - Every finalize closure heap-allocates. - One condition variable wakes the writer, the callback thread and retries on every commit. ## P3: memory per object About 1,136 bytes in seven heap blocks for an ungrouped object with one memory replica and a 64-byte key (sizes from `sizeof`, chunks from glibc rounding): - [ ] Drop `ObjectMetadata::user_key` (a second copy of the key) and `ObjectMetadata::tenant_id` (the namespace knows it). - [ ] Keep an ungrouped object's lease inline instead of in a `shared_ptr`. - [ ] Make `AllocatedBuffer::protocol` an enum and move the CXL-only segment name out (−64 B per memory replica). - [ ] Keep the first part and first replica inline (−2 heap blocks), and size `Replica` for the memory case. - [ ] Measure a smaller per-key lock than the 56-byte `std::shared_mutex`. A hook that keeps a per-key index (the eviction index, the in-flight list, a soft-pin index on the slot) needs words or a list hook in `SlotOwnerState` on every key. Count that before adding one. ## P4: structure First: - [ ] **Metadata port out of `MasterService`.** `MetadataAccessorRW/RO` are nested friend classes that call back into `CleanupInvalidMemoryReplicas` and `EraseMetadata`, and the OpLog helpers are private to `MasterService`. So any component that writes a key has to go through `MasterService`. Key access, `ReadBatch`, cursors and hook registration should move into a port of their own. The port needs a new name, since `metadata_store.h` holds the standby structs. - [ ] **Teardown and accounting as hooks.** Each step of `EraseMetadata` and of replica removal moves to the component it belongs to, on `OnTearDown`, `OnReplace` or `OnWriteRelease`. That covers `CleanupInvalidMemoryReplicas`, `CancelPromotionTaskForRemovedReplicas`, the 26 `*WithCacheTotalAccounting` wrappers and the 9 `SyncCacheTotalAccounting` calls. Then each component takes its members and its thread out of `MasterService`, and its periodic loop becomes a `BackgroundWorker`: - [ ] **Evictor:** - what moves in: `MemoryEviction` (policy, reclaimers, offload queue, KV observer), `BatchEvict`, `NoFBatchEvict`, `RunDfsEviction`, the pressure flags and watermarks, and `PolicyStoreAccess`; - the eviction thread then only evicts. Today it also sweeps expired writes and retries promotions. - [ ] **Promotion:** the in-flight counter, the sketch, the settings, `TryPushPromotionQueue` and the retry loop, plus hooks that cancel a task when its source goes. - [ ] **Dynamic replication:** the five execution steps still in `MasterService`. The plan from #4557 (lease and cooldown as one value on `ObjectState`, stateless admission and planning, expiry through `ObjectRef` instead of a pending index) still applies. - [ ] **Drain scheduler:** `DrainJob`, `drain_jobs_`, its validation and scheduling functions, and `job_dispatch_thread_`. - [ ] **`ClientRegistry`:** `client_mutex_`, the liveness records, `ok_client_`, host ids and `client_monitor_thread_`. It tells subscribers when a client comes up or goes down. - [ ] **Client offboarding:** `ProcessClientOffboardingJob` moves into the worker, and the back-pointer goes. - [ ] **NoF heartbeat monitor and HTTP metadata cleaner.** The cleaner acts on `ClientRegistry`'s client-down notice. - [ ] **KV events:** tenant epochs, media snapshots and `PublishKv*`. - [ ] **Soft-pin deadline index:** one per namespace, kept by hooks on the key slot instead of a nested class of `MasterService` with two copies of each key. - [ ] **Weight management:** `MasterStoreBackend` stops reading `MasterService` members. - [ ] **HA:** the OpLog hook, the writer and serializers, and the standby restore state. - [ ] **Lifecycle gate:** components wait on it instead of taking `master_state_mutex_`. #4736 started this. From #4557: - [ ] **Eviction state on the key slot, and who owns a group.** - The route keeps the shared lease and a member count. - Eviction membership lives only in the index, as typed entries. - Weight import commit checks a key list against the manifest. - [ ] **Cleanups left by the cutover:** - fold `KvEventObserver` into the immediate reclaimer; - keep one quota policy handle instead of `policy_` and `tenant_quota_policy_`; - share the grow-and-allocate step of `UpsertStart` and `AllocateAndInsertMetadata`. - [ ] **Eviction cursor:** read a bucket's keys in batches, and name objects internally by slot instead of by key string. New: - [ ] **Remove dead and test-only code:** - `ValidateDrainRequest`, which has no callers; - `SerializeMetadataForOpLogWithoutMemReplicas`; - the public `SubmitReplicaActionProposal` and the key-only `BatchReplicaClear` overload, used only by tests; - the test hooks `kv_remove_all_tenant_hook_` and `nof_probe_fn_`. - [ ] **Share duplicated code:** `CopyEnd`/`MoveEnd`, `CopyStart`/`MoveStart`, and the three `SerializeMetadataForOpLog*`. Open: - Do the request flows (Put, Get, Remove, Copy, Move) become an object read/write component, or stay as the one piece of orchestration `MasterService` keeps? - Once components stand alone, each gets its own unit tests instead of going through `MasterServiceTestPeer`.
Author
Owner

@wjinxu commented on GitHub (Oct 26, 2026):

Hi, I'd like to work on “DFS bucket eviction skips the managed-group check.”

<!-- gh-comment-id:6417127561 --> @wjinxu commented on GitHub (Oct 26, 2026): Hi, I'd like to work on “DFS bucket eviction skips the managed-group check.”
Author
Owner

@github-actions[bot] commented on GitHub (Oct 26, 2026):

Thanks for opening this issue, @Aionw!

Field Value
Issue #4955
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.

<!-- gh-comment-id:6419079530 --> @github-actions[bot] commented on GitHub (Oct 26, 2026): Thanks for opening this issue, @Aionw! | Field | Value | |-------|-------| | **Issue** | #4955 | | **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.
Author
Owner

@sssqqeer commented on GitHub (Oct 26, 2026):

I'd like to work on "PutStart and UpsertStart build the set of live clients for every key" and "Expired in-flight work is swept with four lock passes" (one pass, one lock per key, ordered by deadline).

<!-- gh-comment-id:6423404956 --> @sssqqeer commented on GitHub (Oct 26, 2026): I'd like to work on "PutStart and UpsertStart build the set of live clients for every key" and "Expired in-flight work is swept with four lock passes" (one pass, one lock per key, ordered by deadline).
Author
Owner

@Colors-111 commented on GitHub (Oct 27, 2026):

Hi, I'd like to work on "Drop ObjectMetadata::user_key (a second copy of the key) and ObjectMetadata::tenant_id" and "Keep an ungrouped object's lease inline instead of in a shared_ptr"

<!-- gh-comment-id:6432512511 --> @Colors-111 commented on GitHub (Oct 27, 2026): Hi, I'd like to work on "Drop ObjectMetadata::user_key (a second copy of the key) and ObjectMetadata::tenant_id" and "Keep an ungrouped object's lease inline instead of in a shared_ptr"
Author
Owner

@kunkunblueberry commented on GitHub (Oct 27, 2026):

I'd like to take ownership of the OpLog route-hook work, including defining the write-path contract and migrating the existing write paths to ensure consistent failure handling and HA correctness.

<!-- gh-comment-id:6436968583 --> @kunkunblueberry commented on GitHub (Oct 27, 2026): I'd like to take ownership of the OpLog route-hook work, including defining the write-path contract and migrating the existing write paths to ensure consistent failure handling and HA correctness.
Author
Owner

@Colors-111 commented on GitHub (Oct 27, 2026):

7. Hi, I'd like to work on "Drop ObjectMetadata::user_key (a second copy of the key) and ObjectMetadata::tenant_id" and "Keep an ungrouped object's lease inline instead of in a shared_ptr"

I'd also like to take the rest of the P3 items:

  • Make AllocatedBuffer::protocol an enum and move the CXL-only segment name out (−64 B per memory replica).
  • Keep the first part and first replica inline (−2 heap blocks), and size Replica for the memory case.

I'll send these as separate PRs, each with before/after numbers like the two items above.

<!-- gh-comment-id:6437847314 --> @Colors-111 commented on GitHub (Oct 27, 2026): > 7\. Hi, I'd like to work on "Drop ObjectMetadata::user_key (a second copy of the key) and ObjectMetadata::tenant_id" and "Keep an ungrouped object's lease inline instead of in a shared_ptr" I'd also like to take the rest of the P3 items: - Make `AllocatedBuffer::protocol` an enum and move the CXL-only segment name out (−64 B per memory replica). - Keep the first part and first replica inline (−2 heap blocks), and size `Replica` for the memory case. I'll send these as separate PRs, each with before/after numbers like the two items above.
Author
Owner

@sssqqeer commented on GitHub (Oct 27, 2026):

Hi, I'd like to take the rest of P1 as well (I claimed "PutStart and UpsertStart build the set of live clients for every key" above):

  • "Batch writes take global locks per key"
  • "IsManagedGroup locks the weight store for ungrouped keys"
  • the three "Smaller" items

For "Batch reads by stripe", I tried an ObjectRoute::ReadBatch version and benchmarked it with the same setup as #4844. It's ~25% faster at the route level and BatchExistKey is ~12% faster, but BatchGetReplicaList over RPC actually got 8-9% slower (p99 85-92ms -> 92-97ms), so it didn't bring the p99 back in my test. I'd like to look into why the RPC path got slower before opening a PR for it.

<!-- gh-comment-id:6441823405 --> @sssqqeer commented on GitHub (Oct 27, 2026): Hi, I'd like to take the rest of P1 as well (I claimed "PutStart and UpsertStart build the set of live clients for every key" above): - "Batch writes take global locks per key" - "`IsManagedGroup` locks the weight store for ungrouped keys" - the three "Smaller" items For "Batch reads by stripe", I tried an `ObjectRoute::ReadBatch` version and benchmarked it with the same setup as #4844. It's ~25% faster at the route level and BatchExistKey is ~12% faster, but BatchGetReplicaList over RPC actually got 8-9% slower (p99 85-92ms -> 92-97ms), so it didn't bring the p99 back in my test. I'd like to look into why the RPC path got slower before opening a PR for it.
Author
Owner

@Colors-111 commented on GitHub (Oct 28, 2026):

Hi @sssqqeer — #5013 covers the drain scheduling sub-item of P2 "Full scans on a timer", thanks. Do you plan to pick up the remaining sub-items of that checkbox (client offboarding scanning twice per retry, NoF eviction's 10 ms full scan while nothing is evictable, and unmount running ClearInvalidHandles on the RPC thread in HA mode)?

If they are free, I'd like to take two of them:

"client offboarding scans twice per retry"
"unmount in HA mode runs ClearInvalidHandles on the RPC thread"

<!-- gh-comment-id:6465234029 --> @Colors-111 commented on GitHub (Oct 28, 2026): Hi @sssqqeer — #5013 covers the drain scheduling sub-item of P2 "Full scans on a timer", thanks. Do you plan to pick up the remaining sub-items of that checkbox (client offboarding scanning twice per retry, NoF eviction's 10 ms full scan while nothing is evictable, and unmount running ClearInvalidHandles on the RPC thread in HA mode)? If they are free, I'd like to take two of them: "client offboarding scans twice per retry" "unmount in HA mode runs ClearInvalidHandles on the RPC thread"
Author
Owner

@sssqqeer commented on GitHub (Oct 28, 2026):

Hi @Colors-111, sure, go ahead with those two. #5013 only touches the drain scheduler, so they shouldn't overlap.

<!-- gh-comment-id:6468340129 --> @sssqqeer commented on GitHub (Oct 28, 2026): Hi @Colors-111, sure, go ahead with those two. #5013 only touches the drain scheduler, so they shouldn't overlap.
Author
Owner

@losewayy commented on GitHub (Oct 28, 2026):

I'd like to work on the "Standby restore repair batch" item.

<!-- gh-comment-id:6473200713 --> @losewayy commented on GitHub (Oct 28, 2026): I'd like to work on the "Standby restore repair batch" item.
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#1258
No description provided.