feat(cn): a validated registry for the CN's transport tunables - #1706
Draft
aocsa wants to merge 1 commit into
Draft
feat(cn): a validated registry for the CN's transport tunables#1706aocsa wants to merge 1 commit into
aocsa wants to merge 1 commit into
Conversation
Ad-hoc .parse().ok() turned SIRIUS_CN_NIXL_WARMUP_TIMEOUT_SECS=6O (letter O) into the silent 180 s default, so the knob looked dead. tunable.rs declares the six transport knobs with defaults and ranges, rejects bad values instead of clamping or ignoring them, resolves the set once at bring-up as the first statement of main::run so a typo fails startup, and logs what the CN got. Consumers (nixl transport, PRPC client, warmup) land in later PRs; the fields and Tunables::get are pub so clippy -D warnings stays green meanwhile. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Motivation. The multi-CN transport that follows this PR has six environment knobs (RPC and nixl
transfer timeouts, the bandwidth canary size and floor, the warmup budget and peer count). On the
source branch the warmup path read its two knobs with
.and_then(|v| v.parse().ok()), whichturned
SIRIUS_CN_NIXL_WARMUP_TIMEOUT_SECS=6O(letter O, not zero) into the 180 s default with nodiagnostic. The knob looked dead. The other four were hardcoded constants with no env path at all;
the registry is what gives them one. An unparsable timeout is a startup error, not a default.
What changed (one reviewable unit: the registry, its wiring, and its doc).
experimental/starrocks/src/tunable.rs(new): aKnob<T>with name, default and[min, max],six knob constants, and
Tunables::{resolve, get}over aOnceLock. Three rules. A value thatdoes not parse or falls outside its range is rejected, never clamped and never ignored. The set
is resolved once at bring-up and a bad value fails the process. The resolved set is logged, so
the log says what the CN got rather than what the launcher echoed. Unset and empty both mean
the compiled default.
NaNis rejected before the range check (it compares false against bothbounds).
SIRIUS_CN_NIXL_CANARY_FLOOR_GBPS=0is the documented off switch and logs a warning.experimental/starrocks/src/main.rs:Tunables::resolve()is the first statement ofrun,before any port is bound or the GPU pool is reserved.
experimental/starrocks/src/lib.rs:mod tunable;andpub use tunable::Tunables;.experimental/starrocks/docs/TUNABLES.md(new): how the registry behaves and a table of the sixknobs. The engine-side knobs the source branch listed there are not on
devyet, so I leftthem out; their PRs add the rows.
SIRIUS_CN_DUMP_FRAGMENTSandSIRIUS_CN_TRANSLATE_ONLYareon
devalready and stay undocumented here; this PR covers the transport registry only.I made the fields and
Tunables::getpub: the crate is a library, so public items are not deadcode, and
clippy -D warningsstays green without anallow(dead_code)a later PR must remove.Configuration changes. Six new env vars, defaults equal to the constants the source branch had
hardcoded:
SIRIUS_CN_RPC_TIMEOUT_SECS(60, in [1, 3600]),SIRIUS_CN_NIXL_XFER_TIMEOUT_SECS(30, in [1, 3600]),
SIRIUS_CN_NIXL_CANARY_BYTES(16 MiB, in [1 MiB, 1 GiB]),SIRIUS_CN_NIXL_CANARY_FLOOR_GBPS(2.0, in [0, 10000], 0 disables),SIRIUS_CN_NIXL_WARMUP_TIMEOUT_SECS(180, in [1, 3600]),SIRIUS_CN_NIXL_WARMUP_EXPECT_PEERS(0 means no early exit, max 4096). Each is documented on its constant and in
docs/TUNABLES.md.Tests. Ten unit tests in
tunable.rs, all in CI's--no-default-featuresjob: defaults pinnedto the old constants, a value taken, whitespace tolerated, empty read as unset,
6Orejected,out-of-range rejected instead of clamped,
NaNand negatives rejected on the float knob,0takenas the floor's off switch, expect-peers
0mapped toNone, one bad knob failing the wholeresolution. Tests that touch the environment serialize on a mutex.
How I tested it. On a GB200 box (aarch64) I ran the CI trio: cargo fmt, clippy with warnings as errors, and the CN test suite without the engine feature. All 181 tests passed, the ten new tunable tests among them. Pre-commit skips everything under experimental/, so I ran the markdown linter on docs/TUNABLES.md by hand and it came back clean.
Intentionally not handled. Nothing on
devreads these values yet, so the registry validatesand logs only. The nixl transport, the PRPC client and the session warmup land in later PRs and read
them through
Tunables::get(); this PR exists so each of those stays small, and it gates the PRPCclient PR. The two warmup switches
SIRIUS_CN_NIXL_WARMUPandSIRIUS_CN_NIXL_WARMUP_PEERSshipwith the warmup PR and should join this registry there rather than keep their raw
env::varreads.Checklist
References
aocsa/feat/pin-table-cnumbrella (draft fix(starrocks): demo don't merge #1686); supersedes that part of draft Stream fragment execution #1644.