Skip to content

[Store] Structure ClientMetricConfig environment settings - #3773

Merged
Aionw merged 3 commits into
kvcache-ai:mainfrom
bitborne:store/client-metric-environ-convergence
Sep 2, 2026
Merged

Aionw merged 3 commits into
kvcache-ai:mainfrom
bitborne:store/client-metric-environ-convergence

Conversation

@bitborne

@bitborne bitborne commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Description

This PR continues the staged convergence of mooncake-store environment configuration for client metrics.

  • Group MC_STORE_CLIENT_METRIC, MC_STORE_CLIENT_METRIC_INTERVAL, and MC_STORE_CLIENT_METRIC_BANDWIDTH definitions under ClientMetricEnvironmentVariables.
  • Add ClientMetricConfig::FromEnvironment() to own environment reads and parsing before ClientMetric construction.
  • Preserve the existing defaults, accepted values, invalid and empty value fallbacks, diagnostics, and the disabled-metrics short-circuit.
  • Add focused tests for default, valid, invalid, empty, negative, overflow, zero, logging, and environment restoration behavior.

PR #1538 also touches client metric environment handling as part of a broad cross-component migration. This PR is intentionally limited to the current grouped environment-definition and configuration-object pattern, without changing existing client metric behavior.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Reshard (mooncake-reshard)
  • Mooncake EP (mooncake-ep)
  • Mooncake PG (mooncake-pg)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • Common (mooncake-common)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Performance improvement
  • Other

How Has This Been Tested?

Test commands:

cmake --build build --target client_metric_config_test client_metrics_test -j32
ctest --test-dir build -R '^(client_metric_config_test|client_metrics_test)$' --output-on-failure
pre-commit run --files mooncake-common/include/environment_variables.h mooncake-store/include/client_metric.h mooncake-store/src/CMakeLists.txt mooncake-store/src/client_metric.cpp mooncake-store/src/config/client_metric_config.cpp mooncake-store/tests/CMakeLists.txt mooncake-store/tests/client_metric_config_test.cpp mooncake-store/tests/client_metrics_test.cpp

Test results:

  • Unit tests pass
  • Integration tests pass (if applicable)
  • Manual testing done (describe below)

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

AI Assistance Disclosure

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

Codex was used to inspect the existing configuration patterns, implement the focused refactor and tests, and run review and verification checks. I reviewed the final diff and can explain the changes end to end.

Comment thread mooncake-store/include/client_metric.h Outdated

struct ClientMetricConfig {
bool enabled = true;
uint64_t reporting_interval_seconds = 0;

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.

Nit. Prefer std::chrono::milliseconds instead of uint64_t.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated ClientMetricConfig::reporting_interval to std::chrono::milliseconds, while preserving the environment variable’s seconds semantics through explicit conversion.

@bitborne
bitborne force-pushed the store/client-metric-environ-convergence branch from 30bc358 to d75b583 Compare August 31, 2026 09:48
Signed-off-by: Schatten <czhengt@qq.com>
Signed-off-by: Schatten <czhengt@qq.com>
@bitborne
bitborne force-pushed the store/client-metric-environ-convergence branch from d75b583 to 25e083f Compare September 1, 2026 08:04
@Aionw
Aionw merged commit 0d3ef5e into kvcache-ai:main Sep 2, 2026
7 of 8 checks passed
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.

3 participants