[Store] Make Environ an injectable environment reader by Aionw · Pull Request #4401 · kvcache-ai/Mooncake

Aionw · 2026-09-29T12:30:04Z

Description

Part of #3809. Replaces #4388, whose earlier revision was too large; see the review there. This is the first of three stacked PRs. It is behavior-preserving and does not change any Store config.

Environ was a process-wide singleton. It eagerly cached about 40 settings, most of them unused, and it exposed static helpers that called getenv() directly. That tied every config loader to the real process environment.

This PR turns it into a reader over an injectable source:

Environ (mooncake-common/src/environ.h) reads through an EnvironSource.
- Environ::Process() reads the live process environment. It behaves as before and takes no snapshot.
- Tests can inject a MapEnvironSource.
- The API is Get, GetTyped, GetTypedOr and GetList, over the existing typed parser.
- The header moves to mooncake-common/src and is exported only to in-tree targets, through mooncake_common's BUILD_INTERFACE.
The cached singleton is removed. The settings still in use move to their owners and are still resolved once per process:
- RPC client I/O threads → RpcClientIoThreadsConfig (common).
- WITH_NVIDIA_PEERMEM, MC_RDMA_DATA_DIRECT and MC_EFA_CQ_THREADS → the Transfer Engine's GlobalConfig, loaded by loadGlobalConfig() next to its other environment settings. Parsing and defaults are unchanged.
Compatibility. The static Read, ReadOr and GetDouble stay as thin wrappers over Process() with the previous behavior, so no Store config changes here. The next PR migrates the configs and removes these wrappers.

Stack:

This PR
[Store] Load Store configs from an injected Environ #4400: Store configs take an injected Environ
[Store] Group environment variable definitions by process #4392: group environment variable definitions by process

Module

Common (mooncake-common)
Transfer Engine (mooncake-transfer-engine)
Mooncake Store (mooncake-store)

Type of Change

Refactor

How Has This Been Tested?

New and updated unit tests: environ_test (injection, typed reads, lists, and the compatibility wrappers) and rpc_client_io_threads_test.
Store config tests are unchanged and still pass, which shows the compatibility wrappers keep the previous behavior. I ran them standalone on macOS.
Linux CI (pending on this revision). The previous, larger revision passed full CI.

Checklist

I have performed a self-review of my own code
I have formatted my code using ./scripts/code_format.sh
I have run pre-commit on the files changed in this PR and all hooks pass
I have added tests to prove my changes are effective

AI Assistance Disclosure

AI tools were used (specify below)

Claude Code (Claude Opus) assisted with the refactor and verification. I reviewed every changed line.

Environ was a process-wide singleton that eagerly cached ~40 settings, most of them unused, plus static helpers that called getenv() directly, so every config loader was tied to the real process environment. Turn it into a reader over an injectable EnvironSource: production reads through Environ::Process() (the live process environment) and tests can inject a MapEnvironSource. The API is Get/GetTyped/GetTypedOr/GetList, and environ.h moves to mooncake-common/src, exported only to in-tree targets. The settings still in use move to their owners and are still resolved once per process: the RPC client I/O thread counts to RpcClientIoThreadsConfig, and the Transfer Engine's WITH_NVIDIA_PEERMEM, MC_RDMA_DATA_DIRECT and MC_EFA_CQ_THREADS to its GlobalConfig, next to its other environment settings. The static Read/ReadOr/GetDouble remain as compatibility wrappers over Process() so Store configs are unchanged; a follow-up migrates them and removes the wrappers.

Icedcoco

LGTM

…on API kvcache-ai#4401 removed Environ::GetBool and Environ::Get(), while two call sites that landed around the same time still use them, so main does not compile: - real_client.cpp (kvcache-ai#3665) calls Environ::GetBool for MOONCAKE_ENABLE_EMBEDDED_MASTER. Declare the variable in environment_variables.h and read it with Environ::ReadOr. - rdma_transport.cpp (kvcache-ai#4329, USE_CUDA only) calls Environ::Get().GetRdmaDataDirect(). Use globalConfig().rdma_data_direct, as kvcache-ai#4401 did in rdma_context.cpp. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…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.

The rebased Store build still calls Environ::GetBool, removed by kvcache-ai#4401. Include the current reader definition and use its typed ReadOr API for the two embedded-master checks. Preserve the environment key, false default, canonical boolean parser, and invalid-value diagnostics.

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. (cherry picked from commit 25958ec)

…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. (cherry picked from commit b41aeac)

* [Bugfix][Store] Restore the build after Environ::GetBool removal #3665 reads MOONCAKE_ENABLE_EMBEDDED_MASTER in real_client.cpp through Environ::GetBool, and #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. * [Bugfix][TransferEngine] Read Data Direct from globalConfig in CUDA MR registration #4329 reads MC_RDMA_DATA_DIRECT in RdmaTransport::registerLocalMemoryInternal through Environ::Get().GetRdmaDataDirect(). #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.

github-actions Bot added run-ci Store Transfer Engine Common labels Sep 29, 2026

This was referenced Sep 29, 2026

[Store] Load Store configs from an injected Environ #4400

Closed

[Store] Group environment variable definitions by process #4392

Closed

[Store] Make Environ an injectable environment reader #4388

Closed

Aionw force-pushed the refactor/environ-reader branch from 7d261e5 to 033333b Compare September 29, 2026 12:44

Aionw marked this pull request as ready for review September 29, 2026 12:49

Aionw requested review from XucSh, YiXR, alogfans, chestnut-Q, doujiang24, staryxchen, stmatengss, whn09 and ykwd as code owners September 29, 2026 12:49

Icedcoco approved these changes Sep 30, 2026

View reviewed changes

ykwd approved these changes Sep 30, 2026

View reviewed changes

alogfans approved these changes Oct 8, 2026

View reviewed changes

Aionw merged commit 775163d into kvcache-ai:main Oct 8, 2026
51 of 54 checks passed

Aionw deleted the refactor/environ-reader branch October 8, 2026 09:22

This was referenced Oct 8, 2026

[Bugfix] Restore builds broken by the Environ API removal #4513

Merged

[Store] Load Store configs from an injected Environ #4514

Open

SongOf mentioned this pull request Oct 8, 2026

[Bugfix][TENT] Release the MRs that did register when a NIC refuses a buffer #4515

Merged

8 of 32 tasks

he-yufeng mentioned this pull request Oct 8, 2026

Nightly build/test failure - 2026-10-08 #4520

Open

pingqiu mentioned this pull request Oct 8, 2026

[Store] Add S3-compatible ObjectStorageAdapter #4494

Merged

13 of 32 tasks

quantz8a mentioned this pull request Oct 9, 2026

Guard TCP pooled slice lifecycle during transfer completion #4428

Open

DHX98 mentioned this pull request Oct 9, 2026

[Store] Prefetch SSD-only objects to DRAM on exist #4272

Merged

10 tasks done

Juhyun-Kim-Memphis mentioned this pull request Oct 14, 2026

[Store] Fence the local disk tier on the first unrecoverable disk error #4490

Open

10 of 12 tasks

Navigation Menu

Repository navigation

[Store] Make Environ an injectable environment reader - #4401

[Store] Make Environ an injectable environment reader#4401
Aionw merged 1 commit into
kvcache-ai:mainfrom
Aionw:refactor/environ-reader

Aionw commented Sep 29, 2026 •

edited

Loading

Uh oh!

Icedcoco left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Aionw commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Module

Type of Change

How Has This Been Tested?

Checklist

AI Assistance Disclosure

Uh oh!

Icedcoco left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Aionw commented Sep 29, 2026 •

edited

Loading