Skip to content

enhance: use StorageVersion to identify v3 segments and fix stats task atomicity#48996

Open
congqixia wants to merge 1 commit intomilvus-io:masterfrom
congqixia:enhance/loon/cleanup_v3_verification
Open

enhance: use StorageVersion to identify v3 segments and fix stats task atomicity#48996
congqixia wants to merge 1 commit intomilvus-io:masterfrom
congqixia:enhance/loon/cleanup_v3_verification

Conversation

@congqixia
Copy link
Copy Markdown
Contributor

Related to #48547

Replace ManifestPath != "" checks with StorageVersion-based checks across Go and C++ code to use the explicit version identifier instead of an indirect side-effect. This makes version gating clearer and decoupled from the manifest path value itself.

Fix statsTask.SetJobInfo to apply manifest path and stats/bm25/text/json operators in a single atomic UpdateSegment call, eliminating a race window where manifest could be updated without stats (or vice versa). The previous code used separate UpdateSegment and UpdateSegmentsInfo calls that could partially fail.

Also fix SetManifestPath SegmentOperator which had a self-comparison bug (segment.GetManifestPath() == segment.GetManifestPath()) causing it to always no-op.

@sre-ci-robot sre-ci-robot requested review from aoiasd and czs007 April 13, 2026 15:15
@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Apr 13, 2026
@sre-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia

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

@mergify mergify bot added dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement labels Apr 13, 2026
@sre-ci-robot
Copy link
Copy Markdown
Contributor

[ci-v2-notice]
Notice: New ci-v2 system is enabled for this PR.

To rerun ci-v2 checks, comment with:

  • /ci-rerun-code-check // for ci-v2/code-check
  • /ci-rerun-build // for ci-v2/build
  • /ci-rerun-build-all // for ci-v2/build-all (multi-arch builds)
  • /ci-rerun-buildenv // for ci-v2/build-env (build milvus-env builder images)
  • /ci-rerun-ut-integration // for ci-v2/ut-integration, will rerun ci-v2/build
  • /ci-rerun-ut-go // for ci-v2/ut-go, will rerun ci-v2/build
  • /ci-rerun-ut-cpp // for ci-v2/ut-cpp
  • /ci-rerun-ut // for all ci-v2/ut-integration, ci-v2/ut-go, ci-v2/ut-cpp, will rerun ci-v2/build
  • /ci-rerun-e2e-default // for ci-v2/e2e-default
  • /ci-rerun-e2e-amd // for ci-v2/e2e-amd (e2e pool dispatcher)
  • /ci-rerun-build-ut-cov // for ci-v2/build-ut-cov (build + unit tests in one pipeline)
  • /ci-rerun-gosdk // for ci-v2/go-sdk (Go SDK E2E tests, ARM)

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

…k atomicity

Related to milvus-io#48547

Replace ManifestPath != "" checks with StorageVersion-based checks across
Go and C++ code to use the explicit version identifier instead of an
indirect side-effect. This makes version gating clearer and decoupled
from the manifest path value itself.

Fix statsTask.SetJobInfo to apply manifest path and stats/bm25/text/json
operators in a single atomic UpdateSegment call, eliminating a race
window where manifest could be updated without stats (or vice versa).
The previous code used separate UpdateSegment and UpdateSegmentsInfo
calls that could partially fail.

Also fix SetManifestPath SegmentOperator which had a self-comparison bug
(segment.GetManifestPath() == segment.GetManifestPath()) causing it to
always no-op.

Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
@congqixia congqixia force-pushed the enhance/loon/cleanup_v3_verification branch from 86ecde6 to f9cb0ae Compare April 13, 2026 23:39
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.44%. Comparing base (f3aacd1) to head (f9cb0ae).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
internal/core/src/segcore/SegmentLoadInfo.cpp 0.00% 3 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #48996       +/-   ##
===========================================
+ Coverage   77.97%   84.44%    +6.46%     
===========================================
  Files        2167      642     -1525     
  Lines      356240   101207   -255033     
===========================================
- Hits       277783    85465   -192318     
+ Misses      69915    15742    -54173     
+ Partials     8542        0     -8542     
Components Coverage Δ
Client ∅ <ø> (∅)
Core 84.44% <0.00%> (-0.01%) ⬇️
Go ∅ <ø> (∅)
Files with missing lines Coverage Δ
internal/core/src/segcore/SegmentLoadInfo.cpp 67.09% <0.00%> (-0.22%) ⬇️

... and 1526 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-passed DCO check passed. kind/enhancement Issues or changes related to enhancement size/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants