docs(kvbm): drop the object-storage tier that has no implementation - #14012
docs(kvbm): drop the object-storage tier that has no implementation#14012ayaangazali wants to merge 1 commit into
Conversation
The KVBM configuration reference documents seven DYN_KVBM_OBJECT_* variables
and states that the G4 tier stores blocks in an S3-compatible object store,
down to a {worker_id} bucket template and credential handling. Nothing in the
runtime reads any of them. Their only appearances are the constants in
environment_names.rs and this page.
Someone following the page would supply S3 credentials and expect KV blocks to
offload, and get silence: there is no reader to warn, so the tier just never
does anything.
Remove the section and the two intro claims that promise the tier, and note in
its place that object storage has no environment surface yet, so the page does
not simply lose the concept.
Signed-off-by: ayaangazali <ayaangazali.work@gmail.com>
|
👋 Hi ayaangazali! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
Bot
added
docs
external-contribution
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. WalkthroughThe KVBM reference now documents cache tiers through disk (G3). It states that object storage (G4) is not read from runtime environment variables and removes its configuration section and parameters. ChangesKVBM configuration documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This PR removes documentation for an unimplemented object-storage tier and replaces it with a clarifying note; it does not change runtime behavior or production configuration, so no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides detailed context, implementation scope, reviewer guidance, and validation results. It does not include the required Related Issues section or confirm that no related issue exists. Resolution Add the required Related Issues section. Either link the relevant issue, such as Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
Comment |
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
Summary
The KVBM configuration reference documents an object-storage tier that is not implemented. The section opens with
and then documents seven variables, including a
{worker_id}bucket template and access/secret key handling. Nothing in the runtime reads any of them. Across the whole repository the only appearances ofDYN_KVBM_OBJECT_*are the constants inenvironment_names.rsand this page:There is also no prefix-based loader that could pick them up indirectly.
config.rsregistersEnv::prefixedforDYN_WORKER_,DYN_RUNTIME_,DYN_SYSTEM_,DYN_COMPUTE_,DYN_HEALTH_CHECK_andDYN_CANARY_, and none forDYN_KVBM_.The failure mode is quiet, which is what makes it worth fixing. Someone follows the page, puts real S3 credentials in a worker manifest, and gets no offload and no warning, because there is no reader to complain.
Removed the section and the two intro claims that promise the tier, and left a short note in its place so the page does not silently lose the concept.
Two things I deliberately did not do:
environment_names.rsalone. feat(kvbm-engine): integrate Mooncake Store as G4 object storage backend #9786 is actively building this tier, and it takes a different approach (anObjectConfigserde struct in the newkvbm-configcrate rather than environment variables), so whether an environment surface comes back is that author's call, not mine.Validation
Documentation only, no code changes.
pre-commit run --files docs/fern/pages/reference/components/kvbm-configuration.mdxpasses every applicable hook, including the Fern asset-path check.pytest-marker-reportfails identically on an untouchedREADME.mdhere because vLLM and TensorRT-LLM are not installed.grep -rn "DYN_KVBM_OBJECT" docs/ agent-docs/is now empty, so no other page contradicts this one.<Note>is used on 54 other Fern pages, so the component is the standard one for this site.Summary by CodeRabbit