[Store] Implement tenant metadata map isolation#2232
Conversation
Lin-z-w
requested review from
00fish0,
Libotry,
XucSh,
YiXR,
stmatengss and
ykwd
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces multi-tenancy support to the MasterService by scoping metadata, processing keys, and tasks under a new TenantState structure within each metadata shard. Key APIs such as PutStart, PutEnd, GetReplicaList, and Remove have been updated to accept an optional tenant_id parameter, defaulting to "default". The review feedback highlights a performance optimization opportunity in the serialization logic, suggesting the use of a custom struct with direct pointers to avoid redundant hash map lookups during key sorting.
| std::vector<std::pair<std::string, std::string>> sorted_keys; | ||
| sorted_keys.reserve(metadata_count); | ||
| for (const auto& [tenant_id, tenant_state] : shard.tenants) { | ||
| for (const auto& [key, metadata] : tenant_state.metadata) { | ||
| sorted_keys.emplace_back(tenant_id, key); | ||
| } | ||
| } | ||
| std::sort(sorted_keys.begin(), sorted_keys.end()); | ||
|
|
||
| for (const auto& key : sorted_keys) { | ||
| const auto& metadata = shard.metadata.at(key); | ||
| // Each metadata item format: [key, metadata_object] | ||
| packer.pack_array(2); | ||
| for (const auto& [tenant_id, key] : sorted_keys) { | ||
| const auto& tenant_state = shard.tenants.at(tenant_id); | ||
| const auto& metadata = tenant_state.metadata.at(key); | ||
| // Each metadata item format: [tenant_id, key, metadata_object]. | ||
| packer.pack_array(3); | ||
| packer.pack(tenant_id); | ||
| packer.pack(key); | ||
|
|
||
| auto result = SerializeMetadata(metadata, packer); |
There was a problem hiding this comment.
During serialization, sorting the keys and then looking up each tenant_state and metadata using .at() introduces significant overhead because it performs two hash map lookups (shard.tenants.at and tenant_state.metadata.at) for every single metadata item.
We can optimize this by storing pointers to the ObjectMetadata objects directly in the sorted collection, sorting them with a custom comparator, and then accessing the metadata directly without any map lookups.
struct SortedEntry {
std::string tenant_id;
std::string key;
const ObjectMetadata* metadata;
};
std::vector<SortedEntry> sorted_entries;
sorted_entries.reserve(metadata_count);
for (const auto& [tenant_id, tenant_state] : shard.tenants) {
for (const auto& [key, metadata] : tenant_state.metadata) {
sorted_entries.push_back({tenant_id, key, &metadata});
}
}
std::sort(sorted_entries.begin(), sorted_entries.end(),
[](const SortedEntry& a, const SortedEntry& b) {
return a.tenant_id != b.tenant_id ? a.tenant_id < b.tenant_id
: a.key < b.key;
});
for (const auto& entry : sorted_entries) {
// Each metadata item format: [tenant_id, key, metadata_object].
packer.pack_array(3);
packer.pack(entry.tenant_id);
packer.pack(entry.key);
auto result = SerializeMetadata(*entry.metadata, packer);
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
One small compatibility note with the grouped-lifecycle work in #2127/#2180: this PR moves object metadata into tenant-scoped Otherwise tenants using the same user key or group_id could accidentally share routing or lifecycle state. This does not look blocking for this PR now, but it would be good to keep in mind for the later PR. |
Lin-z-w
force-pushed
the
feat/tenant-metadata-map
branch
2 times, most recently
from
b987ec1 to
566b3f4
Compare
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Lin-z-w
force-pushed
the
feat/tenant-metadata-map
branch
from
c59f4d5 to
4ed418e
Compare
3 participants
Description
This PR introduces the first stage of tenant-aware metadata isolation in Mooncake Store.
The main change is to refactor
MasterServicemetadata from a single-levelkey -> ObjectMetadatamap into a tenant-aware namespace:This provides the internal metadata foundation needed for multi-tenancy while preserving legacy/default-tenant behavior. The default tenant keeps the old shard mapping so snapshots written before this change can still restore keys that are reachable by legacy APIs.
This PR also updates related master paths to avoid incorrect cross-tenant behavior:
defaulttenant.This PR does not implement full server-side tenant authorization. In particular, client-to-tenant binding, RPC-level tenant identity propagation, and tenant validation will be implemented in follow-up PRs. The planned staging is:
client_id -> tenant_idbinding in master.Module
mooncake-transfer-engine)mooncake-store)mooncake-ep)mooncake-integration)mooncake-p2p-store)mooncake-wheel)mooncake-pg)mooncake-rl)Type of Change
How Has This Been Tested?
Built the focused test targets with Ninja:
Ran the focused CTest suite:
ctest --test-dir build -R "^(master_service_test|offload_on_evict_test|promotion_on_hit_test|snapshot_child_process_test)$" --output-on-failureResult:
Checklist
./scripts/code_format.shbefore submitting.