Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions mooncake-conductor/include/conductor/common/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,22 @@ inline std::optional<PublisherKind> ParsePublisherKind(std::string_view value) {
struct HashProfileConfig {
std::string strategy;
std::string algorithm;
std::string root_digest;
std::string python_hash_seed;
std::string index_projection;

bool operator==(const HashProfileConfig&) const = default;
};

struct ResolvedHashProfile {
std::string strategy;
std::string algorithm;
std::string python_hash_seed;
std::string root_digest;
std::string index_projection;

bool operator==(const ResolvedHashProfile&) const = default;
};

struct ServiceConfig {
std::string endpoint; // kv publisher endpoint
std::string replay_endpoint; // replay publisher endpoint
Expand All @@ -50,7 +60,7 @@ struct ServiceConfig {
int64_t block_size = 0;
int dp_rank = 0;
std::optional<int64_t> cache_group;
HashProfileConfig hash_profile;
ResolvedHashProfile hash_profile;

bool operator==(const ServiceConfig&) const = default;
};
Expand Down
11 changes: 9 additions & 2 deletions mooncake-conductor/include/conductor/prefixindex/hash_strategy.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,17 @@ class HashStrategy {
std::vector<HashBlock>* out) const = 0;
};

// Returns an empty string when the profile is supported and well formed.
// Resolves a supported source profile and derives its root digest. Returns an
// empty string on success.
std::string ResolveHashProfile(const common::HashProfileConfig& config,
HashProfile* out);

// Returns an empty string when the resolved profile is supported, well formed,
// and its root digest matches a fresh derivation from python_hash_seed.
std::string ValidateHashProfile(const HashProfile& profile);

// Returns nullptr and sets error when the profile is invalid or unsupported.
// Returns nullptr and sets error when the resolved profile shape is invalid or
// unsupported. This consumes the derived root without hashing the seed again.
std::unique_ptr<HashStrategy> CreateHashStrategy(const HashProfile& profile,
std::string* error);

Expand Down
11 changes: 3 additions & 8 deletions mooncake-conductor/include/conductor/prefixindex/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include <string>
#include <vector>

#include "conductor/common/types.h"

namespace conductor {
namespace prefixindex {

Expand All @@ -19,14 +21,7 @@ struct ContextKey {
bool operator==(const ContextKey&) const = default;
};

struct HashProfile {
std::string strategy;
std::string algorithm;
std::string root_digest;
std::string index_projection;

bool operator==(const HashProfile&) const = default;
};
using HashProfile = common::ResolvedHashProfile;

struct ProjectedPrefix {
uint64_t value = 0;
Expand Down
58 changes: 49 additions & 9 deletions mooncake-conductor/src/kvevent/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
#include <cstdlib>
#include <fstream>
#include <limits>
#include <set>

#include "conductor/common/utils.h"
#include "conductor/prefixindex/hash_strategy.h"

namespace conductor {
namespace kvevent {
Expand Down Expand Up @@ -44,17 +46,50 @@ bool JsonInt64(const Json::Value& value, int64_t* out) {
return false;
}

common::HashProfileConfig ParseHashProfile(const Json::Value& raw) {
common::HashProfileConfig profile;
bool ParseHashProfile(const Json::Value& raw,
common::ResolvedHashProfile* profile,
std::string* error) {
if (!raw.isMember("hash_profile")) {
*error = "hash_profile is required";
return false;
}
const Json::Value& value = raw["hash_profile"];
if (!value.isObject()) {
return profile;
*error = "hash_profile must be an object";
return false;
}

static const std::set<std::string> kAllowedProfileFields = {
"algorithm", "index_projection", "python_hash_seed", "strategy"};
for (const std::string& name : value.getMemberNames()) {
if (!kAllowedProfileFields.contains(name)) {
*error = "unsupported hash_profile field: " + name;
return false;
}
}
profile.strategy = JsonString(value, "strategy");
profile.algorithm = JsonString(value, "algorithm");
profile.root_digest = JsonString(value, "root_digest");
profile.index_projection = JsonString(value, "index_projection");
return profile;

common::HashProfileConfig source;
const auto require_string = [&](const char* field, std::string* out) {
if (!value.isMember(field)) {
*error = std::string(field) + " is required";
return false;
}
if (!value[field].isString()) {
*error = std::string(field) + " must be a string";
return false;
}
*out = value[field].asString();
return true;
};
if (!require_string("strategy", &source.strategy) ||
!require_string("algorithm", &source.algorithm) ||
!require_string("python_hash_seed", &source.python_hash_seed) ||
!require_string("index_projection", &source.index_projection)) {
return false;
}

*error = prefixindex::ResolveHashProfile(source, profile);
return error->empty();
}

bool ParseCacheGroup(const Json::Value& raw,
Expand Down Expand Up @@ -174,7 +209,12 @@ std::vector<common::ServiceConfig> ParseConfig(int* http_server_port) {
LOG(ERROR) << "Invalid cache_group instance_id=" << name;
continue;
}
svc.hash_profile = ParseHashProfile(raw);
std::string profile_error;
if (!ParseHashProfile(raw, &svc.hash_profile, &profile_error)) {
LOG(ERROR) << "Invalid hash_profile stream_key=" << name
<< " error=" << profile_error;
continue;
}
services.push_back(std::move(svc));
}
}
Expand Down
5 changes: 1 addition & 4 deletions mooncake-conductor/src/kvevent/event_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ prefixindex::ContextKey ContextFromService(

prefixindex::HashProfile ProfileFromService(
const common::ServiceConfig& service) {
return {.strategy = service.hash_profile.strategy,
.algorithm = service.hash_profile.algorithm,
.root_digest = service.hash_profile.root_digest,
.index_projection = service.hash_profile.index_projection};
return service.hash_profile;
}

prefixindex::EngineOwner EngineOwnerFromService(
Expand Down
41 changes: 30 additions & 11 deletions mooncake-conductor/src/kvevent/event_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ prefixindex::ContextKey ContextFromService(

prefixindex::HashProfile ProfileFromService(
const common::ServiceConfig& service) {
return {.strategy = service.hash_profile.strategy,
.algorithm = service.hash_profile.algorithm,
.root_digest = service.hash_profile.root_digest,
.index_projection = service.hash_profile.index_projection};
return service.hash_profile;
}

prefixindex::EngineRegistration RegistrationFromService(
Expand Down Expand Up @@ -194,7 +191,7 @@ bool ParseOptionalCacheGroup(const Json::Value& body, coro_http_response& resp,
}

bool ParseHashProfileConfig(const Json::Value& body, coro_http_response& resp,
common::HashProfileConfig* profile) {
common::ResolvedHashProfile* profile) {
if (!body.isMember("hash_profile")) {
HttpJsonError(resp, "missing", "hash_profile is required",
"hash_profile");
Expand All @@ -207,15 +204,35 @@ bool ParseHashProfileConfig(const Json::Value& body, coro_http_response& resp,
return false;
}
static const std::set<std::string> kAllowedProfileFields = {
"algorithm", "index_projection", "root_digest", "strategy"};
"algorithm", "index_projection", "python_hash_seed", "strategy"};
if (!RejectUnknownFields(value, kAllowedProfileFields, resp)) {
return false;
}
return RequiredString(value, "strategy", resp, &profile->strategy) &&
RequiredString(value, "algorithm", resp, &profile->algorithm) &&
RequiredString(value, "root_digest", resp, &profile->root_digest) &&
RequiredString(value, "index_projection", resp,
&profile->index_projection);

common::HashProfileConfig source;
if (!RequiredString(value, "strategy", resp, &source.strategy) ||
!RequiredString(value, "algorithm", resp, &source.algorithm) ||
!RequiredString(value, "python_hash_seed", resp,
&source.python_hash_seed) ||
!RequiredString(value, "index_projection", resp,
&source.index_projection)) {
return false;
}

if (std::string error = prefixindex::ResolveHashProfile(source, profile);
!error.empty()) {
const char* field = "python_hash_seed";
if (source.strategy != "vllm_v1") {
field = "strategy";
} else if (source.algorithm != "sha256_cbor") {
field = "algorithm";
} else if (source.index_projection != "low64_be") {
field = "index_projection";
}
HttpJsonError(resp, "invalid_value", error, field);
return false;
}
Comment on lines +222 to +234

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic for determining the field that caused the error is a bit fragile as it duplicates the validation logic from prefixindex::ValidateProfileSelectors. If the supported profiles change in ValidateProfileSelectors, this code will also need to be updated to report the correct field in case of an error. This could lead to maintenance issues in the future.

A more robust approach could be for ResolveHashProfile to return a structured error indicating the problematic field. Barring that, you might consider reporting the error on the hash_profile object as a whole to avoid this duplication and potential for getting out of sync.

return true;
}

std::string ValidateServiceConfig(const common::ServiceConfig& service) {
Expand Down Expand Up @@ -499,6 +516,7 @@ Json::Value ServiceConfigToJson(const common::ServiceConfig& svc) {
Json::Value profile(Json::objectValue);
profile["strategy"] = svc.hash_profile.strategy;
profile["algorithm"] = svc.hash_profile.algorithm;
profile["python_hash_seed"] = svc.hash_profile.python_hash_seed;
profile["root_digest"] = svc.hash_profile.root_digest;
profile["index_projection"] = svc.hash_profile.index_projection;
out["HashProfile"] = profile;
Expand Down Expand Up @@ -969,6 +987,7 @@ void EventManager::RegisterHttpHandlers() {
Json::Value profile(Json::objectValue);
profile["strategy"] = view.profile.strategy;
profile["algorithm"] = view.profile.algorithm;
profile["python_hash_seed"] = view.profile.python_hash_seed;
profile["root_digest"] = view.profile.root_digest;
profile["index_projection"] = view.profile.index_projection;
c["hash_profile"] = profile;
Expand Down
Loading
Loading