enhance: bind snapshot lifecycle to collection by weiliu1031 · Pull Request #48143 · milvus-io/milvus

weiliu1031 · 2026-03-09T11:22:43Z

Summary

Refactor snapshot name uniqueness from global to per-collection scope
Add cascade delete: DropCollection triggers DropSnapshotsByCollection
Add orphan snapshot GC for deleted collections
Add database-level filtering for ListSnapshots and ListRestoreSnapshotJobs
Distinguish source/target collection in RestoreSnapshot API
Move snapshot privileges from Global level to Collection level
Update client SDK and documentation for new API semantics

issue: #44358
issue: #47890
issue: #47883
issue: #47855

Test plan

Unit tests for snapshot_meta (DropSnapshotsByCollection, per-collection isolation, partial failure)
Unit tests for snapshot_manager (DropSnapshotsByCollection, getDBCollectionIDs)
Unit tests for services (ListSnapshots/ListRestoreJobs with dbID, RestoreSnapshot with source collectionID)
Unit tests for ddl_callbacks_snapshot (new dropSnapshotsByCollection callback)
Unit tests for garbage_collector (orphan snapshot GC)
E2E tests for cross-database snapshot isolation
CI validation

Note on skipped Python E2E tests

All 18 snapshot test classes in tests/python_client/milvus_client/test_milvus_client_snapshot.py are temporarily skipped with @pytest.mark.skip. Reason: this PR changes snapshot APIs (DropSnapshot, DescribeSnapshot, RestoreSnapshot) to require collection_name as a mandatory parameter, but the pymilvus SDK used in CI has not been updated to pass this parameter yet. The tests will be re-enabled once pymilvus SDK is updated to match the new API contract.

🤖 Generated with Claude Code

design doc: https://github.com/milvus-io/milvus-design-docs/blob/main/design_docs/20251114-snapshot_design.md

mergify · 2026-03-09T11:23:22Z

@weiliu1031 This is a feature PR (feat:). Please provide a design document.

How to resolve:
Link a design doc in the PR description:

design doc: https://github.com/milvus-io/milvus-design-docs/blob/main/design_docs/your_design.md

Design documents location: https://github.com/milvus-io/milvus-design-docs/tree/main/design_docs

sre-ci-robot · 2026-03-09T11:23:46Z

