-
Notifications
You must be signed in to change notification settings - Fork 957
[Store] feat: add workload-aware KV cache hints #2830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,23 @@ inline std::ostream& operator<<(std::ostream& os, | |
| return os; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Workload-level hints associated with an object. | ||
| */ | ||
| struct WorkloadHints { | ||
| std::string session_id{}; // Empty means unset. | ||
| int32_t retention_priority{0}; // Zero means unset. | ||
|
|
||
| bool operator==(const WorkloadHints&) const = default; | ||
|
|
||
| friend std::ostream& operator<<(std::ostream& os, | ||
| const WorkloadHints& hints) noexcept { | ||
| return os << "{ session_id: " << hints.session_id | ||
| << ", retention_priority: " << hints.retention_priority | ||
| << " }"; | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Configuration for replica management | ||
| */ | ||
|
|
@@ -96,6 +113,11 @@ struct ReplicateConfig { | |
| // ungrouped. Grouped keys share metadata routing, coalesced lease refresh, | ||
| // and memory eviction behavior. | ||
| std::optional<std::vector<std::string>> group_ids{}; | ||
| WorkloadHints workload_hints{}; | ||
|
|
||
| bool operator==(const ReplicateConfig&) const = default; | ||
|
|
||
| bool IsDefault() const { return *this == ReplicateConfig{}; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| ReplicateConfig ForSingleKey(size_t key_index) const { | ||
| ReplicateConfig key_config = *this; | ||
|
|
@@ -142,6 +164,7 @@ struct ReplicateConfig { | |
| } | ||
| os << "]"; | ||
| } | ||
| os << ", workload_hints: " << config.workload_hints; | ||
| os << " }"; | ||
| return os; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3012,7 +3012,8 @@ auto MasterService::AllocateAndInsertMetadata( | |
| std::piecewise_construct, std::forward_as_tuple(key), | ||
| std::forward_as_tuple(client_id, now, value_length, std::move(replicas), | ||
| config.with_soft_pin, config.with_hard_pin, | ||
| config.data_type, group_id, tenant_id, key)); | ||
| config.data_type, group_id, tenant_id, key, | ||
| config.workload_hints)); | ||
| if (!inserted) { | ||
| LOG(INFO) << "key=" << key << ", info=object_already_exists"; | ||
| abort_reserved_quota(); | ||
|
|
@@ -3713,6 +3714,7 @@ auto MasterService::UpsertStart(const UUID& client_id, const std::string& key, | |
| merged_config.with_hard_pin || metadata.IsHardPinned(); | ||
| merged_config.with_soft_pin = | ||
| merged_config.with_soft_pin || metadata.IsSoftPinned(); | ||
| merged_config.workload_hints = metadata.workload_hints; | ||
|
|
||
| const std::string existing_group_id = metadata.group_id; | ||
| const uint64_t old_quota_charge = | ||
|
|
@@ -8557,7 +8559,8 @@ MasterService::MetadataSerializer::DeserializeShard(const msgpack::object& obj, | |
| metadata_ptr->size, metadata_ptr->PopReplicas(), | ||
| metadata_ptr->soft_pin_timeout.has_value(), | ||
| metadata_ptr->IsHardPinned(), metadata_ptr->data_type, | ||
| metadata_ptr->group_id, tenant_id, user_key)); | ||
| metadata_ptr->group_id, tenant_id, user_key, | ||
| metadata_ptr->workload_hints)); | ||
|
|
||
| it->second.lease_timeout = metadata_ptr->lease_timeout; | ||
| it->second.soft_pin_timeout = metadata_ptr->soft_pin_timeout; | ||
|
|
@@ -8580,11 +8583,13 @@ MasterService::MetadataSerializer::SerializeMetadata( | |
| // Pack ObjectMetadata using array structure for efficiency | ||
| // Format: [client_id, put_start_time, size, lease_timeout, | ||
| // has_soft_pin_timeout, soft_pin_timeout, replicas_count, data_type, | ||
| // replicas..., hard_pinned, group_id] | ||
| // replicas..., hard_pinned, group_id, | ||
| // [workload_session_id, workload_retention_priority]] | ||
|
|
||
| size_t array_size = 10; // client_id, put_start_time, size, lease_timeout, | ||
| size_t array_size = 11; // client_id, put_start_time, size, lease_timeout, | ||
| // has_soft_pin_timeout, soft_pin_timeout, | ||
| // replicas_count, data_type, hard_pinned, group_id | ||
| // replicas_count, data_type, hard_pinned, | ||
| // group_id, workload_hints | ||
| array_size += metadata.CountReplicas(); // One element per replica | ||
| packer.pack_array(array_size); | ||
|
|
||
|
|
@@ -8638,6 +8643,9 @@ MasterService::MetadataSerializer::SerializeMetadata( | |
|
|
||
| packer.pack(metadata.IsHardPinned()); | ||
| packer.pack(metadata.group_id); | ||
| packer.pack_array(2); | ||
| packer.pack(metadata.workload_hints.session_id); | ||
| packer.pack(metadata.workload_hints.retention_priority); | ||
|
|
||
| return {}; | ||
| } | ||
|
|
@@ -8690,12 +8698,14 @@ MasterService::MetadataSerializer::DeserializeMetadata( | |
| // v1: 7 + replicas_count, no optional fields | ||
| // v2: 8 + replicas_count, either data_type or hard_pinned | ||
| // v3: 9 + replicas_count, data_type + hard_pinned or hard_pinned + | ||
| // group_id v4: 10 + replicas_count, data_type + hard_pinned + group_id | ||
| // group_id | ||
| // v4: 10 + replicas_count, data_type + hard_pinned + group_id | ||
| // v5: 11 + replicas_count, v4 + nested workload_hints | ||
| // 64-bit arithmetic keeps an attacker-controlled near-UINT32_MAX | ||
| // replicas_count from wrapping the bounds and slipping an out-of-bounds | ||
| // index past the size check. | ||
| constexpr uint64_t kBaseFieldCount = 7; | ||
| constexpr uint64_t kMaxOptionalFieldCount = 3; | ||
| constexpr uint64_t kMaxOptionalFieldCount = 4; | ||
| const uint64_t total_elements = obj.via.array.size; | ||
| const uint64_t min_elements = kBaseFieldCount + replicas_count; | ||
| if (total_elements < min_elements || | ||
|
|
@@ -8745,14 +8755,43 @@ MasterService::MetadataSerializer::DeserializeMetadata( | |
| group_id = array[index++].as<std::string>(); | ||
| } | ||
|
|
||
| WorkloadHints workload_hints; | ||
| if (index < total_elements) { | ||
| const auto& hints_object = array[index++]; | ||
| if (hints_object.type != msgpack::type::ARRAY || | ||
| hints_object.via.array.size != 2) { | ||
|
Comment on lines
+8759
to
+8762
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To ensure robust forward-compatibility, we should check if the next element is actually an array before consuming it as if (index < total_elements && array[index].type == msgpack::type::ARRAY) {
const auto& hints_object = array[index++];
if (hints_object.via.array.size != 2) { |
||
| return tl::unexpected(SerializationError( | ||
| ErrorCode::DESERIALIZE_FAIL, | ||
| "deserialize workload_hints must be a two-element array")); | ||
| } | ||
|
|
||
| const auto* hints_array = hints_object.via.array.ptr; | ||
| if (hints_array[0].type != msgpack::type::STR || | ||
| (hints_array[1].type != msgpack::type::POSITIVE_INTEGER && | ||
| hints_array[1].type != msgpack::type::NEGATIVE_INTEGER)) { | ||
| return tl::unexpected(SerializationError( | ||
| ErrorCode::DESERIALIZE_FAIL, | ||
| "deserialize workload_hints has invalid field types")); | ||
| } | ||
|
|
||
| try { | ||
| workload_hints.session_id = hints_array[0].as<std::string>(); | ||
| workload_hints.retention_priority = hints_array[1].as<int32_t>(); | ||
| } catch (const std::exception& e) { | ||
| return tl::unexpected(SerializationError( | ||
| ErrorCode::DESERIALIZE_FAIL, | ||
| "deserialize workload_hints failed: " + std::string(e.what()))); | ||
| } | ||
| } | ||
|
|
||
| // Create ObjectMetadata instance | ||
| bool enable_soft_pin = has_soft_pin_timeout; | ||
| auto metadata = std::make_unique<ObjectMetadata>( | ||
| client_id, | ||
| std::chrono::system_clock::time_point( | ||
| std::chrono::milliseconds(put_start_time_timestamp)), | ||
| size, std::move(replicas), enable_soft_pin, is_hard_pinned, data_type, | ||
| group_id); | ||
| group_id, "default", std::string{}, std::move(workload_hints)); | ||
| metadata->lease_timeout = std::chrono::system_clock::time_point( | ||
| std::chrono::milliseconds(lease_timestamp)); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking
operator<<asnoexceptis unsafe because stream insertion operations (such as writing toosor copying/formattinghints.session_id) can throw exceptions (e.g., if the stream has its exception mask configured to throw on failure, or due to allocation failures). If an exception is thrown inside anoexceptfunction,std::terminatewill be called immediately. It is standard practice to omitnoexceptfrom stream insertion operators.