enhance: integrate Cardinal builds with resource admission by marcelo-cjl · Pull Request #1876 · zilliztech/knowhere

marcelo-cjl · 2026-10-13T03:15:52Z

Summary

Integrate Cardinal GPU builds with explicit CUDA architectures and propagate build context and input options through wrappers.
Add backend resource reservations and a shared serial lane for builds without backend admission.

Cardinal pins will be updated after the companion v1/master adapter changes land.

Test Plan

Release integration build and Cohere 1M GPU Hybrid/PiPNN e2e passed with the workspace dependencies. Submission hooks passed.

/kind feature

alexanderguzhva · 2026-10-16T11:20:17Z

 // This flag indicates that the index will be lazy loaded
 constexpr uint64_t LAZY_LOAD = 1UL << 22;
+// The compiled backend can use GPU construction when requested by gpu_build.
+constexpr uint64_t GPU_BUILD = 1UL << 23;


why is additional flag needed, given that the current code already can perform a GPU-based build using cuVS?

Removed the unnecessary flag.

mergify · 2026-10-16T18:26:39Z

@marcelo-cjl e2e jenkins job failed, comment /run-e2e can trigger the job again.

liliu-z

Both pinned Cardinal adapters (v1 v2.5.112, v2 fd18410) still override Build/BuildAsync with the old signatures, so main won't compile with WITH_CARDINAL=ON after this lands, and CI here doesn't build Cardinal to catch it. Any Milvus knowhere bump or unrelated Cardinal pin bump in between would hit that. Could we bump both pins in this PR once the adapter changes (e.g. zilliztech/cardinal#985) are merged?

liliu-z · 2026-10-19T12:34:28Z

+if(WITH_CUVS OR (WITH_CARDINAL AND WITH_GPU))
+  if(NOT DEFINED CMAKE_CUDA_ARCHITECTURES OR CMAKE_CUDA_ARCHITECTURES STREQUAL "")
+    if(WITH_CARDINAL AND WITH_GPU)
+      set(CMAKE_CUDA_ARCHITECTURES native)


Defaulting to native fails on build hosts without a GPU (most image builders), and otherwise ties the binary to whatever GPU the builder has. It also replaces RAPIDS for cuVS when both are on. Maybe just require CMAKE_CUDA_ARCHITECTURES when WITH_GPU is on and fail fast if it's missing?

Confirmed with a minimal CUDA target using this architecture-selection block: with no visible GPU, the default native setting failed to build (nvcc fatal: Unsupported gpu architecture 'compute_No CUDA devices found.' in my environment), while an explicit target architecture built successfully. Could we require CMAKE_CUDA_ARCHITECTURES for Cardinal GPU builds and report a clear configuration error when it is missing, or provide a documented set of supported target architectures? native could remain an explicit choice for local builds.

Cardinal GPU builds now require an explicit architecture, which may be native; cuVS-only still defaults to RAPIDS.

liliu-z · 2026-10-19T12:34:28Z


    Status
-    Build(const DataSetPtr dataset, std::shared_ptr<Config> cfg, bool use_knowhere_build_pool) override;
+    Build(const DataSetPtr dataset, std::shared_ptr<Config> cfg, bool use_knowhere_build_pool,


op_context is forwarded to the base node here, but TryReserveBuild isn't, so MRL always returns the default true. If the base is a Cardinal GPU node, the caller gets admitted without any reservation. Not sure that combination exists yet, but if it does, MRL should forward it with the prefix dim, same as Build.

Please forward TryReserveBuild through both MRLIndexNode and IndexNodeDataMockWrapper. Both currently inherit the default implementation that returns true, so a wrapped backend requiring a reservation can be admitted without consulting its resource availability. MRL should use the same effective prefix dimension and configuration as the base build; the data-type wrapper should preserve the underlying build's shape/type contract. Reservation and construction must reach the same underlying node. Please also add a regression test with a fake backend that returns false: the wrapped call should reach that backend and propagate false rather than report admission.

MRL and data-type wrappers now forward TryReserveBuild to the build node, with MRL using the prefix dimension.

liliu-z · 2026-10-19T12:34:28Z

    float query_data[] = {0.0f, 0.0f, 0.0f, 0.0f};
    auto base = knowhere::GenDataSet(2, kSourceDim, base_data);
    auto query = knowhere::GenDataSet(1, kSourceDim, query_data);
+    milvus::OpContext op_context;


These pass &op_context but nothing checks that it reaches the node, so they'd still pass if a layer like BuildEmbList or the mock wrapper dropped it. Maybe extend BaseFlatIndexNode in test_index_node.cc to record the op_context its Build receives, and check it through sync, async and emb-list builds?

Added assertions that OpContext reaches the underlying node.

- Add backend build reservations and a shared serial lane for unreserved builds. - Forward build context and input options through wrappers and asynchronous builds. - Require explicit CUDA architectures for Cardinal while retaining cuVS-only RAPIDS defaults. - Cover wrapper reservations and operation-context forwarding. Signed-off-by: marcelo-cjl <marcelo.chen@zilliz.com>

marcelo-cjl · 2026-10-20T12:01:17Z

@liliu-z Regarding the Cardinal dependency pins: once both adapter changes land, we will update both pins and verify the WITH_CARDINAL=ON build.

foxspy

/lgtm

sre-ci-robot · 2026-10-21T04:28:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: foxspy, marcelo-cjl

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [foxspy,marcelo-cjl]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sre-ci-robot added the kind/feature label Oct 13, 2026

sre-ci-robot requested review from CLiqing and alexanderguzhva October 13, 2026 03:15

sre-ci-robot added approved size/L labels Oct 13, 2026

mergify Bot added the dco-passed label Oct 13, 2026

marcelo-cjl changed the title ~~feat: integrate Cardinal GPU index builds~~ enhance: integrate Cardinal GPU index builds Oct 13, 2026

marcelo-cjl force-pushed the codex/cardinal-gpu-build branch 2 times, most recently from 3bede70 to dd14fe9 Compare October 13, 2026 06:08

mergify Bot added the ci-passed label Oct 13, 2026

This was referenced Oct 15, 2026

[Enhancement]: Pipeline index builds with bounded memory and concurrent storage I/O milvus-io/milvus#54295

Open

enhance: pipeline index builds with ordered resource admission milvus-io/milvus#54296

Open

alexanderguzhva reviewed Oct 16, 2026

View reviewed changes

marcelo-cjl force-pushed the codex/cardinal-gpu-build branch from dd14fe9 to c424743 Compare October 16, 2026 16:24

mergify Bot removed the ci-passed label Oct 16, 2026

marcelo-cjl force-pushed the codex/cardinal-gpu-build branch from c424743 to 1581d0b Compare October 17, 2026 01:55

mergify Bot added the ci-passed label Oct 17, 2026

liliu-z reviewed Oct 19, 2026

View reviewed changes

marcelo-cjl changed the title ~~enhance: integrate Cardinal GPU index builds~~ enhance: integrate Cardinal builds with resource admission Oct 20, 2026

marcelo-cjl force-pushed the codex/cardinal-gpu-build branch from 1581d0b to 2b500fb Compare October 20, 2026 11:58

sre-ci-robot added size/XL and removed size/L labels Oct 20, 2026

mergify Bot removed the ci-passed label Oct 20, 2026

mergify Bot added the ci-passed label Oct 20, 2026

foxspy approved these changes Oct 21, 2026

View reviewed changes

sre-ci-robot assigned foxspy Oct 21, 2026

sre-ci-robot added the lgtm label Oct 21, 2026

sre-ci-robot merged commit 1c0ebeb into zilliztech:main Oct 21, 2026
15 of 16 checks passed

Conversation

marcelo-cjl commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Oct 16, 2026

Uh oh!

liliu-z left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcelo-cjl commented Oct 20, 2026

Uh oh!

foxspy left a comment

Choose a reason for hiding this comment

Uh oh!

sre-ci-robot commented Oct 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marcelo-cjl commented Oct 13, 2026 •

edited

Loading