[GH-ISSUE #4360] [RFC]: [Store] Phase 2 Configuration Ownership and Compatibility Plan #1093

New issue

Open

opened 2026-10-29 21:22:49 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-29 21:22:49 +00:00

Owner

Originally created by @bitborne on GitHub (Sep 28, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4360

Changes proposed

Context and goal

Mooncake Store currently assembles Master configuration across flat fields, file parsing, gflags, environment fallbacks, and HA/non-HA startup paths. A reader must trace master.cpp and several consumers to learn who owns a setting, which source wins, and when it is checked. That makes a seemingly small configuration change easy to apply to only one startup path.

This RFC continues the configuration direction in #2694 and the phased work in #3809. The immediate goal is one typed, component-owned configuration per coherent setting group, resolved once at startup and passed to its consumers. It is an internal ownership refactor, not a new public configuration schema or a behavior change.

Scope and ownership

A bootstrap config contains startup inputs needed to construct a component. A runtime config describes settings consumed during service operation. A runtime config may still be an immutable initial snapshot in this phase; hot reload is not implied.
Each group has one owner and one resolved value. A field should not remain independently mutable in both the typed config and MasterConfig. MasterConfig may temporarily aggregate the typed configs while existing constructors and HA/non-HA paths are migrated.
Put source parsing and merge logic in the group's loader; keep gflags inspection at the process boundary. Business code receives a typed config, not FLAGS_*, file keys, or environment lookups.
Keep the public flat YAML/JSON keys and gflags names for this phase. Grouped public keys, config-file schema migration, reload, and cross-component reorganization require separate decisions.

Resolution contract

For each field, start with the current default, apply an existing file key when present, then apply an explicitly supplied CLI flag. An unspecified flag must not erase a file value. Where an existing environment variable is a fallback, apply it only under its current condition (for example, pod identity only when the effective value is empty). Preserve deprecated aliases, warning text/conditions, special zero or empty meanings, and failure timing unless a PR calls out a separately reviewed behavior change.

The loader's interface follows the pattern already in use:

struct XCommandLineOverrides {
    std::optional<ValueType> setting;
    // One optional per existing CLI setting; absent means "not explicitly set".
};

XConfig ResolveXConfig(
    const DefaultConfig* file_config,
    const XCommandLineOverrides& command_line);

XConfig declares domain types and defaults close to the owner (std::chrono::seconds/milliseconds for durations, for example). The loader reads the existing flat file keys, merges explicit overrides, validates the effective result, and returns a value. Each default is declared once, as a named constant on the owner config, and the corresponding DEFINE_* gflags default references that constant instead of repeating the literal, so a later default change cannot silently diverge between the file default, the config default, and the flag default. It must not silently convert invalid signed values to unsigned or narrow ports/sizes. If a group has legacy aliases or a conditional fallback, document and test their exact order rather than forcing a generic merge helper that changes semantics. OS-dependent resolution and side effects can stay at startup assembly until a focused design places them behind a testable boundary.

What changes at the call site

Before:
  file / flags / environment -> scattered MasterConfig fields
  -> startup conditionals and HA/non-HA adapters -> component

After this phase:
  file + explicitly set flags (+ existing conditional fallback)
  -> ResolveXConfig(...) -> one typed config
  -> temporary MasterConfig aggregation -> both startup paths -> component

Target after adapter cleanup:
  validated BootstrapConfig + initial RuntimeConfig snapshot
  -> component constructors / runtime consumers

This deliberately allows a narrow PR to change ownership without rewriting every constructor at once. It does not authorize leaving a second parser for the same group in InitMasterConf or LoadConfigFromCmdline: once a group is migrated, both old assignments should be removed, and both startup paths should consume the same resolved value. The temporary aggregation is a forwarding adapter, not another source of truth.

Worked example for a future PR: RPC server bind settings

The RPC listener's rpc_port, rpc_thread_num, rpc_address, and rpc_interface are a coherent future owner group, separate from the RPC connection timeout/TCP settings. A focused PR could introduce RpcBindBootstrapConfig and RpcBindCommandLineOverrides, resolve the current flat keys once, then forward the result through the existing HA and non-HA server construction paths. The public flags and file keys stay unchanged.

// Illustrative boundary; exact member types follow the existing consumers.
RpcBindBootstrapConfig ResolveRpcBindBootstrapConfig(
    const DefaultConfig* file_config,
    const RpcBindCommandLineOverrides& command_line,
    const RpcBindStartupContext& startup);  // e.g. hardware thread count

master_config.rpc_bind = ResolveRpcBindBootstrapConfig(
    loaded_default_config, GetRpcBindCommandLineOverrides(), startup);
// HA and non-HA adapters both forward master_config.rpc_bind.

This group has nontrivial compatibility rules: deprecated port/max_threads aliases, preference for positive rpc_port/rpc_thread_num, the hardware-concurrency fallback when no config file is provided, and rpc_interface overriding rpc_address after interface lookup. The PR must pin the current behavior with tests before moving the logic. Interface lookup and its diagnostics may remain in startup assembly in that PR; a pure loader need not perform network-interface I/O. A proposal to change alias precedence or defaults is out of scope for that ownership PR.

Reusable PR change template

Each subsequent PR should cover one coherent owner group, with this reviewable shape:

Typed contract: add XConfig in the owner's config header and a ResolveXConfig loader with an optional-valued override struct. Use semantic types where they prevent unit mistakes; keep conversions and validation at the boundary. Keep unrelated settings, public key renames, and broad constructor rewrites out of the PR.
One assembly path: collect only explicitly set flags in master.cpp, call the resolver once, remove the group's old flat parsing/assignments, and forward the typed value to every relevant adapter/consumer. Update build registration only if a new source file requires it.
Compatibility tests: cover defaults, file only, explicit CLI over file, unspecified CLI not overriding file, special zero/empty values, aliases or environment fallbacks where applicable, invalid values and diagnostics, and both HA/non-HA forwarding when relevant.

The PR is complete when every setting in its group has one effective resolved value, all existing input forms still work, and no consumer reads the old field or source independently. A failed compatibility case blocks the ownership migration; it is not deferred to a later cleanup PR.

Phase boundary and risks

Now (Phase 2a): migrate groups incrementally to typed owner configs and loaders, preserving flat public inputs and startup behavior. Existing open owner PRs use this shape; this RFC does not depend on their merge order. The NoF heartbeat startup resolver is an initial-input step, not a claim that its operational settings permanently belong to bootstrap.
Later (Phase 2b): assemble validated bootstrap and initial runtime snapshots as described in #2694, retire temporary flat forwarding, and pass owner configs directly to consumers. Classify NoF heartbeat's operational settings with the runtime owner at that boundary. This does not require hot reload.
Separate proposal: any grouped YAML/JSON schema, flag removal, changed precedence/defaults, or runtime reload contract.

The main risks are split ownership during migration, changed alias/fallback precedence, silent narrowing or unit conversion, and drift between HA and non-HA wiring. The one-resolver rule, the shared default constants, and the compatibility tests above are the acceptance checks for those risks. PRs touching shared master.cpp may need sequencing or conflict resolution, but should not broaden their ownership boundary to absorb adjacent settings.

Before submitting a new issue...

Searched for relevant issues and read the existing configuration direction in #2694 and #3809.

Originally created by @bitborne on GitHub (Sep 28, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4360 ## Changes proposed ### Context and goal Mooncake Store currently assembles Master configuration across flat fields, file parsing, gflags, environment fallbacks, and HA/non-HA startup paths. A reader must trace `master.cpp` and several consumers to learn who owns a setting, which source wins, and when it is checked. That makes a seemingly small configuration change easy to apply to only one startup path. This RFC continues the configuration direction in #2694 and the phased work in #3809. The immediate goal is **one typed, component-owned configuration per coherent setting group**, resolved once at startup and passed to its consumers. It is an internal ownership refactor, not a new public configuration schema or a behavior change. ### Scope and ownership - A bootstrap config contains startup inputs needed to construct a component. A runtime config describes settings consumed during service operation. A runtime config may still be an immutable *initial snapshot* in this phase; hot reload is not implied. - Each group has one owner and one resolved value. A field should not remain independently mutable in both the typed config and `MasterConfig`. `MasterConfig` may temporarily aggregate the typed configs while existing constructors and HA/non-HA paths are migrated. - Put source parsing and merge logic in the group's loader; keep gflags inspection at the process boundary. Business code receives a typed config, not `FLAGS_*`, file keys, or environment lookups. - Keep the public flat YAML/JSON keys and gflags names for this phase. Grouped public keys, config-file schema migration, reload, and cross-component reorganization require separate decisions. ### Resolution contract For each field, start with the current default, apply an existing file key when present, then apply an **explicitly supplied** CLI flag. An unspecified flag must not erase a file value. Where an existing environment variable is a fallback, apply it only under its current condition (for example, pod identity only when the effective value is empty). Preserve deprecated aliases, warning text/conditions, special zero or empty meanings, and failure timing unless a PR calls out a separately reviewed behavior change. The loader's interface follows the pattern already in use: ```cpp struct XCommandLineOverrides { std::optional<ValueType> setting; // One optional per existing CLI setting; absent means "not explicitly set". }; XConfig ResolveXConfig( const DefaultConfig* file_config, const XCommandLineOverrides& command_line); ``` `XConfig` declares domain types and defaults close to the owner (`std::chrono::seconds`/`milliseconds` for durations, for example). The loader reads the **existing** flat file keys, merges explicit overrides, validates the effective result, and returns a value. Each default is declared once, as a named constant on the owner config, and the corresponding `DEFINE_*` gflags default references that constant instead of repeating the literal, so a later default change cannot silently diverge between the file default, the config default, and the flag default. It must not silently convert invalid signed values to unsigned or narrow ports/sizes. If a group has legacy aliases or a conditional fallback, document and test their exact order rather than forcing a generic merge helper that changes semantics. OS-dependent resolution and side effects can stay at startup assembly until a focused design places them behind a testable boundary. ### What changes at the call site ```text Before: file / flags / environment -> scattered MasterConfig fields -> startup conditionals and HA/non-HA adapters -> component After this phase: file + explicitly set flags (+ existing conditional fallback) -> ResolveXConfig(...) -> one typed config -> temporary MasterConfig aggregation -> both startup paths -> component Target after adapter cleanup: validated BootstrapConfig + initial RuntimeConfig snapshot -> component constructors / runtime consumers ``` This deliberately allows a narrow PR to change ownership without rewriting every constructor at once. It does **not** authorize leaving a second parser for the same group in `InitMasterConf` or `LoadConfigFromCmdline`: once a group is migrated, both old assignments should be removed, and both startup paths should consume the same resolved value. The temporary aggregation is a forwarding adapter, not another source of truth. ### Worked example for a future PR: RPC server bind settings The RPC listener's `rpc_port`, `rpc_thread_num`, `rpc_address`, and `rpc_interface` are a coherent future owner group, separate from the RPC *connection* timeout/TCP settings. A focused PR could introduce `RpcBindBootstrapConfig` and `RpcBindCommandLineOverrides`, resolve the current flat keys once, then forward the result through the existing HA and non-HA server construction paths. The public flags and file keys stay unchanged. ```cpp // Illustrative boundary; exact member types follow the existing consumers. RpcBindBootstrapConfig ResolveRpcBindBootstrapConfig( const DefaultConfig* file_config, const RpcBindCommandLineOverrides& command_line, const RpcBindStartupContext& startup); // e.g. hardware thread count master_config.rpc_bind = ResolveRpcBindBootstrapConfig( loaded_default_config, GetRpcBindCommandLineOverrides(), startup); // HA and non-HA adapters both forward master_config.rpc_bind. ``` This group has nontrivial compatibility rules: deprecated `port`/`max_threads` aliases, preference for positive `rpc_port`/`rpc_thread_num`, the hardware-concurrency fallback when no config file is provided, and `rpc_interface` overriding `rpc_address` after interface lookup. The PR must pin the current behavior with tests **before** moving the logic. Interface lookup and its diagnostics may remain in startup assembly in that PR; a pure loader need not perform network-interface I/O. A proposal to change alias precedence or defaults is out of scope for that ownership PR. ### Reusable PR change template Each subsequent PR should cover **one coherent owner group**, with this reviewable shape: 1. **Typed contract:** add `XConfig` in the owner's config header and a `ResolveXConfig` loader with an optional-valued override struct. Use semantic types where they prevent unit mistakes; keep conversions and validation at the boundary. Keep unrelated settings, public key renames, and broad constructor rewrites out of the PR. 2. **One assembly path:** collect only explicitly set flags in `master.cpp`, call the resolver once, remove the group's old flat parsing/assignments, and forward the typed value to every relevant adapter/consumer. Update build registration only if a new source file requires it. 3. **Compatibility tests:** cover defaults, file only, explicit CLI over file, unspecified CLI not overriding file, special zero/empty values, aliases or environment fallbacks where applicable, invalid values and diagnostics, and both HA/non-HA forwarding when relevant. The PR is complete when every setting in its group has one effective resolved value, all existing input forms still work, and no consumer reads the old field or source independently. A failed compatibility case blocks the ownership migration; it is not deferred to a later cleanup PR. ### Phase boundary and risks - **Now (Phase 2a):** migrate groups incrementally to typed owner configs and loaders, preserving flat public inputs and startup behavior. Existing open owner PRs use this shape; this RFC does not depend on their merge order. The NoF heartbeat startup resolver is an initial-input step, not a claim that its operational settings permanently belong to bootstrap. - **Later (Phase 2b):** assemble validated bootstrap and initial runtime snapshots as described in #2694, retire temporary flat forwarding, and pass owner configs directly to consumers. Classify NoF heartbeat's operational settings with the runtime owner at that boundary. This does not require hot reload. - **Separate proposal:** any grouped YAML/JSON schema, flag removal, changed precedence/defaults, or runtime reload contract. The main risks are split ownership during migration, changed alias/fallback precedence, silent narrowing or unit conversion, and drift between HA and non-HA wiring. The one-resolver rule, the shared default constants, and the compatibility tests above are the acceptance checks for those risks. PRs touching shared `master.cpp` may need sequencing or conflict resolution, but should not broaden their ownership boundary to absorb adjacent settings. ### Before submitting a new issue... - [x] Searched for relevant issues and read the existing configuration direction in #2694 and #3809.

bsakaguchi commented

2026-10-29 21:22:50 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Sep 28, 2026):

Thanks for opening this issue, @bitborne!

Field	Value
Issue	#4360
GitHub user ID	`177341921`
Reporter	@bitborne

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Sep 28, 2026): Thanks for opening this issue, @bitborne! | Field | Value | |-------|-------| | **Issue** | #4360 | | **GitHub user ID** | `177341921` | | **Reporter** | @bitborne | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.