Repository navigation
Conversation
CAICAIIs
force-pushed
the
master_service
branch
2 times, most recently
from
September 28, 2026 22:32
92eae85 to
28d1d6b
Compare
8 of 9 tasks
CAICAIIs
added a commit
to CAICAIIs/Mooncake
that referenced
this pull request
Oct 2, 2026
A tenant's object route held one map under one shared_mutex: writes to distinct keys serialized on it, and a map that outgrew its buckets rehashed every node it held while that lock was held, a walk proportional to the nodes already in the map. Each stripe is a shared_mutex and a map, and a key's stripe is a function of the key alone, so a per-key operation still takes exactly one lock and one key's growth is charged to its stripe's keys. The aggregates are counted stripe by stripe, which the comments record as the weaker meaning they now have, and WithExclusiveRoute takes the key whose stripe it holds. The count belongs to this index: kStripeCount = 64 is the default and the constructor takes a count, so benchmarks/object_index_contention_bench.cpp and the tests can build the same route at another count. The group directory keeps StripedGroupIndex<64> and the count its own measurement chose; nothing needs the two counts to agree. This is the part of kvcache-ai#4370 that stripes the route, taken out of that PR.
9 of 13 tasks
CAICAIIs
added a commit
to CAICAIIs/Mooncake
that referenced
this pull request
Oct 2, 2026
A tenant's object route held one map under one shared_mutex: writes to distinct keys serialized on it, and a map that outgrew its buckets rehashed every node it held while that lock was held, a walk proportional to the nodes already in the map. Each stripe is a shared_mutex and a map, and a key's stripe is a function of the key alone, so a per-key operation still takes exactly one lock and one key's growth is charged to its stripe's keys. The aggregates are counted stripe by stripe, which the comments record as the weaker meaning they now have, and WithExclusiveRoute takes the key whose stripe it holds. The count belongs to this index: kStripeCount = 64 is the default and the constructor takes a count, so benchmarks/object_index_contention_bench.cpp and the tests can build the same route at another count. The group directory keeps StripedGroupIndex<64> and the count its own measurement chose; nothing needs the two counts to agree. This is the part of kvcache-ai#4370 that stripes the route, taken out of that PR.
CAICAIIs
added a commit
to CAICAIIs/Mooncake
that referenced
this pull request
Oct 2, 2026
A tenant's object route held one map under one shared_mutex: writes to distinct keys serialized on it, and a map that outgrew its buckets rehashed every node it held while that lock was held, a walk proportional to the nodes already in the map. Each stripe is a shared_mutex and a map, and a key's stripe is a function of the key alone, so a per-key operation still takes exactly one lock and one key's growth is charged to its stripe's keys. The aggregates are counted stripe by stripe, which the comments record as the weaker meaning they now have, and WithExclusiveRoute takes the key whose stripe it holds. The count belongs to this index: kStripeCount = 64 is the default and the constructor takes a count, so benchmarks/object_index_contention_bench.cpp and the tests can build the same route at another count. The group directory keeps StripedGroupIndex<64> and the count its own measurement chose; nothing needs the two counts to agree. This is the part of kvcache-ai#4370 that stripes the route, taken out of that PR.
CAICAIIs
force-pushed
the
master_service
branch
3 times, most recently
from
October 5, 2026 22:19
c85e2e9 to
0cd8347
Compare
…gistry MasterService keeps one metadata::TenantRegistry in place of the 1024 metadata shards. A tenant's objects live on that tenant's own route, its groups on the tenant's group table, and its quota account is bound by the registry's factory, so an object is excluded only while it is itself being read, written or evicted and two unrelated objects never contend. Every object-level path resolves its tenant through the registry, resolves the entry the tenant's route publishes, and runs its read-modify-write under that entry's own lock: put, upsert, remove, remove-all, the point and batch reads, eviction, offload, promotion, dynamic replication, client offboarding, the durable finalizers and the stale-handle sweeps. EraseMetadata is the single teardown point and claims the publication through the entry's own state, so a second eraser of the same object bails out instead of releasing its refcounts, quota charges and KV events twice. The bucket allocator's eviction keeps the serialization the shard locks gave it: the background pass takes the same mutex the write path holds across the admission retry and the recovery that retry decides on, so a pass and a recovery never evict at the same time and a caller that waited behind a pass re-admits before it evicts a bucket of its own. The per-tenant replica-action records, the client leases and the promotion-candidate key index stay in the service keyed by tenant id: each belongs to the subsystem that keeps it, and that subsystem validates what it holds against the entry the route publishes. Per-object task state lives on the entry, so the parallel per-key maps have no successor. The snapshot payload keeps its layout, one zstd-compressed entry per tenant holding objects with [tenant_id, key, metadata] items, and a reader still accepts the legacy two-element item. The old shard containers, the four accessor classes and the group domain are gone. The metadata layer itself is untouched: metadata/tenant.h, metadata/tenant_registry.h, object_entry.h, object_index.h, group_index.h and dynamic_replication_lease_table.h keep the API this migration consumes. Four tests that named shard behaviour now name what they cover: grouped routing that does not follow group membership, the invalid-handle sweep over unmounted segments, the promotion queue limit across tenants, and a retry round that queues nothing. Three tests go with the container, because re-routing restored objects between shards and the shrink of the sparse per-shard maps on eviction and on the stale-handle sweep no longer describe anything; the peer helpers those tests used go with them, along with two helpers that had no caller.
CAICAIIs
force-pushed
the
master_service
branch
from
October 7, 2026 17:10
0cd8347 to
a182721
Compare
A tenant's route was one map under one lock, so the route's own growth was paid by every reader of that tenant: growing an unordered_map walks each node it holds under the exclusive lock, and that walk is cache-bound. A probe of the route with 50k objects measures a single growth event at 9.4 ms with one reader and 28 ms at 200k objects, and every read of the tenant waits behind it. That is the tail the synthetic workload shows as a p999 several times the p99, and it is also why a reload, an eviction pass or a snapshot had to wait for one lock that every key of the tenant shares. The route is striped into 64 maps, each under its own lock, and a key always lands in the same stripe, so every per-key operation still takes exactly one lock and sees the same map. Growth is charged to one stripe's keys: the same probe measures 0.11 ms for the largest growth event at 50k objects, and readers of different keys no longer share a lock. Tenant::RemoveObject names the key whose stripe it needs, which is the key it already resolves; a teardown of one key cannot affect another key's stripe. ObjectCount, Empty and SnapshotObjects walk the stripes one at a time; the snapshot keeps its contract of handing out the handles to resolve again, and it counts the stripes before it copies them, so the handles land in one allocation instead of a reallocation and a copy of the whole vector per stripe. The count came from sweeping the route at 1, 4, 16, 64 and 256 stripes, at 1, 4, 16 and 32 writer threads: 256 stripes reach 114M lookups and 9.1M publishes per second against 79M and 6.7M at 64, about 1.4 times the lookups and three times the mixed rate for four times the memory, 30.7 kB against 7.7 kB per tenant, so 64 is where a further split stops paying for itself.
CAICAIIs
force-pushed
the
master_service
branch
from
October 8, 2026 02:27
a182721 to
e810d8e
Compare
CAICAIIs
marked this pull request as ready for review
October 8, 2026 02:27
CAICAIIs
requested review from
00fish0,
Aionw,
Icedcoco,
Libotry,
XucSh,
YiXR,
stmatengss and
ykwd
as code owners
October 8, 2026 02:27
…ight work per tenant Move dynamic replication, promotion-candidate retry and the tenant quota control plane out of MasterService into DynamicReplicationController, PromotionCandidateTracker and TenantQuotaManager. Each owns its own index and locks; per-object state stays on ObjectEntry::State under the entry lock. Restore the MetadataAccessorRW/RO call shape on per-entry scoped holds, keeping master_service.cpp close to main. Keep a per-tenant intrusive list of entries with work in flight, so the expiry sweep walks those instead of every object.
Walk each tenant's route with a cursor that visits objects under their own lock without copying handles, and answer replica availability in place instead of building a descriptor or taking std::atomic_load's global lock.
- ReadCursor/WriteCursor replace the object walk; SnapshotObjects is gone. - Entry holds come only from Tenant::ReadHold/WriteHold, which confirm under the lock that the route still publishes the entry; WithPublishedObject is gone. - TearDownObject and EraseMetadata take the write hold, and the in-flight list is settled as a write hold is released, so no caller tracks work by hand. - ObjectEntry's raw lock access is private: WithUnpublished covers entries not yet published, and InsertObject runs a callback under the hold that publishes.
Tenant keeps only the route, the groups and the in-flight list. Charges, reservations and the ledger rebuild move to TenantQuotaBinding, which the tenant registry hands out with the tenant as a TenantHandle, so one lookup yields both.
This was referenced Oct 10, 2026
This was referenced Oct 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
MasterServicehalf of the tenant-first metadata model (RFC #3670), on top of the layer that is already inmain: per-object metadata and the object route from #4127 and #4161, the tenant directory, catalog and registry from #4215.MasterServicekeeps onemetadata::TenantRegistryin place of the 1024 metadata shards. A tenant's objects live on that tenant's own route, its groups on the tenant's group table, and its quota account is bound by the registry's factory, so an object is excluded only while it is itself being read, written or evicted, and two unrelated objects never contend.This PR carries the model swap and the route striping that keeps the object path's lock granularity, and nothing else: the correctness and cost work the migration made reachable is listed under Follow-ups.
What the migration does
EraseMetadatais the single teardown point and claims the publication through the entry's own state, so a second eraser of the same object bails out instead of releasing its refcounts, quota charges and KV events twice.[tenant_id, key, metadata]items, and a reader still accepts the legacy two-element item. Decoding replaces what the routes held, so a key the payload does not carry cannot survive from the state the decode started in. The weight metadata objectmainadded to the payload is written and validated here too.metadata/tenant.h,metadata/tenant_registry.h,object_entry.h,object_index.h,group_index.handdynamic_replication_lease_table.hkeep the API this migration consumes, and the featuresmaingained since the branch point (managed weight revisions,ProbeKey/BatchProbeKey, the DFS immutable bucket allocator) are integrated on the tenant model rather than dropped.Module
mooncake-store)Type of Change
How Has This Been Tested?
Test commands:
Test results:
Unit tests pass
Integration tests pass (if applicable)
Manual testing done (describe below)
Build: the tree builds with no errors; the 18 test targets that carry these paths link and run.
Suites: 349 cases pass across
master_service_test(65),master_service_tenant_quota_test(35),master_service_group_test(12),promotion_on_hit_test(55),master_service_evict_scenario_test(22),master_service_concurrent_scenario_test(10),master_service_group_scenario_test(13),master_service_tenant_scenario_test(10),master_service_offload_scenario_test(25),master_service_scenario_test(32),offload_on_evict_test(1),dynamic_replication_test(27),master_service_processing_key_double_erase_test(1),master_service_weight_management_test(12),metadata_tenant_test(12),object_index_test(6),group_index_test(7) andeviction_strategy_test(4). The three op-log cases ofmaster_service_evict_scenario_testtalk to an external etcd and flake against a shared instance: each of them passes on its own, and the same flake reproduces onmain.Formatting:
./scripts/code_format.sh --changed-lines --checkreports no violation with clang-format 20.1.8, andpre-commit run --filespasses over the changed files.ThreadSanitizer:
master_service_test(65),master_service_tenant_quota_test(35),promotion_on_hit_test(55),dynamic_replication_test(27),master_service_group_test(12) andmaster_service_evict_scenario_test(22) pass under-DENABLE_TSAN=ONwith no sanitizer report and no lock-order inversion.Performance. Synthetic master workload: 8 segments of 400 GiB that stay far below the eviction watermark, and the same 8 segments of 4 GiB held at a 0.5 watermark so the background evictor works throughout; 448 KiB values,
group_size=5, fixed arrival,replica_num=1, preferred placement, 60 s per run, Release builds. The two versions run interleaved inside each round with the order flipped every round, six rounds per version and rate. Rates per segment: exist 6.3359, put 3.1431, get 2.0242 at the default, doubled at 2×.mainmainmainmainmain, so read it as neither a win nor a regression. The 8 × 4 GiB runs take the eviction path throughout, 8 passes per run at the default rate and 33–34 at 2×.completed_tasks=scheduled_tasks(5512 and 11032), identicalcommitted_keysandretained_keys, no overload events, and refused puts within 0.6% in the 8 × 4 GiB runs (432 on both sides at the default rate, 1392 against 1400 at 2×).Checklist
./scripts/code_format.shAI Assistance Disclosure
AI assistance was used for parts of this migration and to draft this description. The submitter has reviewed every changed line and can defend the change end to end.
Follow-ups
Work the migration made reachable, left out of this PR. Each item needs its own measurement or test before it is worth doing, and none of them changes what this PR guarantees.
Correctness on paths the migration rewrote
FinalizeRemovedReplicasAfterDurableresolves the object by key and then releases quota, emits the removal delta and tears the object down on whatever that key publishes when it runs, so a same-key recreate that landed while the cleanup was in flight can be charged for it. It should act only on the publication the cleanup was armed with, asFinalizeMetadataEraseAfterDurableandFinalizeExpiredProcessingReplicasAfterDurablealready do.EvictGroupOrObjectcollects the members it invalidated and erases them in a second pass, so a replica another path adds to one of those entries in between goes down with it. The teardown belongs inside the member's own callback, which already holds that entry's lock.PutStartresolves through get-or-create before it allocates, so a rejected or failed request leaves an empty tenant behind. It should resolve the tenant instead and let the tenant be created when the object is about to be published.Cost
BatchGetReplicaListkeeps the shared lock across the promotion and dynamic-replication candidate queues, so a reload waits for the whole batch; the lock only has to cover the metadata reads, as the single-key read path already does.BatchEvictholds every tenant's route as one vector of handles for the whole census, and the tenant watermark pass does the same for one tenant; the second pass ofBatchEvictmaterialises every candidate identity. A walk that takes one route stripe at a time and a bounded chunk of it holds a chunk instead. The deadline vectors the census accumulates (no_pin, soft pins, candidates) still grow with the population and need their own decision.replica_action_mutex_into per-tenant state and locks. The one lock over the action records serializes promotion and dynamic-replication action paths across unrelated tenants. Judge a split on the p50, p99 and p999 of those paths, the wait on that lock and their throughput, with promotion and dynamic replication on and off, and with two tenants against many.snapshot_mutex_.BatchEvictandNoFBatchEvicttake it shared for the whole census and scan, so a reload waits for a pass. Anything that replaces that has to keep what the lock buys today: no tenant is removed under a running scan.ObjectIndex::SnapshotObjects(). It counts the stripes before it copies them, which is what keeps the handles in one allocation; one pass would trade that back for repeated growth. Worth changing only with a measurement that shows the count pass costs more.