feat: add gated synchronous LoadCollection warmup - #17
Draft
sunby wants to merge 2 commits into
Draft
Conversation
Native and end-to-end validation remain pending. Signed-off-by: sunby <sunbingyi1992@gmail.com>
This was referenced Sep 8, 2026
Signed-off-by: sunby <sunbingyi1992@gmail.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.
Summary
Add an opt-in
LoadCollectionparameter,load_params["warmup"]="sync", with Go SDKWithSyncWarmup()and REST V2 forwarding. Omission keeps existing behavior. Explicit sync rejects refresh, external collections, and in-place enablement on an ordinarily loaded/loading collection; release first.Persist a separate sync-warmup policy and durable epoch until ReleaseCollection, carry them through load configuration, WAL, query views and segment load information, and qualify physical/transform-loaded fast paths by the successful load epoch. Add stale-view/config-version guards and target-based vchannel × replica progress accounting.
Wire the native force flag into selected field/index/text/JSON-stat/system-field warmup paths, including both outer and inner index warmup settings. This does not change global warmup, lazy or mmap settings, expand selected load fields, or provide a cache-residency guarantee.
Context propagation is unchanged from the PR base. Reuse the existing
JsonKeyStats::Load, column-group/helper signatures and cache-slot call sites; force sync uses the existing synchronous execution mechanism. Task-level checks that reject canceled load/update completions remain. Extending native context propagation or prompt I/O cancellation is a separate follow-up, not part of this feature.Rollout boundary
Draft: native and end-to-end acceptance are still pending.
queryCoord.enableLoadCollectionSyncWarmupremains false by default. Admission and placement check QueryNode/native capabilities. A full compatible Proxy/WAL rollout is a manual prerequisite; the QueryNode capability check does not detect every older upstream component. Do not enable the gate based on these unit tests alone.Scope and references
sunby/milvus:codex/load-1m-segments-pr-stack-rebased-qv-work. This PR contains only the warmup implementation, not the already-merged query-view resolution work.Verification
Checks rerun for the context-scope reduction (
b2b2c34392):JsonKeyStats.cpp/.h,ChunkedColumnGroup.handChunkedSegmentSealedImpl.hnow exactly match the PR base.ChunkedSegmentSealedImpl.cppkeeps only three force-policy hunks; no newly added native context forwarding remains.ChunkedSegmentSealedImpl.cppandJsonKeyStats.cpppassed-fsyntax-onlyusing the current LLVM 21 compile commands, with PCH disabled and OpenMP retained. This is a syntax/signature check, not a full native build or test run.run_clang_format.shpassed with clang-format 15.0.7 in an isolated worktree. Changed-line formatting was also checked. Unrelated baseline formatting is excluded.make lint-fixran gofumpt/gci, then failed in root-module type checking:internal/metastore/kv/querycoord/kv_catalog_test.go:372: undefined: mocks. The same reference exists in the base commit and this file is unchanged by the feature. The target did not reach its pkg/client lint stages; full-repository lint is not green.git diff --checkpassed.Go checks from the previous submission (not rerun for this native-only scope adjustment):
internal/views/coord/loadmgr,internal/views/coord/balancer,internal/views/coord/coordviewandinternal/views/qviews/....TestLoadCollectionSyncWarmupOptionpassed.-tags dynamic,test -gcflags="all=-N -l" -count=1.Earlier implementation checks and their exact scope are recorded in design appendix B: targeted QueryCoord policy/capability/progress, session capability, default-off configuration, proto and selected native compilation checks. They are not full native/E2E acceptance.
Known gaps / adversarial review
milvus_corebuild attempt encountered Knowhere/LLVM 18/libc++std::atomic_refincompatibility, and the chunked-segment build encountered an OpenMP structured-binding capture limitation. The current LLVM 21 syntax checks above pass with OpenMP retained; a full native build/test run was not repeated in this scope-reduction update.TestServer/TestStopreproduced a repeated-close panic;TestServer/TestUpdateAutoBalanceConfigLoopreproduced an etcd session CAS conflict during setup. Full paramtable tests also encounter local-config default overrides.make test-gohas not been run for this feature. Keep the gate closed until the acceptance matrix is completed.