Skip to content

fix: avoid redundant schema reopen locking in segcore#50378

Open
congqixia wants to merge 1 commit into
milvus-io:masterfrom
congqixia:fix/add_cheap_dup_atomic_check
Open

fix: avoid redundant schema reopen locking in segcore#50378
congqixia wants to merge 1 commit into
milvus-io:masterfrom
congqixia:fix/add_cheap_dup_atomic_check

Conversation

@congqixia

Copy link
Copy Markdown
Contributor

Related to #50103

Cache the sealed segment schema version in an atomic and use it as a fast path before taking mutex_ during reopen. Double-check the version under the write lock before publishing schema changes so stale or duplicate reopen requests return early without contending on schema updates.

Note: this patch is only a cheap fix for non-schema lock contention. the root shall be fixed after lock usage improvement

Related to milvus-io#50103

Cache the sealed segment schema version in an atomic and use it as a fast path
before taking mutex_ during reopen. Double-check the version under the write
lock before publishing schema changes so stale or duplicate reopen requests
return early without contending on schema updates.

Note: this patch is only a cheap fix for non-schema lock contention. the
root shall be fixed after lock usage improvement

Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
@sre-ci-robot sre-ci-robot added the size/M Denotes a PR that changes 30-99 lines. label Jun 8, 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/bug Issues or changes related a bug labels Jun 8, 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-code-check-macos // for Code Checker MacOS (GitHub Actions)
  • /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.

@sre-ci-robot

Copy link
Copy Markdown
Contributor

✅ CI Loop Results 71ae215

Stage Result Duration Tests
✅ Build SUCCESS 10.6min -
✅ Code-Check SUCCESS 4.8min -
✅ UT-GO SUCCESS 14.4min 1012 passed
✅ UT-Integration SUCCESS 24.0min 46 passed
✅ UT-CPP-Cov SUCCESS 48.6min 7835 passed

Total: 64min | Pipeline | Artifacts

Overall Coverage: 71.3%
Diff Coverage: CPP 83.3% (25 hit, 5 miss, 30 measurable lines, 18 unmeasured)
Diff Coverage HTML: view changed lines
Total Patch Coverage: 83.3% (25/30 measurable lines, 18 unmeasured)

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.96%. Comparing base (43010a4) to head (71ae215).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
...rnal/core/src/segcore/ChunkedSegmentSealedImpl.cpp 87.50% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #50378      +/-   ##
==========================================
- Coverage   79.01%   78.96%   -0.06%     
==========================================
  Files        2239     2239              
  Lines      396634   396645      +11     
==========================================
- Hits       313391   313197     -194     
- Misses      73703    73865     +162     
- Partials     9540     9583      +43     
Components Coverage Δ
Client 80.59% <ø> (ø)
Core 85.95% <87.50%> (-0.01%) ⬇️
Go 76.82% <ø> (-0.08%) ⬇️
Files with missing lines Coverage Δ
...ternal/core/src/segcore/ChunkedSegmentSealedImpl.h 72.14% <ø> (ø)
...rnal/core/src/segcore/ChunkedSegmentSealedImpl.cpp 66.14% <87.50%> (+0.02%) ⬆️

... and 47 files with indirect coverage changes

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

@sre-ci-robot sre-ci-robot added low-code-coverage add test-label from zhikun, diff coverage > 80% and removed low-code-coverage add test-label from zhikun, diff coverage > 80% labels Jun 8, 2026
@congqixia

Copy link
Copy Markdown
Contributor Author

/ci-rerun-e2e-default

@mergify mergify Bot added the ci-passed label Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved ci-passed dco-passed DCO check passed. kind/bug Issues or changes related a bug size/M Denotes a PR that changes 30-99 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants