Skip to content

[Store] Structure NoFQosConfig environment settings - #4171

Open
bitborne wants to merge 1 commit into
kvcache-ai:mainfrom
bitborne:codex/nof-qos-environ-convergence
Open

bitborne wants to merge 1 commit into
kvcache-ai:mainfrom
bitborne:codex/nof-qos-environ-convergence

Conversation

@bitborne

@bitborne bitborne commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Description

Part of #3809. Move MC_NOF_SUBMIT_CHUNK_BYTES and MC_NOF_INFLIGHT_BYTES_LIMIT into a private, client-owned NoFQosConfig and the common environment catalog. The first QoS construction still caches both byte limits in the original read order, with defaults of 131072 and 33554432 bytes. Absent/empty inputs remain silent; invalid nonempty inputs fall back independently and keep the original raw-value warnings. Conversion to block counts remains in SpdkNofQos because it depends on the current block size. No public API or NoF worker/debug setting changes.

The shared typed integer parser is used for nonempty input. It intentionally accepts trailing ASCII whitespace (for example, "2048 "), which the previous strtol path rejected. The focused tests cover defaults, independent parsing and fallback, raw warnings and their order, integer boundaries, and process-first-use caching. Block-count assertions are guarded by USE_NOF and require an enabled build.

This is a separate configuration owner from the simultaneously proposed NoFWorkerPoolConfig PR. Open #3424 renames the QoS type and #3292 changes related QoS code, but neither moves these variables to an owned config. If either merges first and produces a real conflict or behavior change, integrate against the landed code before merge; a behind count alone does not call for rebase.

Module

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

Type of Change

  • Refactor

How Has This Been Tested?

Test commands:

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

Test results:

  • The five focused NoFQosConfig tests pass with USE_NOF=OFF; transfer_task_test builds.
  • 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

No documentation change: there is no public API or format change; the typed-parser whitespace normalization is disclosed above.

AI Assistance Disclosure

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

Codex assisted with owner analysis, implementation, focused tests, build checks, and self-review. The human submitter must review every changed line and be able to defend the change end to end.

Signed-off-by: Schatten <czhengt@qq.com>
LujhCoconut added a commit to LujhCoconut/Mooncake that referenced this pull request Oct 9, 2026
Resolve conflicts between the NoF/SPDK decoupling and mainline work:

- Env-var structuration PRs (kvcache-ai#4166/kvcache-ai#4170/kvcache-ai#4203): keep the neutral NoF
  naming/refactor while adopting the client-owned config structs
  (NoFDebugConfig, NoFWorkerPoolConfig, FilereadWorkerPoolConfig); QoS
  byte-limit vars stay in transfer_task.cpp, matching main (open kvcache-ai#4171).
- kvcache-ai#3665 embedded master + P2PHANDSHAKE metadata resolution compose with the
  added nof_initiator injection in Client::Create.
- kvcache-ai#4070 TENT native API layered onto the refactored TransferSubmitter.
- kvcache-ai#3717 (ShmHelper/receiver shm SPDK registration) is ported from the deleted
  SpdkWrapper singleton onto the NVMeoFInitiator interface and the
  process-global NofPageRegistry: registrations normalize to 2MB pages with
  per-page refcounts, -EBUSY pages are absorbed as external, and a failed
  Register owns no registration. ShmHelper's singleton is now deliberately
  leaked so exit-time cleanup can never touch a torn-down registry/env; the
  SpdkEnvGuard process pin makes the old construction-ordering hack
  unnecessary.

This branch has not been deployed

No deployments
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