Skip to content

fix: [3.0] update Knowhere for stable IndexNode ABI - #52755

Merged
sre-ci-robot merged 1 commit into
milvus-io:3.0from
marcelo-cjl:codex/3.0-update-knowhere-abi-fix
Aug 22, 2026
Merged

fix: [3.0] update Knowhere for stable IndexNode ABI#52755
sre-ci-robot merged 1 commit into
milvus-io:3.0from
marcelo-cjl:codex/3.0-update-knowhere-abi-fix

Conversation

@marcelo-cjl

Copy link
Copy Markdown
Contributor

pr: #52754
issue: #52723
issue: #52724
issue: #52725

Cherry-pick from master.

What

Why

In a Cardinal-enabled Milvus build, Knowhere translation units define KNOWHERE_WITH_CARDINAL, while Milvus core consumers of the same public header do not. The previous conditional BuildAsync() declaration therefore gave the two DSOs different IndexNode vtable layouts.

Calls intended for GetIdMap() could dispatch to Count() instead and interpret its integer return as an IdMap&, causing the SIGSEGVs reported in #52723, #52724, and #52725.

Knowhere d7cfd888 makes the public vtable independent of that feature macro.

Validation

  • No new local build or test was run for this dependency-pin-only cherry-pick; validation is delegated to Milvus PR CI.
  • The underlying Knowhere fix passed Knowhere CI and a prior Milvus 3.0 Cardinal A/B reproduction: the affected ordinary HNSW test changed from SIGSEGV/exit 139 on the old pin to 1/1 passed with the fix.

Signed-off-by: marcelo-cjl <marcelo.chen@zilliz.com>
@marcelo-cjl marcelo-cjl added this to the 3.0.1 milestone Aug 21, 2026
@sre-ci-robot sre-ci-robot added area/compilation size/XS Denotes a PR that changes 0-9 lines. labels Aug 21, 2026
@mergify mergify Bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Aug 21, 2026
@sre-ci-robot sre-ci-robot added the do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first label Aug 21, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[INFO] PR Label Summary by Default
[FAILED] PR #52754 not merged

Use /refresh-label to update related check and label manually

@sre-ci-robot

Copy link
Copy Markdown
Contributor

[ci-v2-notice]
Notice: ci-v2 system is enabled for this PR (3.0 branch).

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check-macos // for Code Checker MacOS (GitHub Actions)
  • /ci-rerun-build-ut-cov // for ci-v2/build-ut-cov (build + unit tests)
  • /ci-rerun-e2e-amd // for ci-v2/e2e-amd (e2e tests)
  • /ci-rerun-e2e-amd-gray // for ci-v2/e2e-amd-gray (TC pool shadow; non-gating)
  • /ci-rerun-e2e-dist-wp // for ci-v2/e2e-dist-wp (Tencent distributed woodpecker-service boundary)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests)
  • /ci-rerun-gosdk-std // for ci-v2/go-sdk-std (Go SDK E2E, standalone)
  • /ci-rerun-gosdk-dist-wp // for ci-v2/go-sdk-dist-wp (distributed + Woodpecker service)

If you have any questions or requests, please contact @zhikunyao.

@sre-ci-robot

Copy link
Copy Markdown
Contributor

✅ CI Loop Results 8026c7a

Stage Result Duration Tests
✅ Build SUCCESS 15.3min -
✅ Code-Check SUCCESS 10.4min -
✅ UT-Integration SUCCESS 25.3min -
✅ UT-GO SUCCESS 23.1min -
✅ UT-CPP-Cov SUCCESS 55.1min 8615 total, 8615 passed, 0 failed

Total: 79min | Pipeline | Artifacts

Overall Coverage: 37.3%

@mergify mergify Bot added the ci-passed label Aug 21, 2026

@liliu-z liliu-z left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@sre-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liliu-z

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:

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

Copy link
Copy Markdown
Contributor

[approval-status] effective-owner-approvals=1 [liliu-z(review)]; do-not-merge/disable-approve-self=not-required; do-not-merge/doc-need-two-approve=not-required; ignored=[none]

@sre-ci-robot sre-ci-robot removed the do-not-merge/need-merge-master-first any pr merge to release branch need to merge master first label Aug 22, 2026
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[INFO] PR Label Summary by Default
[SUCCESS] PR #52754 merged to master

Use /refresh-label to update related check and label manually

1 similar comment
@sre-ci-robot

Copy link
Copy Markdown
Contributor

[INFO] PR Label Summary by Default
[SUCCESS] PR #52754 merged to master

Use /refresh-label to update related check and label manually

@sre-ci-robot
sre-ci-robot merged commit bd469df into milvus-io:3.0 Aug 22, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved area/compilation ci-passed dco-passed DCO check passed. kind/bug Issues or changes related a bug lgtm size/XS Denotes a PR that changes 0-9 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants