Repository navigation
Conversation
CAICAIIs
force-pushed
the
metadata-layout
branch
2 times, most recently
from
October 2, 2026 18:33
5961f75 to
08a9c63
Compare
11 of 13 tasks
CAICAIIs
force-pushed
the
metadata-layout
branch
from
October 2, 2026 18:40
08a9c63 to
4d4d888
Compare
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
metadata-layout
branch
from
October 2, 2026 19:05
4d4d888 to
72e0331
Compare
CAICAIIs
added a commit
to CAICAIIs/Mooncake
that referenced
this pull request
Oct 5, 2026
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 stripe count came from the sweep in kvcache-ai#4455, which carries the probe and the benchmark.
CAICAIIs
added a commit
to CAICAIIs/Mooncake
that referenced
this pull request
Oct 7, 2026
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 stripe count came from the sweep in kvcache-ai#4455, which carries the probe and the benchmark.
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
A tenant's object route (
ObjectIndex) held one map under oneshared_mutex,so every key of a tenant serialized on it: writes to distinct keys queued behind
each other, 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.
The route is now striped. Each stripe is a
shared_mutexand a map, and a key'sstripe is a function of the key alone (
TransparentStringHashmodulo the stripecount), so a per-key operation still takes exactly one lock, and one key's
growth is charged to its stripe's keys rather than to every key the tenant
holds. Where the route was one lock, the aggregates now necessarily mean
something weaker, and the comments say so:
ObjectCountandEmptyare countedstripe by stripe, so they describe the route at no one instant, and
SnapshotObjectsis a collection of strong handles to re-resolve rather thanthe route at one instant.
WithExclusiveRoutetakes the key, because thesection it holds is now the key's stripe.
The stripe count belongs to this index:
ObjectIndex::kStripeCount = 64is thedefault, and the constructor takes a count so the benchmark and the tests can
build the same route at another count. The group directory is untouched.
GroupIndexkeepsStripedGroupIndex<64>with the count its own contentionmeasurement chose, and the two indexes keep their own counts. They are separate
structures over separate key spaces -- object keys, and the group ids a client
passes in
ReplicateConfig::group_ids-- so nothing needs the two counts toagree, and the lock order in
metadata/tenant.h(entry lock, route stripe, thenthe group table) is ordered by kind and holds at any count. This is the part of
#4370 that stripes the route, taken out so it can be reviewed on its own; #4370
drops its own copy of
object_index.honce this lands.Related RFC: #3670
The count of 64 is measured
benchmarks/object_index_contention_bench.cppsweeps 1, 4, 16, 64 and 256stripes at 1, 4, 16 and 32 threads in three modes, and separately reports the
worst single publish into a growing route, which is the rehash walk that a
stripe bounds. Two runs on an idle 256-core AMD EPYC 7742 host, Release build,
32 writer threads, keys spread so that no two threads write one key:
One stripe is the route as it shipped, and 64 stripes take it from 8.5M lookups
and 250k publishes per second to 79M and 6.7M, while the worst single publish
drops from 20 to 30 ms to under 2.6 ms. 256 stripes come out higher again on all
three modes, by about 1.4x on lookups and publishes and three times on the mixed
one, at four times the memory: a stripe is a
shared_mutexand a map, about 120bytes, so 64 stripes are about 7.7 kB per tenant against 30.7 kB at 256. At 16
threads the two are level on lookups (60.0M and 45.6M against 60.6M and 58.3M),
while publishes and the mixed mode still gain.
What the sweep settles is a ratio rather than a count. 256 stripes over 131,072
keys is one stripe per 512 keys, and that was still ahead of the 2,048 keys per
stripe that 64 stripes give, so the useful ratio is no coarser than a few
hundred keys per stripe. 64 stripes are that ratio for a tenant of about thirty
thousand objects; a tenant holding fewer keys cannot use more stripes than it
has, and one holding far more keys would want more stripes than this constant
gives. The count therefore follows the tenant's size, and this constant stands
in for one size. The constructor takes a count, which the measurement and the
tests use, and nothing reads it from configuration.
Module
mooncake-store)Type of Change
How Has This Been Tested?
Built and run on Ubuntu 22.04.5 with GCC 11.4.0, CMake 3.31.0 and Ninja, in a
Debug configuration for the tests (
CMAKE_BUILD_TYPE=Debug,WITH_STORE=ON,WITH_STORE_RUST=ON,STORE_USE_ETCD=ON,WITH_CONDUCTOR=OFF) and a Releaseconfiguration for the benchmark:
Test results:
The benchmark is built in Release and run twice; the numbers in the description
come from those two runs:
Tests added to
mooncake-store/tests/object_index_test.cpp:EveryStripeCountRoutesTheSameWay-- insert, get, erase-if-expected,IsCurrent,Contains,ObjectCount,EmptyandStripeCountat 1, 4, 16and 64 stripes.
AKeyLandsInOneStripeHoweverOftenItIsReached-- a key is inserted once, thenlooked up repeatedly and through
SnapshotObjects; every accessor must reachthe stripe the insert placed it in.
SnapshotObjectsHoldsTheHandlesItCollected-- the handles a snapshot handsback are strong: the route slot can go while what the snapshot named is still
readable.
SnapshotObjectsOverlappingMutationNamesEachKeyOnce-- collections takenwhile a mutator keeps replacing keys name each key at most once, and the route
still holds every key afterwards.
ConcurrentInsertOfDistinctKeysPublishesEveryOne-- four threads insertdistinct keys at each stripe count; every insert publishes and every key
resolves to the entry it was inserted with.
No test asserts byte sizes or resident memory.
The benchmark above is the manual testing: it is what the default count is
chosen from, and it is the only place the byte sizes are reported.
Checklist
./scripts/code_format.shOn pre-commit: every hook that applies to this change passes --
clang-format20.1.8 throughscripts/code_format.sh(no diff on the four C++files), trailing whitespace, end of file, merge conflicts, large files and
codespell.
cmake-formatrewrites pre-existing comments and continuationindentation elsewhere in
mooncake-store/benchmarks/CMakeLists.txt; the fourlines this PR adds there are already in its form, and the unrelated rewrites are
left out of this change.
AI Assistance Disclosure
An AI coding assistant drafted the striping, the tests, the benchmark and this
description. The build, test and benchmark commands above were run against this
branch before it was opened. The human submitter is responsible for having
reviewed every changed line and can defend the change end-to-end.