[bitborne]

Description

Continue the environment-variable convergence work with FileStorageConfig::FromEnvironment() as an independent Mooncake Store pilot.

This change:

• Groups all environment-variable definitions owned by FileStorageConfig in mooncake-common.
• Uses the actual environment-variable names as the definition identifiers.
• Moves FileStorageConfig environment loading and validation into mooncake-store/src/config/.
• Routes typed values through the existing Environ entry point.
• Keeps legacy boolean and disk-ratio parsing explicit where required to preserve existing behavior.
• Splits configuration tests from the FileStorage runtime tests.

This PR does not change:

• Existing default values
• Environment-variable names
• Parsing rules
• Alias precedence
• Configuration read timing
• Configuration-file or CLI behavior

The focused tests cover default values, valid values, invalid values, empty values, legacy alias fallback, preferred-variable precedence, validation behavior, and diagnostic behavior. Further environment-variable convergence can continue module by module after this pilot.

Module

• Mooncake Store (mooncake-store)
• Transfer Engine (mooncake-transfer-engine)
• 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-dir> --target file_storage_config_test -j32
<build-dir>/mooncake-store/tests/file_storage_config_test --gtest_color=no
clang-format-20 --dry-run --Werror \
  mooncake-common/include/environment_variables.h \
  mooncake-store/src/config/file_storage_config.cpp \
  mooncake-store/src/file_storage.cpp \
  mooncake-store/tests/file_storage_config_test.cpp \
  mooncake-store/tests/file_storage_test.cpp
git diff --check

Test results:

• Unit tests pass: file_storage_config_test passed all 13 tests
• 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

Codex assisted with repository investigation, implementation, test migration, verification, and code review. The human submitter reviewed the changed lines and is responsible for understanding and defending the complete change.

[he-yufeng]

Read the whole diff against current main and checked the refactor is behavior-preserving, chain by chain:

• All four alias chains keep their precedence: preferred wins when set, legacy fills in, and the two odd corners match the old code. Set-but-empty preferred falls through to the legacy var; set-but-invalid preferred on the ratio and bool vars resolves to the built-in default rather than the legacy value (the scanmeta chain is the exception, invalid preferred still falls to legacy there, same as the old nested GetInt64).
• The ratio parser keeps its exact acceptance surface: classic-locale stream, rejects trailing garbage ("0.75suffix"), nan, non-positive and >1.0, all landing on the default.
• The diagnostics split is preserved: the typed numeric reads log the variable name on garbage, the strict-bool and ratio paths stay silent, and the new tests assert both directions.

ScopedEnvVar saving and restoring the pre-existing value instead of plain unset is the right call; env tests that only unset are order-dependent.

One nit, not blocking: the two uring vars and the watermark bool/ratios are declared as EnvironmentVariablestd::string while their siblings are typed numerics. That is what preserves the custom strict-bool semantics and the silent-on-invalid behavior, so it is intentional, but a one-line comment near the struct would save the next reader a trip to FromEnvironment to learn why.

[bitborne]

Thanks for the suggestion. I added a comment next to the string-typed bool/ratio definitions to clarify that they intentionally preserve the legacy custom parsing and silent invalid-value behavior.

[Aionw]

Here we can use macro to simplify variable def. e.g.

#define MC_DEFINE_ENV_VAR(Type, Name) \
    inline static constexpr EnvironmentVariable<Type> Name{#Name}

MC_DEFINE_ENV_VAR(
    int64_t,
    MC_STORE_PINNED_RESTORE_ARENA_SIZE_BYTES
);

[bitborne]

Updated all FileStorage environment-variable definitions to use MC_DEFINE_ENV_VAR(Type, Name), with #Name generating the environment-variable string. The macro is undefined after the grouped definitions to avoid leaking it to includers.

[Aionw]

LGTM