Comments say snapshot shard transfer uses `PartialSnapshot`, but the code uses `Recovery`
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 1/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 86/100
Research direction
Read the cited comments in lib/collection/src/shards/transfer/snapshot.rs, src/common/snapshots.rs, and lib/collection/src/shards/replica_set/snapshots.rs, then check the test at lib/collection/src/shards/replica_set/snapshots/tests.rs. The issue explains that start_shard_transfer uses Recovery; update the outdated state references and set the test target to ReplicaState::Recovery. Done when the comments and test match the state used by real transfers.
Written by the indexing model from the issue text.
Description
Current Behavior
Several comments say that the receiving replica of a snapshot shard transfer is in the PartialSnapshot state:
transfer_snapshotdoc comment and its sequence diagram:lib/collection/src/shards/transfer/snapshot.rsL31-L32, L47, L87, L118-L120- why it is safe to clear the local shard before downloading the snapshot:
src/common/snapshots.rsL206 clear_local_for_snapshot_recovery:lib/collection/src/shards/replica_set/snapshots.rsL425, L436- a test that puts the target replica in
PartialSnapshot"because that is where the receiving replica of a snapshot transfer sits":lib/collection/src/shards/replica_set/snapshots/tests.rsL136-L138
The code does something else. Since #3847 (Qdrant 1.9), Collection::start_shard_transfer sets the receiving replica to Recovery for both the Snapshot and WalDelta methods (shard_transfer.rs L140). ReplicaState::PartialSnapshot is marked as deprecated since 1.9.0 (replica_set_state.rs L113).
Line numbers are from dev at 23320c2c8.
Expected Behavior
The comments name the state the code actually uses (Recovery), and the test sets the same state that a real transfer uses.
Fixed by
PR #11310 fixes this issue
Possible Solution
Replace PartialSnapshot with Recovery in those comments, and set ReplicaState::Recovery in the test. Both states return false from can_be_source_of_truth(), so the test checks the same thing as before.
Context (Environment)
I found this while reading the shard replication path (replicate_shard, snapshot method) to understand how a new replica gets its data.
- Dominant language
- Rust
- Stars
- 35.2k
- Forks
- 2.8k
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 284
Getting set up
- Ships a Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from qdrant/qdrant
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
Maintainers usually reply within 1 day
-
mmr in a leaf prefetch is validated, then silently discardedPossibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
qdrant/qdrant#11002 · 2 comments ·
Maintainers usually reply within 1 day
-
Renaming an alias to the name of an existing collection is accepted and redirects that collection's requestsMay be free again A pull request for this issue was closed without being merged. Openarea/storage bug to be released
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Maintainers usually reply within 1 day
-
`grpc_responses_total` is missing for DeleteVectors, CreateFieldIndex, DeleteFieldIndex, SearchMatrixPairs, SearchMatrixOffsetsPossibly taken A pull request linked to this issue is open or already merged. Openaccepted area/observability bug
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 1 day
-
`qdrant-edge-py`: `MatchValue(value=True)` is converted to integer `1`, so boolean filters never matchPossibly taken A pull request linked to this issue is open or already merged. Openbug
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
Maintainers usually reply within 1 day
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
farion1231/cc-switch#8829 ·
Maintainers usually reply within 1 day
-
bug ci good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 92/100
longbridge/gpui-kit#3677 ·
Maintainers usually reply within 1 day
-
E-pr-welcome
Difficulty 2/5 1-3 hours Newbie friendliness 77/100
seanmonstar/reqwest#3121 · 2 comments ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 68/100
ilya-zlobintsev/LACT#1235 ·
Maintainers usually reply within 1 day