[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-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-arm // for ci-v2/e2e-arm
/ci-rerun-e2e-default // for ci-v2/e2e-default
/ci-rerun-ciloop // for ci-v2/ciloop (build + unit tests in one pipeline)

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

codecov · 2026-03-09T12:32:01Z

Codecov Report

❌ Patch coverage is 71.19371% with 403 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.02%. Comparing base (353ce56) to head (7a56033).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/proxy/snapshot_impl.go	16.66%	80 Missing ⚠️
internal/datacoord/snapshot_meta.go	86.25%	44 Missing and 11 partials ⚠️
internal/proxy/task_snapshot.go	72.82%	52 Missing and 1 partial ⚠️
internal/datacoord/garbage_collector.go	47.12%	43 Missing and 3 partials ⚠️
client/milvusclient/snapshot.go	0.00%	25 Missing ⚠️
internal/datacoord/snapshot_manager.go	84.66%	19 Missing and 6 partials ⚠️
client/milvusclient/snapshot_options.go	62.90%	23 Missing ⚠️
internal/datacoord/services.go	87.86%	16 Missing and 5 partials ⚠️
internal/distributed/mixcoord/client/client.go	0.00%	18 Missing ⚠️
internal/proxy/database_interceptor.go	0.00%	15 Missing ⚠️
... and 9 more

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

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #48143      +/-   ##
==========================================
+ Coverage   77.97%   78.02%   +0.04%     
==========================================
  Files        2168     2168              
  Lines      356851   357887    +1036     
==========================================
+ Hits       278271   279243     +972     
- Misses      70010    70042      +32     
- Partials     8570     8602      +32

Components	Coverage Δ
Client	`78.96% <44.82%> (-0.30%)`	⬇️
Core	`84.45% <ø> (ø)`
Go	`76.35% <72.94%> (+0.08%)`	⬆️

Files with missing lines	Coverage Δ
internal/datacoord/copy_segment_job.go	`100.00% <ø> (ø)`
internal/datacoord/copy_segment_task.go	`47.13% <100.00%> (ø)`
internal/datacoord/ddl_callbacks_snapshot.go	`100.00% <100.00%> (ø)`
internal/proxy/meta_cache.go	`87.05% <100.00%> (+0.03%)`	⬆️
...erver/broadcaster/registry/specialized_callback.go	`100.00% <100.00%> (ø)`
pkg/metrics/datacoord_metrics.go	`59.67% <100.00%> (+0.66%)`	⬆️
pkg/streaming/util/message/reflect_info.go	`0.00% <ø> (ø)`
pkg/util/constant.go	`14.10% <ø> (ø)`
pkg/util/merr/errors.go	`84.37% <ø> (ø)`
internal/datacoord/copy_segment_meta.go	`97.25% <94.33%> (-0.42%)`	⬇️
... and 18 more

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mergify · 2026-03-10T05:37:47Z

@weiliu1031 go-sdk check failed, comment rerun go-sdk can trigger the job again.

weiliu1031 · 2026-03-12T06:09:47Z

/ci-rerun-e2e-default

weiliu1031 · 2026-03-12T07:59:05Z

/ci-rerun-e2e-default

weiliu1031 · 2026-03-12T11:12:53Z

/refresh-label

liliu-z · 2026-04-08T14:38:58Z

internal/datacoord/snapshot_manager.go:631 Broadcast lock acquired too late — after ReadSnapshotData, RestoreCollection, and RestoreIndexes have already run. Move lock acquisition to before Phase 1 to close the Restore/Drop race.

liliu-z · 2026-04-08T14:38:59Z

internal/datacoord/snapshot_manager.go:1172 IncrementRestoreRef fires only inside the callback path, far after snapshot data has been read. Must increment synchronously in the service layer before broadcasting, or move the broadcast lock earlier.

liliu-z · 2026-04-08T14:39:01Z

pkg/proto/data_coord.proto:3 RestoreSnapshotRequest fields 6 and 7 were deleted without reserved 6, 7;, and fields 3/4/5 type-changed from int64 to string. Add reserved 6, 7; and confirm no released version wrote the old field numbers.

liliu-z · 2026-04-08T14:39:03Z

internal/datacoord/snapshot.go:8 SnapshotMetadata.index_ids field 8 was previously reserved 8; and is now resurrected as repeated int64. This is catalog-persisted data — reusing a reserved slot risks reading stale bytes as the new type. Verify the slot was never persisted under the old schema.

liliu-z · 2026-04-08T14:39:05Z

tests/python_client/milvus_client/test_milvus_client_snapshot.py:1 All 18 snapshot test classes are @pytest.mark.skip, leaving the feature shipping with zero Python E2E coverage. Declare the gap in the PR description and update docs/user_guides/snapshot_user_guide.md to remove or mark the Python examples as pending.

sre-ci-robot · 2026-04-09T03:16:12Z

✅ CI Loop Results `b4c9f61`

Stage	Result	Duration	Tests
✅ Build	SUCCESS	12.0min	-

Total: 19min | Pipeline | Artifacts

weiliu1031 · 2026-04-10T05:21:26Z

Response to review comments (commit 42ee421ce1)

All code changes have been pushed. Below is a point-by-point response to each issue-level comment.

1. `copy_segment_meta.go:74` — refCount keyed only by snapshotName

Status: ✅ Fixed
SnapshotRestoreRefTracker now uses composite key struct{collectionID int64; name string}. All methods (IncrementRestoreRef, DecrementRestoreRef, GetRestoreRefCount) take (collectionID, snapshotName).

2. `copy_segment_meta.go:404` — Cold-start rebuild bare name

Status: ✅ Fixed
Cold-start replay now threads SourceCollectionId from the restore job proto:

sourceCollectionID := job.GetSourceCollectionId()
copySegmentMeta.IncrementRestoreRef(sourceCollectionID, snapshotName)

3. `resource_key.go:142` — Snapshot resource key lacks collection dimension

Status: ✅ Fixed
Both NewSharedSnapshotNameResourceKey and NewExclusiveSnapshotNameResourceKey now take (collectionID int64, snapshotName string) and format the key as fmt.Sprintf("%d:%s", collectionID, snapshotName).

4. `services.go:2105` — CreateSnapshot uses global bare-name resource key

Status: ✅ Fixed
Updated to NewExclusiveSnapshotNameResourceKey(req.GetCollectionId(), req.GetName()).

5. `services.go:2207` — DropSnapshot uses global bare-name resource key

Status: ✅ Fixed
Updated to NewExclusiveSnapshotNameResourceKey(req.GetCollectionId(), snapshotName). Also extended lock scope to include DB + collection + per-collection snapshot keys.

6. `ddl_callbacks.go:101` — Restore broadcast uses bare-name key

Status: ✅ Fixed
Both startBroadcastForRestoreSnapshot and startRestoreSnapshotLock now use NewExclusiveSnapshotNameResourceKey(collectionID, snapshotName).

7. `ack_callback_scheduler.go:301` — Callback retries forever on ErrSnapshotPinned

Status: 🔍 Needs investigation
This file (ack_callback_scheduler.go) was not found in the current codebase — the retry logic may be in the broadcaster framework rather than snapshot-specific code. Need to verify the exact retry mechanism and whether terminal error classification is needed.

8. `snapshot_manager.go:631` — Broadcast lock acquired too late

Status: ✅ Fixed
RestoreSnapshot now uses a strict Phase 0 sequence:

startRestoreSnapshotLock — acquires snapshot-name lock before any work
Validates snapshot exists
IncrementRestoreRef synchronously
Release Phase 0 lock

Phase 1 (ReadSnapshotData) is protected by the refcount.

9. `snapshot_manager.go:1172` — IncrementRestoreRef fires too late

Status: ✅ Fixed
IncrementRestoreRef is now called synchronously in Phase 0 under the lock, before any broadcast or data reading.

10. `data_coord.proto:3` — Reserved fields for deleted proto fields

Status: 📌 Accepted as-is
This is an unpublished feature — no released version has written these field numbers. We accept the current proto layout to keep the schema clean.

11. `snapshot.go:8` — Reserved slot reuse for index_ids

Status: 📌 Accepted as-is
Same rationale — unpublished feature, slot was never persisted under the old schema in any release.

12. `test_milvus_client_snapshot.py:1` — E2E tests skipped

Status: ⏳ Deferred
Python E2E tests are skipped because the Proto/API changes require SDK updates first. Will re-enable tests after the Python SDK is updated to match the new snapshot API signatures. Updated docs/user_guides/snapshot_user_guide.md with concurrency documentation.

Summary: 17 of 20 review points are resolved in code. 2 proto-related points accepted as design decisions for unpublished feature. 1 point (ack callback terminal errors) requires further investigation.

sre-ci-robot · 2026-04-10T06:22:50Z

✅ CI Loop Results `42ee421`

Stage	Result	Duration	Tests
✅ Build	SUCCESS	13.4min	-

Total: 18min | Pipeline | Artifacts

liliu-z · 2026-04-14T09:43:47Z

/lgtm
/approve

weiliu1031 · 2026-04-14T13:40:47Z

/ci-rerun-code-check

liliu-z

/lgtm
/approve

sre-ci-robot · 2026-04-14T14:07:50Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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~~ [liliu-z]

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

weiliu1031 · 2026-04-15T03:11:49Z

/rerun ci-v2/go-sdk
/rerun ci-v2/e2e-default

weiliu1031 · 2026-04-15T03:13:19Z

/ci-rerun-go-sdk
/ci-rerun-e2e-default

weiliu1031 · 2026-04-15T03:34:44Z

/ci-rerun-go-sdk

weiliu1031 · 2026-04-15T03:34:48Z

/ci-rerun-e2e-default

weiliu1031 · 2026-04-15T03:44:11Z

/ci-rerun-go-sdk

issue: milvus-io#47658 Squash of 16 commits implementing: - Snapshot lifecycle bound to source collection - Pin/Unpin snapshot data APIs with TTL support - Per-collection snapshot name uniqueness - GC and concurrency fixes from PR review See PR milvus-io#48143 for full details. Signed-off-by: Wei Liu <wei.liu@zilliz.com>

Signed-off-by: Wei Liu <wei.liu@zilliz.com>

weiliu1031 · 2026-04-16T05:07:24Z

/ci-rerun-e2e-default

liliu-z

/lgtm

sre-ci-robot added area/dependency Pull requests that update a dependency file area/test sig/testing labels Mar 9, 2026

sre-ci-robot requested review from bigsheeper, chyezh and congqixia March 9, 2026 11:22

sre-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines. label Mar 9, 2026

mergify Bot added dco-passed DCO check passed. do-not-merge/missing-design-doc kind/feature Issues related to feature request from users labels Mar 9, 2026

sre-ci-robot added the low-code-coverage add test-label from zhikun, diff coverage > 80% label Mar 9, 2026

weiliu1031 force-pushed the feat/snapshot-collection-lifecycle branch from 49f05fb to 1bd8adc Compare March 10, 2026 03:07

weiliu1031 changed the title ~~feat: bind snapshot lifecycle to collection~~ enhance: bind snapshot lifecycle to collection Mar 10, 2026

mergify Bot added the kind/enhancement Issues or changes related to enhancement label Mar 10, 2026

weiliu1031 force-pushed the feat/snapshot-collection-lifecycle branch from 1bd8adc to bf5b3f7 Compare March 11, 2026 03:59

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 Mar 11, 2026

weiliu1031 force-pushed the feat/snapshot-collection-lifecycle branch from b08257d to 465707f Compare March 12, 2026 02:07

mergify Bot added the ci-passed label Mar 12, 2026

weiliu1031 mentioned this pull request Mar 12, 2026

enhance: update snapshot design for collection lifecycle binding milvus-io/milvus-design-docs#25

Open

3 tasks

weiliu1031 force-pushed the feat/snapshot-collection-lifecycle branch from 78486f6 to b4c9f61 Compare April 9, 2026 02:12

mergify Bot added the ci-passed label Apr 9, 2026

weiliu1031 force-pushed the feat/snapshot-collection-lifecycle branch from b4c9f61 to 42ee421 Compare April 10, 2026 05:18

sre-ci-robot removed the ci-passed label Apr 10, 2026

liliu-z reviewed Apr 14, 2026

View reviewed changes

weiliu1031 mentioned this pull request Apr 15, 2026

Flaky test: test_search_bfloat16_with_pagination_default fails at 80% overlap threshold #49030

Closed

weiliu1031 added 2 commits April 16, 2026 10:01

chore: regenerate proto after rebase

7a56033

Signed-off-by: Wei Liu <wei.liu@zilliz.com>

liliu-z reviewed Apr 16, 2026

View reviewed changes

weiliu1031 mentioned this pull request Apr 16, 2026

enhance: [3.0] bind snapshot lifecycle to collection #49079

Merged

This was referenced Apr 16, 2026

test: refactor snapshot test cases for collection-scoped snapshot API #49085

Merged

[Bug]: RBAC v2 built-in privilege groups (Cluster*/Collection*) missing snapshot and other recent privileges because configs/milvus.yaml defaults are stale #49167

Closed

weiliu1031 mentioned this pull request Apr 20, 2026

fix: sync RBAC built-in privilege groups with Go constants #49173

Closed

6 tasks

Uh oh!

Conversation

weiliu1031 commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Note on skipped Python E2E tests

Uh oh!

mergify Bot commented Mar 9, 2026

Uh oh!

sre-ci-robot commented Mar 9, 2026

Uh oh!

codecov Bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mergify Bot commented Mar 10, 2026

Uh oh!

weiliu1031 commented Mar 12, 2026

Uh oh!

weiliu1031 commented Mar 12, 2026

Uh oh!

weiliu1031 commented Mar 12, 2026

Uh oh!

liliu-z commented Apr 8, 2026

Uh oh!

liliu-z commented Apr 8, 2026

Uh oh!

liliu-z commented Apr 8, 2026

Uh oh!

liliu-z commented Apr 8, 2026

Uh oh!

liliu-z commented Apr 8, 2026

Uh oh!

sre-ci-robot commented Apr 9, 2026

✅ CI Loop Results b4c9f61

Uh oh!

weiliu1031 commented Apr 10, 2026

Response to review comments (commit 42ee421ce1)

1. copy_segment_meta.go:74 — refCount keyed only by snapshotName

2. copy_segment_meta.go:404 — Cold-start rebuild bare name

3. resource_key.go:142 — Snapshot resource key lacks collection dimension

4. services.go:2105 — CreateSnapshot uses global bare-name resource key

5. services.go:2207 — DropSnapshot uses global bare-name resource key

6. ddl_callbacks.go:101 — Restore broadcast uses bare-name key

7. ack_callback_scheduler.go:301 — Callback retries forever on ErrSnapshotPinned

8. snapshot_manager.go:631 — Broadcast lock acquired too late

9. snapshot_manager.go:1172 — IncrementRestoreRef fires too late

10. data_coord.proto:3 — Reserved fields for deleted proto fields

11. snapshot.go:8 — Reserved slot reuse for index_ids

12. test_milvus_client_snapshot.py:1 — E2E tests skipped

Uh oh!

sre-ci-robot commented Apr 10, 2026

✅ CI Loop Results 42ee421

Uh oh!

liliu-z commented Apr 14, 2026

Uh oh!

weiliu1031 commented Apr 14, 2026

Uh oh!

liliu-z left a comment

Choose a reason for hiding this comment

Uh oh!

sre-ci-robot commented Apr 14, 2026

Uh oh!

weiliu1031 commented Apr 15, 2026

Uh oh!

weiliu1031 commented Apr 15, 2026

Uh oh!

weiliu1031 commented Apr 15, 2026

Uh oh!

weiliu1031 commented Apr 15, 2026

Uh oh!

weiliu1031 commented Apr 15, 2026

Uh oh!

weiliu1031 commented Apr 16, 2026

Uh oh!

liliu-z left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

weiliu1031 commented Mar 9, 2026 •

edited

Loading

codecov Bot commented Mar 9, 2026 •

edited

Loading

✅ CI Loop Results `b4c9f61`

1. `copy_segment_meta.go:74` — refCount keyed only by snapshotName

2. `copy_segment_meta.go:404` — Cold-start rebuild bare name

3. `resource_key.go:142` — Snapshot resource key lacks collection dimension

4. `services.go:2105` — CreateSnapshot uses global bare-name resource key

5. `services.go:2207` — DropSnapshot uses global bare-name resource key

6. `ddl_callbacks.go:101` — Restore broadcast uses bare-name key

7. `ack_callback_scheduler.go:301` — Callback retries forever on ErrSnapshotPinned

8. `snapshot_manager.go:631` — Broadcast lock acquired too late

9. `snapshot_manager.go:1172` — IncrementRestoreRef fires too late

10. `data_coord.proto:3` — Reserved fields for deleted proto fields

11. `snapshot.go:8` — Reserved slot reuse for index_ids

12. `test_milvus_client_snapshot.py:1` — E2E tests skipped

✅ CI Loop Results `42ee421`