mirror of
https://github.com/kvcache-ai/Mooncake.git
synced 2026-11-05 01:17:55 +00:00
[GH-ISSUE #4955] [Track]: [Store] MasterService follow-ups after the tenant-first migration #1258
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#1258
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 @Aionw on GitHub (Oct 26, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4955
#4557 moved
MasterServiceonto tenant namespaces, per-key locks, theEvictorandTenantQuotaPolicy. This issue tracks what is left after it, in priority order. A PR that takes an item links back here.Findings come from reading
mainatb7a50b39c, rechecked againstmainafter #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
MasterServiceis 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.Target
Each component owns its loop: its state, threads and invariants.
MasterServiceonly 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.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.
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:ClientOffboardingWorkerholds aMasterService*, and its jobs run inMasterService::ProcessClientOffboardingJob.MasterStoreBackendis a friend that readsenable_oplog_andordered_oplog_writer_.PromotionCandidateTrackerholds only the retry index.The clearest case is
EraseMetadata. Every teardown through an accessor runs, in an orderMasterServicehard-codes, cleanup for:A component is closed when:
MasterServicekeeps no member for it.MasterServiceand is not its friend. It is given the ports it uses when it is built.Components use them as follows:
ClientRegistry's client-down notice;What stays in
MasterService:P0: correctness of HA writes
Most of these come from
MasterServicewriting 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.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:OnWriteBeginruns under the key lock before the first change and may fail. It validates the entry and reserves a slot.OnWriteReleaseruns 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.ImmediateReclaimerandOpLogReclaimer. Barrier users (restore, weight ops) exist only under HA and keep calling the writer.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 (AdmitWriteruns before the lock, andOnGrowonly runs on growth);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.BatchReplicaClear,PutRevoke,EvictDiskReplicaImpl,MoveEndandNoFBatchEvictmark replicas removed and thencontinueor return on failure. The replica stays REMOVED: its memory and quota are never released (MemoryChargecounts 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.PutRevokecannot simply callcancel_remove(), which restores COMPLETE rather than PROCESSING.OpLogReclaimerreports 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.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.AwaitDurablealready returns an error in that case. #4736 added the restore state machine and did not change this.IsManagedGroupcheck, unlike shard DFS, NoF and drain, so a managed weight may be evicted from a DFS bucket. One shared predicate fixes it.EvictDiskReplicaImpltakes a key lock withoutmaster_state_mutex_, against the lock order.BatchRemoveruns stale-handle cleanup that a singleRemovedoes not, so the two return different errors in HA mode.P1: foreground latency
Batch reads by stripe.
BatchGetReplicaListandBatchExistKeybuild 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). AnObjectRoute::ReadBatchwould 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;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.
IsManagedGrouplocks the weight store for ungrouped keys.Remove,BatchRemove,UpsertStartand NoF eviction call it without checking for an empty group id, under the key's write lock.Smaller:
ObjectIdentityby value into the accessor);AdmitWithDfsRecoverytakes astd::functionand allocates per put;BuildStaleHandleCleanupPlancopies 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:
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.
DiscardExpiredProcessingReplicasruns 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.
ClearInvalidHandleson 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.
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):ObjectMetadata::user_key(a second copy of the key) andObjectMetadata::tenant_id(the namespace knows it).shared_ptr.AllocatedBuffer::protocolan enum and move the CXL-only segment name out (−64 B per memory replica).Replicafor the memory case.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
SlotOwnerStateon every key. Count that before adding one.P4: structure
First:
MasterService.MetadataAccessorRW/ROare nested friend classes that call back intoCleanupInvalidMemoryReplicasandEraseMetadata, and the OpLog helpers are private toMasterService. So any component that writes a key has to go throughMasterService. Key access,ReadBatch, cursors and hook registration should move into a port of their own. The port needs a new name, sincemetadata_store.hholds the standby structs.EraseMetadataand of replica removal moves to the component it belongs to, onOnTearDown,OnReplaceorOnWriteRelease. That coversCleanupInvalidMemoryReplicas,CancelPromotionTaskForRemovedReplicas, the 26*WithCacheTotalAccountingwrappers and the 9SyncCacheTotalAccountingcalls.Then each component takes its members and its thread out of
MasterService, and its periodic loop becomes aBackgroundWorker:MemoryEviction(policy, reclaimers, offload queue, KV observer),BatchEvict,NoFBatchEvict,RunDfsEviction, the pressure flags and watermarks, andPolicyStoreAccess;TryPushPromotionQueueand the retry loop, plus hooks that cancel a task when its source goes.MasterService. The plan from #4557 (lease and cooldown as one value onObjectState, stateless admission and planning, expiry throughObjectRefinstead of a pending index) still applies.DrainJob,drain_jobs_, its validation and scheduling functions, andjob_dispatch_thread_.ClientRegistry:client_mutex_, the liveness records,ok_client_, host ids andclient_monitor_thread_. It tells subscribers when a client comes up or goes down.ProcessClientOffboardingJobmoves into the worker, and the back-pointer goes.ClientRegistry's client-down notice.PublishKv*.MasterServicewith two copies of each key.MasterStoreBackendstops readingMasterServicemembers.master_state_mutex_. #4736 started this.From #4557:
KvEventObserverinto the immediate reclaimer;policy_andtenant_quota_policy_;UpsertStartandAllocateAndInsertMetadata.New:
ValidateDrainRequest, which has no callers;SerializeMetadataForOpLogWithoutMemReplicas;SubmitReplicaActionProposaland the key-onlyBatchReplicaClearoverload, used only by tests;kv_remove_all_tenant_hook_andnof_probe_fn_.CopyEnd/MoveEnd,CopyStart/MoveStart, and the threeSerializeMetadataForOpLog*.Open:
MasterServicekeeps?MasterServiceTestPeer.@wjinxu commented on GitHub (Oct 26, 2026):
Hi, I'd like to work on “DFS bucket eviction skips the managed-group check.”
@github-actions[bot] commented on GitHub (Oct 26, 2026):
Thanks for opening this issue, @Aionw!
41376987A maintainer will triage this when possible. To help us respond faster, please include:
Useful links: Documentation · Contributing guide
@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).
@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"
@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.
@Colors-111 commented on GitHub (Oct 27, 2026):
I'd also like to take the rest of the P3 items:
AllocatedBuffer::protocolan enum and move the CXL-only segment name out (−64 B per memory replica).Replicafor the memory case.I'll send these as separate PRs, each with before/after numbers like the two items above.
@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):
IsManagedGrouplocks the weight store for ungrouped keys"For "Batch reads by stripe", I tried an
ObjectRoute::ReadBatchversion 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.@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"
@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.
@losewayy commented on GitHub (Oct 28, 2026):
I'd like to work on the "Standby restore repair batch" item.