Repository navigation
Conversation
kvcache-ai#3665 reads MOONCAKE_ENABLE_EMBEDDED_MASTER in real_client.cpp through Environ::GetBool, and kvcache-ai#4401, merged after it, removed that static helper. Each change built on its own branch, but main no longer compiles mooncake_store. Define MOONCAKE_ENABLE_EMBEDDED_MASTER in the environment variable catalog and read it with Environ::Process().GetTypedOr(..., false). This keeps the previous behavior: an unset value is false, the canonical boolean spellings are accepted, and an invalid value warns and falls back to false.
Aionw
requested review from
00fish0,
Icedcoco,
Libotry,
XucSh,
YiXR,
stmatengss and
ykwd
as code owners
October 8, 2026 12:14
…R registration kvcache-ai#4329 reads MC_RDMA_DATA_DIRECT in RdmaTransport::registerLocalMemoryInternal through Environ::Get().GetRdmaDataDirect(). kvcache-ai#4401, merged after it, removed that accessor and moved the setting to GlobalConfig::rdma_data_direct. The call is inside #ifdef USE_CUDA, so only CUDA builds fail. Read globalConfig().rdma_data_direct, as rdma_context.cpp already does. Both parse MC_RDMA_DATA_DIRECT with TryParseBool and fall back to false on an invalid value.
Every Store config exposes a single FromEnvironment(const Environ& env, ...) entry point. Production call sites pass Environ::Process(); these are the places where a later change threads resolved configs from the process entry point instead. Config tests inject a MapEnvironSource instead of mutating the process environment; tests that exercise production paths reading the process environment keep using setenv(). This removes the Read/ReadOr/GetDouble compatibility wrappers. Parsing, defaults and diagnostics are unchanged.
Aionw
force-pushed
the
refactor/environ-injectable-configs-v2
branch
from
October 8, 2026 12:35
101ab4b to
24ffbb6
Compare
Aionw
requested review from
alogfans,
chestnut-Q,
doujiang24 and
staryxchen
as code owners
October 8, 2026 12:35
This branch has not been deployed
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
Part of #3809. Replaces #4400, which was closed when its base branch
(
refactor/environ-reader, #4401) merged. Stacked on #4513: until thatmerges, this PR also shows its two commits; review only the last commit.
Every Store config now takes the environment as a parameter:
FromEnvironment(const Environ& env, ...).Environ::Process(). These are the places where a later change will pass down configs resolved at the process entry point instead.MapEnvironSourceinstead of callingsetenv/unsetenvand restoring values. Tests that exercise production paths that still read the process environment keep usingsetenv().Environ::Read/ReadOr/GetDoublewrappers that [Store] Make Environ an injectable environment reader #4401 kept.The change is mechanical. Parsing, defaults, diagnostics and alias handling are
unchanged. Most of the line count is tests (+1331/-1980 of +1876/-2363), which
drop their per-file process-environment guards.
Module
mooncake-store)mooncake-common)Type of Change
How Has This Been Tested?
s3_helper.cppandspdk_wrapper.cppare behind build options that were off; their changes are one-line call-site updates.environ_testandstandalone_client_test, pass on Linux.-fsyntax-only) withUSE_NOF(SPDK v23.01.1 headers),HAVE_AWS_SDK(AWS SDK 1.11 headers),USE_CUDA(GCC 11.4, CUDA 12.8) andUSE_CXL.Checklist
./scripts/code_format.shAI Assistance Disclosure
Claude Code (Claude Opus) assisted with the mechanical call-site and test migration. I reviewed every changed line.