[eval] Don't lock SegmentHolder for the entire duration of read operations#7
Open
Uzay-G wants to merge 5 commits intoeval/base-pr-8056from
Open
[eval] Don't lock SegmentHolder for the entire duration of read operations#7Uzay-G wants to merge 5 commits intoeval/base-pr-8056from
SegmentHolder for the entire duration of read operations#7Uzay-G wants to merge 5 commits intoeval/base-pr-8056from
Conversation
|
Review started. |
|
Review started. |
|
Review started. |
There was a problem hiding this comment.
All checks passed.
Expected behavior
SegmentHolderread lock is held only long enough to snapshot the segment list, not for the entire duration of read operations (search, retrieve, scroll, count, collection info, shard stats)- Search results remain correct after the refactor — same results, same ordering
- Point retrieval by ID returns correct payloads and vectors
- Scroll, count, and collection info endpoints produce correct results
- Concurrent reads and writes do not deadlock, corrupt data, or produce significantly degraded latency
- Error handling (non-existent collections, wrong dimensions, bad JSON) is unaffected
- The
non_appendable_then_appendableordering invariant is preserved in all collected segment snapshots
What happens
- ✅ All read operations (search, retrieve, scroll, count, collection info) return correct results after the refactor
- ✅ Concurrent reads and writes: search latency stays low (avg 11ms, p99 20ms) under continuous upsert load, with no deadlocks or data corruption
- ✅ Data consistency verified: all 9400 points correctly stored and retrievable after concurrent read/write workload
- ✅ Error handling works correctly: proper error messages for non-existent collections, wrong vector dimensions, and malformed JSON
- ✅ Empty collection edge case: search returns empty results without errors
Detailed evidence
Setup
# Built Qdrant from the PR branch (eval/upstream-pr-8056)
export PATH="/home/agent/.cargo/bin:$PATH"
CARGO_PROFILE_RELEASE_CODEGEN_UNITS=16 CARGO_PROFILE_RELEASE_LTO=false cargo build --release -j 1
# Finished `release` profile [optimized] target(s) in 38m 43s
# Started Qdrant
/home/agent/qdrant/target/release/qdrant &
# Version: 1.16.4-dev, build: 34cf12f6
curl -s http://localhost:6333/healthz
# healthz check passedFunctional correctness: search, retrieve, scroll, count
# Created collection
curl -s -X PUT http://localhost:6333/collections/test_collection \
-H 'Content-Type: application/json' \
-d '{"vectors": {"size": 4, "distance": "Cosine"}}'
# {"result":true,"status":"ok","time":0.050764601}
# Upserted 10 points with payloads
curl -s -X PUT http://localhost:6333/collections/test_collection/points \
-H 'Content-Type: application/json' \
-d '{"points": [
{"id": 1, "vector": [0.1, 0.2, 0.3, 0.4], "payload": {"city": "Berlin", "count": 10}},
{"id": 5, "vector": [0.5, 0.6, 0.7, 0.8], "payload": {"city": "NYC", "count": 50}},
... (10 points total)
]}'
# {"result":{"operation_id":1,"status":"acknowledged"},"status":"ok","time":0.000721671}
# Search — returns correct nearest neighbors
curl -s -X POST http://localhost:6333/collections/test_collection/points/search \
-H 'Content-Type: application/json' \
-d '{"vector": [0.5, 0.6, 0.7, 0.8], "limit": 3, "with_payload": true, "with_vector": true}'
# Top result: point 5 (NYC) with score=1.0 (exact match), followed by 6 (Moscow, 0.9998) and 4 (Tokyo, 0.9995)
# Retrieve by ID — correct payloads and vectors
curl -s -X POST http://localhost:6333/collections/test_collection/points \
-H 'Content-Type: application/json' \
-d '{"ids": [1, 5, 10], "with_payload": true, "with_vector": true}'
# All 3 points returned with correct payloads: Berlin/10, NYC/50, Delhi/100
# Scroll — correct pagination
curl -s -X POST http://localhost:6333/collections/test_collection/points/scroll \
-H 'Content-Type: application/json' \
-d '{"limit": 5, "with_payload": true}'
# Returns first 5 points (1-5) with next_page_offset=6
# Count — correct total
curl -s -X POST http://localhost:6333/collections/test_collection/points/count \
-H 'Content-Type: application/json' \
-d '{"exact": true}'
# {"result":{"count":10},...}
# Collection info — correct segment/point counts
curl -s http://localhost:6333/collections/test_collection
# Status: green, points_count: 10, segments_count: 2
# Search with filter
curl -s -X POST http://localhost:6333/collections/test_collection/points/search \
-H 'Content-Type: application/json' \
-d '{"vector": [0.5, 0.6, 0.7, 0.8], "limit": 5, "filter": {"must": [{"key": "count", "range": {"gte": 30}}]}, "with_payload": true}'
# Returns 5 results, all with count >= 30, correctly filtered
# Count with filter
curl -s -X POST http://localhost:6333/collections/test_collection/points/count \
-H 'Content-Type: application/json' \
-d '{"filter": {"must": [{"key": "city", "match": {"value": "Paris"}}]}, "exact": true}'
# {"result":{"count":1},...}Concurrent read/write contention test
# Created larger collection (128-dim, 2000 points, 4 segments)
curl -s -X PUT http://localhost:6333/collections/contention_test \
-H 'Content-Type: application/json' \
-d '{"vectors": {"size": 128, "distance": "Cosine"}, "shard_number": 1, "optimizers_config": {"default_segment_number": 4, "indexing_threshold": 100000}}'
# Bulk inserted 2000 points in batches of 200
for i in 0..9; do python3 gen_points.py 200 $((i*200+1)) | curl -s -X PUT .../points -d @-; done
# Count verified: {"result":{"count":2000},...}
# Baseline: 50 searches without concurrent writes
# avg=9.4ms, median=9ms, p95=12ms, p99=13ms, max=13ms
# Contention: 50 searches WITH continuous background upserts (100-point batches)
# avg=11.0ms, median=10ms, p95=16ms, p99=20ms, max=20ms
# ~7000 new points were upserted during the test
# After concurrent test:
# Total points: 9400 (2000 original + ~7400 from background writer)
# Status: green, segments: 4
# All original points still retrievable with correct payloads
# Search still returns correct resultsError handling
# Non-existent collection
curl -s -X POST http://localhost:6333/collections/nonexistent/points/search -d '...'
# {"status":{"error":"Not found: Collection `nonexistent` doesn't exist!"},...}
# Non-existent point ID
curl -s -X POST http://localhost:6333/collections/test_collection/points -d '{"ids": [99999], "with_payload": true}'
# {"result":[],"status":"ok",...}
# Wrong vector dimension
curl -s -X POST http://localhost:6333/collections/test_collection/points/search -d '{"vector": [0.1, 0.2], "limit": 3}'
# {"status":{"error":"Wrong input: Vector dimension error: expected dim: 4, got 2"},...}
# Empty collection search
curl -s -X POST http://localhost:6333/collections/empty_collection/points/search -d '...'
# {"result":[],"status":"ok",...}Minor note
In lib/shard/src/segment_holder/read_points.rs:73, the timeout error message says "fill query context" but this method (read_points_locked) is called from retrieve_blocking, not from fill_query_context. This is cosmetic — the wrong error message would appear when a retrieve operation times out waiting for the segment holder lock.
Test suite
Could not run cargo test due to disk space constraints on the sandbox VM (9.9GB disk, ~1.1GB free after release build). The build itself compiled and linked successfully with no warnings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of qdrant#8056 (MERGED) for Orpheus review evaluation.
Upstream: qdrant#8056
Original PR description:
Continuation of qdrant#8024
Benchmarks
Setup
Parallel updates + searches using the following commands:
For hardware, a 2 core 8GB cloud cluster with 1 node was used.
For the searches, a slightly modified version of BFB was used, mostly to measure slow requests in detail.
The changes can be found here: qdrant/bfb#97
Results
The results of 2-3 runs on each Qdrant@1.16.3 and this PR averaged.
PR
Qdrant@1.16.3
Furthermore:
These results show that this PR improves on slow requests during search