Skip to content

[Store] Structure NoFDebugConfig environment settings - #4203

Open
bitborne wants to merge 2 commits into
kvcache-ai:mainfrom
bitborne:codex/nof-debug-environ-convergence
Open

bitborne wants to merge 2 commits into
kvcache-ai:mainfrom
bitborne:codex/nof-debug-environ-convergence

Conversation

@bitborne

@bitborne bitborne commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Description

Part of #3809. Extract MC_NOF_DEBUG and MC_NOF_DEBUG_INTERVAL_MS from transfer_task.cpp into a private, client-owned NoFDebugConfig and the common environment catalog. The NoF call sites retain their existing behavior: truthy tokens are case-insensitive (1, true, yes, on), invalid/absent intervals use 1000 ms, and each value is cached independently on its first use. The existing NoF QoS and worker settings remain outside this PR.

Raw strings intentionally preserve the old strtol syntax, including accepted leading whitespace and rejected trailing whitespace, without adding warnings or changing the public API. This follows the single-owner boundaries of merged #4165 and #4166. Open #4171 and #3424 also edit nearby NoF code but do not own these two debug variables; #4179 moves the client source layout. Any integration conflicts can be addressed if those PRs merge first.

Module

  • Mooncake Store (mooncake-store)
  • Common (mooncake-common)

Type of Change

  • Refactor

How Has This Been Tested?

Test commands:

cmake --build build/config-check --target nof_debug_config_test --parallel 231
ctest --test-dir build/config-check -R '^nof_debug_config_test$' --output-on-failure
./scripts/code_format.sh --check --staged
pre-commit run --files mooncake-common/include/environment_variables.h mooncake-store/src/CMakeLists.txt mooncake-store/src/config/nof_debug_config.cpp mooncake-store/src/config/nof_debug_config.h mooncake-store/src/transfer_task.cpp mooncake-store/tests/CMakeLists.txt mooncake-store/tests/nof_debug_config_test.cpp

Test results:

  • Unit tests pass for the new NoF debug configuration.
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

The new tests first failed with the default-only implementation, then passed after the configuration extraction. NoF-enabled compilation and integration should be validated in the NoF CI job.

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 updated the documentation (if applicable)
  • I have added tests to prove my changes are effective
  • For changes >500 LOC: I have filed an RFC issue

No documentation change: existing opt-in values and behavior remain unchanged.

AI Assistance Disclosure

  • No AI tools were used
  • AI tools were used (specify below)

Codex assisted with ownership analysis, implementation, focused RED-to-GREEN tests, and review. The human submitter will review every changed line and is responsible for the change end to end.

Signed-off-by: Schatten <czhengt@qq.com>

struct NoFDebugConfig {
bool enabled = false;
int interval_ms = 1000;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

std::chrono::milliseconds

bool enabled = false;
int interval_ms = 1000;

static bool ReadEnabledFromEnvironment();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Visibility issue, only part of those static method need to be public.

Signed-off-by: Schatten <czhengt@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants