* test: wait for WAL flock release on peer restart in dirty-shard test
The flake addressed by #9124 (bumping `wait_for_peer_online` to 60s) was
misdiagnosed as CPU contention. Logs show the restarted peer panics within
~1s of startup at `consensus_wal.rs:36` with:
Wal error: Can't init WAL: Kind(WouldBlock)
Panic: Can't open consensus WAL: Kind(WouldBlock)
`wal::Wal::open` calls `fs4::FileExt::try_lock` (non-blocking `flock`) on
the WAL directory fd. After `p.kill()` (SIGKILL + waitpid) the kernel
normally releases the killed peer's flock immediately, but under
pytest-xdist load there is a small window where it lags. The fresh peer's
startup then races and panics. After the panic `/readyz` never returns
200, so neither 30s nor 60s rescues the test.
Fix: add a `wait_for_wal_unlocked` helper that polls both the consensus
WAL directory and the local-shard WAL directory with the same exclusive
non-blocking flock that qdrant uses, and call it after every `p.kill()`
that is followed by a `start_peer` on the same `peer_dir`. The two
60s timeouts are restored to the default 30s now that the underlying
race is gone.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: fail fast if sync restart crashes in dirty-shard test
The sync restart in `test_dirty_shard_survives_update_collection` used
plain `wait_for_peer_online(sync_uri)`, which only polls `/readyz`. If
the freshly started peer panics on startup (e.g. WAL `WouldBlock`), the
test waits the full 30s timeout and then reports a `/readyz` timeout
instead of the actual panic message and exit code.
Switch the sync restart to `wait_for_peer_online_or_crash(...)` (same
helper already used for the dirty restart). On crash it dumps the peer
log tail so the next CI failure shows the real reason instead of a
generic timeout.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: drop speculative WAL flock wait, keep crash-detection switch
Reverts the `wait_for_wal_unlocked` helper that polled the WAL
directories with non-blocking flock. The kernel-side flock race it was
guarding against could not be reproduced in isolation (0/300 iters of
SIGKILL+wait+re-flock on bare Linux), so it was speculative.
Kept:
- Sync restart now uses `wait_for_peer_online_or_crash(...)` instead of
plain `wait_for_peer_online`, so any startup panic surfaces fast with
the actual log tail instead of hiding behind a 30s `/readyz` timeout.
- The two 60s timeouts bumped in #9124 are restored to the default 30s.
If the flake recurs in CI, the new failure output will tell us the
actual cause (panic message + exit code), which is more useful than
papering over it.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* tests: use uniform `from .utils import *` in consensus tests
Eight consensus tests imported specific symbols from `utils` instead of
using `from .utils import *`. The most consequential side effect was
missing the `every_test` autouse fixture, which is responsible for
cleaning up leaked `processes` and resetting the port-slice allocator
between tests. Without it, leaks from one test can carry into the next
on the same xdist worker, causing `processes.pop(target_idx)` to return
the wrong peer in tests like `test_dirty_shard_crash_loop` and triggering
WAL-lock conflicts when the intended target is left running.
Make the imports uniform across the package so the autouse fixture is
always in scope.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* tests: keep `test_issues_api` on explicit imports
This module uses a `@pytest.fixture(scope="module")` cluster setup
shared across all tests in the file. With `from .utils import *` the
function-scoped `every_test` autouse fixture comes into scope and runs
after the module-scoped `setup`, so on the first test it sees the
already-populated `processes` and calls `kill_all_processes()` — killing
the shared cluster before the test body runs. The test then fails with
connection refused on the cluster's port.
Keep explicit imports here so `every_test` is not autoloaded; the
module teardown already cleans up via `kill_all_processes()`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`commit_read_hashring` returns once the leader has applied the entry,
but followers may not have applied it yet. The CommitRead handler calls
`invalidate_clean_local_shards` for `old.nodes()`, which cancels any
ongoing shard clean task. If the test's subsequent `cleanup?wait=true`
request to a follower arrives just before that follower applies
CommitRead, the cleanup task is started and then cancelled mid-flight,
and the endpoint returns HTTP 500 "Failed to clean shard points due to
cancellation, please try again".
Add `wait_for_same_commit` after `commit_read_hashring` so every peer
has applied the entry (and thus already invalidated any clean tasks
that don't exist yet) before we issue cleanup.
Observed flake:
https://github.com/qdrant/qdrant/actions/runs/26259341982/job/77289143792
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* test(consensus): snapshot transfer with set-payload for missing points
Add a consensus test reproducing a shard transfer abort caused by
set-payload (and other partial-update) operations targeting non-existing
points.
Such operations are written to the WAL before the point-existence check
rejects them, so the queue proxy replays them to the receiver during a
snapshot transfer. The receiver applies them with force=true, bypassing
the missing-point tolerance in handle_failed_replicas, and the operation
hard-fails with `NotFound: No point with id ... found`. Under sustained
load the bounded queue/driver retries are exhausted, the receiver replica
is marked Dead and the transfer is aborted.
The test keeps the missing-point load running while checking the result,
because consensus auto-recovers Dead replicas: stopping the load first
would let the next recovery transfer succeed and mask the bug. It must
FAIL on current code and PASS once the receiver tolerates missing-point
operations during recovery.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(transfer): skip non-transient errors during queue proxy WAL replay (#9126)
During a shard transfer the queue proxy replays operations from the
sender's WAL to the receiver. Some of these are partial updates
(set_payload, update_vectors, ...) that the receiver rejects with a
non-transient error - most commonly `NotFound: No point with id ...`
for a point that does not exist on the receiver, but also any other
client-caused bad request.
These operations were replayed from the WAL, meaning they were already
applied (and rejected the same way) on the sender, so the sender's state
reflects them as no-ops. Propagating the error aborted the whole transfer;
under sustained load the bounded queue/driver retries were exhausted and
the receiver replica was marked Dead.
Handle the error where the semantic context lives - the transmitter
(`transfer_operations_batch`): skip operations the remote rejects with a
non-transient error and keep going, while still propagating transient
errors so the caller retries delivery. Because the batch update API aborts
at the first failing operation, a non-transient batch error falls back to
one-by-one sending to isolate and skip the offending operation(s).
This complements PR #5991, which handles missing points on the live
forwarded-update path (handle_failed_replicas) but not the WAL replay path.
Fixes the abort reproduced by
test_shard_snapshot_transfer_with_missing_point_updates.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Under parallel pytest-xdist load the rejoining peer needs more than the
default 30s budget to replicate the UpdateCollection entry and report
ready, causing intermittent CI failures:
Exception: Timeout waiting for condition peer_is_online to be satisfied
in 30 seconds
Bump both peer-online waits in the dirty shard crash-loop test to 60s,
matching the precedent set by #8963 for similar CPU-contention flakes.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Recreate optimizers in non-blocking fashion from consensus calls
* Update comments
* On optimizer config update failure, report error status to local shard
* Add a test to confirm we don't block consensus
* Rerun recreation if called multiple times
* Use atomics instead
* Move to the bottom
* Reformat
* Use cluster default shard transfer method for fallback
When a WAL delta automatic transfer fails, the driver falls back to the
method passed via `fallback_method`. This was hard-coded to
`StreamRecords` (unless `prevent_unoptimized` was enabled), which is
inconsistent with the 1.18.0+ default of `Snapshot` and ignores any
configured `default_shard_transfer_method`.
Use `Collection::default_shard_transfer_method()` instead, so the
fallback matches the cluster default. With `prevent_unoptimized` we
still pin to `Snapshot` to preserve deferred point state exactly (raw
segment copy); stream_records would send deferred points but they
would not be deferred on the target.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Avoid wal_delta fallback, update fallback test
If the cluster default transfer method is wal_delta, the same-method
fallback would be refused by the driver. Use snapshot as a safe fallback
in that case; snapshot is also the 1.18.0+ default.
Update test_shard_wal_delta_transfer_fallback to assert the new
snapshot fallback (was stream_records).
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address clippy wildcard_enum_match_arm
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Add test to show cleanup may conflict with update queue
* When invoking clean task, first wait for current update queue
* Don't hold shard holder lock for a long time
* Also assert the clean task finished completely
The test_shard_snapshot_transfer_throttled_updates test was flaky because
it checked data consistency immediately after killing background upload
processes, without waiting for in-flight writes to propagate across peers.
All sibling tests (test_shard_snapshot_transfer_fast_burst,
test_shard_stream_transfer_throttled_updates, etc.) already include a
sleep(1) after killing uploaders. This was the only variant missing it.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Pre-pull the fullstorydev/grpcurl Docker image before running the test
and retry the gRPC health check up to 3 times, so transient Docker
networking hiccups on CI runners no longer cause a hard failure.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: validate vector dimensions before WAL write for async upserts
When upserting points with wait=false (the default), dimension
mismatches were silently discarded during background processing.
The API returned 200 "acknowledged" but the points were never stored,
causing silent data loss with no error feedback to the user.
This adds an early dimension validation check in do_upsert_points()
that runs before the operation is written to WAL. This ensures that
dimension errors are returned to the client regardless of the wait
parameter, matching the behavior of wait=true.
The validation handles all vector types:
- Dense single vectors
- Multi-dense vectors
- Named vectors (dense, multi-dense, sparse)
- Sparse vectors are skipped (no fixed dimension)
Closes#9039
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor: move vector dimension validation into dedicated module
Extract validate_vector_dimensions and helper functions from update.rs
into src/common/validate_vectors.rs for better code organization.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: update shard update test for early dimension validation
The test expected a shard-level error message, but now dimension
mismatches are caught before reaching the shards. Update the assertion
to accept either the early validation error or the shard-level error.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: assert actual dimension error message in shard update test
Check for the descriptive error ("Vector dimension error: expected dim: 4, got 3")
rather than the generic shard failure wrapper.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Add integration test for `match: {except: []}` with integer index
Regression test for https://github.com/qdrant/qdrant/issues/9050
An empty `except` list (NOT IN []) should always match all points that
have the field, both with and without a payload index. Currently the
integer (and keyword) indexed path incorrectly returns zero results
because:
1. serde deserializes `except: []` as `AnyVariants::Strings([])` (first
variant of the untagged enum)
2. The map index filter_impl returns `iter::empty()` for empty
cross-type variant, instead of matching everything
The test covers:
- except: [] without any index (baseline, currently works)
- except: [] with an integer index (currently broken)
- except: [] with a keyword index (currently broken)
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix `match: {except: []}` returning zero results with payload index
Fixes https://github.com/qdrant/qdrant/issues/9050
Root cause: `except: []` deserializes as `AnyVariants::Strings([])`
due to the untagged serde enum trying `Strings` first. On an integer
index, the filter_impl matched `Except + Strings(empty)` and returned
`iter::empty()` (zero results). The same issue existed symmetrically
on keyword indexes with `Integers(empty)` and on UUID indexes with
`Integers(empty)`.
The fix: when the cross-type variant reaches the Except branch (e.g.
Strings on an integer index, Integers on a keyword/UUID index), return
`None` unconditionally — regardless of whether the set is empty. This
delegates to the fallback condition checker, which already handles
`except: []` correctly by matching all values.
The `estimate_cardinality_impl` functions had the same bug (returning
`CardinalityEstimation::exact(0)` for the empty cross-type case) and
are fixed the same way.
Affected index types: integer, keyword (str), UUID.
Bool index is NOT affected — it already returns `None` for all
Any/Except conditions.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Expand match-except test to cover all index types and cross-type filters
Extends the regression test for #9050 to comprehensively cover:
- Integer, keyword, and UUID field indexes
- Empty except list (the original bug) for each index type
- Non-empty except list with matching types (normal filtering)
- Cross-type except values (e.g. strings on integer index, integers on
keyword/UUID index) — type mismatch should exclude nothing → all match
- Exclude-all: listing every value should return zero results
- All scenarios tested both with and without a payload index
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
When the restarted peer's dummy shard is auto-recovered by the
cluster's recovery loop before the test issues its manual
`replicate_shard` call, the manual call returns 400 "already involved
in transfer". Skip the manual call when a transfer is already in
flight — the existing wait_for / transfer-count / replica assertions
still verify the shard recovers.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Raw requests already use QDRANT_HOST, but bypass request_with_validation
and missed QDRANT_HOST_HEADERS. Without these headers, tests cannot run
behind host-based reverse proxies or mocks.
Use unique point IDs across all phases instead of overwriting the same
id repeatedly. The gridstore payload storage size estimate is
bitmask-based: overwrites keep old blocks allocated until a periodic
flush reclaims them, so the test was racing the 5s flush worker. With
unique ids every block stays live and the post-flush size still
reflects all inserted points.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test waited a fixed 0.5s after each PUT/DELETE before reading from
every peer, which raced with raft apply on followers under CI load.
Replace the fixed sleeps with wait_for-based polling so each per-peer
read retries until the expected value is observed.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Under stress, three concurrent user-requested snapshot transfers (10 000
points each) running while the killed peer recovers can starve the
leader's heartbeats long enough to trigger a raft election. If the
recovery transfer's `RecoveryToPartial` proposal is submitted while no
leader exists, raft drops it silently — `recovered_switch_to_partial`
returns Ok regardless because it only sends to a channel — and the
retry path then waits a full CONSENSUS_CONFIRM_TIMEOUT (10s) before
trying again. Combined with sequential per-shard recovery (auto
transfer limit = 1 × 3 shards), the 30s `/readyz` budget runs out.
Add an optional `wait_for_timeout` to `wait_for_peer_online` and bump
this test's wait to 60s. Default behaviour for other callers is
unchanged.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [ai] Add integration test for triggering inconsistent resharding state
* [ai] Also add test for resharding down
* Update test
* Resolve resharding idempotency through setting replica states
* Remove resharding down test
* Reformat
* [ai] Remove resharding abort order, abort before setting replica state
* [ai] Resolve test flakiness
* Collapse matches into helper function
* Check preconditions before aborting resharding
* Fix test flakiness by not waiting for a dead node
* Abort resharding before aborting transfer for idempotency
* Update comment
* Release shard holder lock on transfer/reshard abort to prevent deadlock
* Remove now unused shard holder parameter
* Split handle_replica_changes to eliminate need for juggling locking
* In resharding tests, import all utils to enable every_test cleanup
Resolves flakiness I've been seeing in
test_set_replica_dead_clears_resharding_state test
The test killed the last peer immediately after start_cluster, but
start_cluster only waits for cluster size and a known leader — not for
all peers to be promoted from learner to voter. If the last peer caught
up first, it became the only other voter alongside the leader; killing
it left a 2-of-2 quorum with one voter dead, and the subsequent
CreateCollection commit timed out after 10s.
Wait for all peers to be voters before killing one, so the survivors
form a 2-of-3 voter quorum.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The rapid drop/create loops in test_rejoin_cluster intentionally use
short 3s timeouts to accumulate Raft log entries quickly. Under CI
load the consensus apply for CreateCollection can exceed 3s (segment
setup competes with background flushes/optimizations), and the API
returns 500 even though the operation reaches consensus right after.
The matching upserts already pass `fail_on_error=False`; do the same
for `create_collection` to make the test resilient to that race.
These tests rely on the collection size stats cache being refreshed to
detect that a size limit has been exceeded. Without wait=true, upsert
operations are written to WAL and acknowledged immediately without being
applied to segments. When the cache refreshes, it reads segment data
which may not yet reflect the pending WAL operations, causing the size
check to see stale values and not reject the request.
Adding wait=true ensures operations are applied to segments before the
response returns, so the cache refresh sees the correct sizes.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The deadline loop only waited for the download phase to become
"streaming", but the assertion also required bytes > 0. On CI the
assertion could fire before iter_content yielded the first chunk,
causing a flaky failure. Wait for bytes > 0 in the deadline loop
and improve the error message to show bytes/error state.
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: keep optimizers disabled during snapshot transfer in deferred test
The snapshot variant of test_shard_transfer_includes_deferred_points was
flaky because optimizers were enabled before the transfer, letting the
optimizer race ahead and fully index the segment before the snapshot was
captured (~1s of HNSW build for 500 small vectors fits comfortably before
the snapshot is taken). The deferred-state assertion then fails since all
points are already visible.
Only enable optimizers before the transfer for stream_records (which needs
them for its internal wait=true). For snapshot, leave optimizers disabled
through the transfer so deferred state is preserved on the wire, then
enable them afterwards for trigger_upsert_wait_true. The hung server-side
wait=true from the timeout-and-retry block does not block the snapshot —
wait_for_deferred_points_ready runs in a detached tokio::spawn.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: skip wait=true probe for snapshot variant
CI showed that with optimizers kept disabled through the snapshot transfer
(needed to preserve deferred state on the wire), the wait=true probe at
the start of the test leaves a hung server-side request: update_local
holds local.read() until the deferred wait resolves, and there is no
optimizer to resolve it. The subsequent shard transfer's apply path
deadlocks against that held read lock when queue_proxify_local tries to
take local.write().
For stream_records the config update later cancels the hung worker, so
the probe is fine there. Move the probe (and config update) under the
stream_records branch so the snapshot variant doesn't leave a hung
update around. The probe was auxiliary behaviour verification, not
central to the snapshot-of-deferred-points assertion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Revert "test: skip wait=true probe for snapshot variant"
This reverts commit d07aac78a263d7a91691e43444b9dae44e3d179f.
* test: add reproducer for deferred-wait shard-transfer deadlock
Adds test_shard_transfer_with_hung_deferred_wait_does_not_deadlock as a
focused reproducer for the engine bug surfaced by the snapshot variant
of test_shard_transfer_includes_deferred_points.
Lock-ordering chain:
1. With prevent_unoptimized=true and max_optimization_threads=0, a
wait=true upsert on deferred points enters
wait_for_deferred_points_ready (update_worker.rs:241), which loops
on tokio::select over cancel and optimization_finished. The
optimization_worker hits limit==0 and `continue`s without firing
optimization_finished_sender (optimization_worker.rs:172-174), so
neither branch of the select ever fires.
2. update_local (replica_set/update.rs:49) holds self.local.read()
across the entire update await. actix-web does not cancel the
response future on client disconnect, so the read guard stays alive
even after the client's 5s timeout.
3. A subsequent snapshot transfer eventually calls queue_proxify_local
(replica_set/shard_transfer.rs:122), which needs self.local.write().
tokio::sync::RwLock is write-preferring: the queued writer blocks
new readers, including is_local() calls on the consensus apply
path itself (shard_transfer.rs:129-130). The apply never returns,
the consensus broadcast never fires, POST /cluster times out with
"Waiting for consensus operation commit failed".
The new test asserts the symptom (POST /cluster must return promptly)
without papering over the bug, so it stays red until the engine is
fixed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(replica_set): release local read guard around deferred-points wait (#8862)
* fix(replica_set): drop remotes read guard early in update_impl
`update_impl` was holding `self.remotes.read()` and `self.local.read()`
across the entire update await, including the deferred-points wait that
can park indefinitely under prevent_unoptimized + max_optimization_threads=0.
When a shard transfer is started concurrently with a parked wait=true
update, the consensus apply runs `add_remote`, which calls
`self.remotes.write().await`. tokio::sync::RwLock is write-preferring:
the queued writer is blocked behind the held read, the apply never
returns, and `POST /cluster` times out with "Waiting for consensus
operation commit failed".
Fix: snapshot updatable remote shards into owned `Vec<RemoteShard>` and
drop the read guard before the await. The remote_update futures now own
the cloned RemoteShards, so they no longer borrow from the guard.
The `local` guard is still held across the await (futures borrow
`&Shard` from it). Releasing it would unblock `queue_proxify_local`'s
`local.write()` too, but that requires wrapping `Shard` in `Arc` —
deferred to a follow-up. For the consensus-commit-timeout deadlock
exposed by `test_shard_transfer_with_hung_deferred_wait_does_not_deadlock`,
dropping `remotes` is sufficient.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(updater): wake deferred wait on caller-receiver drop
`wait_for_deferred_points_ready` parked on a `tokio::select` over
`cancel.cancelled()` and `optimization_finished_receiver.changed()`.
Under prevent_unoptimized + max_optimization_threads=0, neither fires:
optimization_worker.rs:171-174 hits `limit == 0` and `continue`s
without notifying, and the cancel token is the worker's lifecycle
token (only fired by stop_update_worker on config update / shutdown).
The top-of-loop `is_closed()` poll didn't help — the loop never
re-runs once the select parks.
Take `feedback_sender` by `&mut` and add `feedback_sender.closed()`
as a third select branch. When the matching `Receiver` is dropped
(by upstream cancellation, client-supplied timeout, or any future
cancellation), the detached task wakes immediately and exits with
WaitTimeout instead of staying parked until the next worker restart.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI] split update operarion into submit and independent wait function
* [AI] refactor `update_local` to drop local shard lock after submitting update operation
* [AI] refactor `update_impl` for early release of the lock in case of local shard update
* fmt
* Apply suggestion from @generall
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
During a snapshot shard transfer recovery, the receiving peer
temporarily takes its local shard before installing the snapshot, so
the cluster info endpoint returns local_shards: [] for a brief window.
The test helper indexed [0] on that list and crashed with IndexError,
making test_triple_replication flaky. Treat the empty case as a
non-Active state so callers poll again instead.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Make resharding state transitions idempotent on replay
Why: consensus entries may be re-applied after a crash (partial state
on disk) or during raft recovery. The unchecked state-transition
helpers used `debug_assert!` to require a specific starting state, so
a replay would panic in debug or silently overwrite in release.
How to apply: use write_optional so the state file is only touched
when the in-memory state needs to change. This also avoids unnecessary
fsyncs when a replay is a no-op.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Skip replica set creation on resharding start replay
Why: on replay, `create_replica_set` would call `create_shard_dir`,
which removes and recreates the existing shard directory -- wiping
any shard contents that had already been migrated or written since
the first apply.
How to apply: check `contains_shard(shard_id)` before creating the
replica set and pass `None` to `start_resharding_unchecked` when a
replica set with the target shard id is already present. Also
relaxes `check_start_resharding` to return `Ok` (instead of a
swallowed `bad_request`) when a matching resharding state is
persisted, so the caller can fall through each idempotent step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Derive resharding shard count from shard id for idempotency
Why: the start/finish/abort paths used ++/-- on the persisted shard
number with a `debug_assert` pinning the expected starting value. On
replay (e.g. after a crash between the shard holder mutation and the
config save) this either panics or produces a wrong count.
Since resharding always targets the last shard (auto sharding assigns
contiguous ids from zero), the target count is a pure function of the
shard id: `shard_id + 1` for start up, `shard_id` for finish down and
abort up. Set it directly and skip the save when it already matches,
so replay converges to the same value without touching the file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fall through resharding finish/abort steps on replay
Why: the early return on `resharding_state.is_none()` assumed that a
missing state means the operation was fully applied. But a crash can
leave the state cleared while the shard drop, key mapping removal, or
shard count update are still pending. Short-circuiting then skips the
reconciling work those replays need to do.
How to apply: drop the early return so every step runs; each step is
already individually idempotent (check_*, drop_and_remove_shard,
remove_shard_from_key_mapping, the set-based shard count update).
The abort_resharding down-invalidation path now reads nodes from the
router regardless of its variant, so a replay over an already-rolled-
back ring doesn't trip the removed debug_assert!s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Skip shard key mapping write when nothing to remove
Why: `write_optional` returns `Some` whenever the shard key is
present, even if the shard id we're removing is already gone. That
still triggers a JSON rewrite and a cache notification on every
replay of a finished finish/abort.
How to apply: check that the shard id is actually in the set before
returning `Some`. If the set is missing or already doesn't contain
the id, return `None` so the on-disk file and in-memory data are
left untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add tests for resharding replay idempotency
Covers the paths that must not error or panic on replay:
- `check_start_resharding` when matching state is already present
- `start_resharding_unchecked` preserves matching state verbatim
- `finish_resharding_unchecked` when state is already cleared
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Simplify finish_resharding_unchecked closure
Replace a manual `match` on `Option` with `as_ref().map(...)` to
satisfy clippy::manual_map.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: add a test for resharding replay (#8839)
* [ai] add a debug_assert on shard_number (#8846)
* Update lib/collection/src/shards/shard_holder/resharding.rs
Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com>
* Reformat
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: tellet-q <166374656+tellet-q@users.noreply.github.com>
Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com>
* [ai] On shard snapshot transfer recovery, drop existing shard before recovery
* [ai] Add integration test to assert clearing behavior
* [ai] Debug assert our replica is not active when we clear it
* [ai] Tweak assertion
* Fix flaky test, replica may temporarily not be visible
* Replace debug assertion with runtime error
The test asserted segment count growth and payload index backfill
immediately after upsert, but both are produced asynchronously by the
optimization worker (new appendable segment via
`ensure_appendable_segment_with_capacity`, payload index backfill).
Replace the bare asserts with `wait_for` polling.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PUT with wait=true only awaits consensus apply on the leader, so
followers can still serve a stale value from their local persistent
state shortly after. The fixed 0.5s sleep was occasionally too short
on slow CI runners (observed ~1.15s follower lag), causing
test_consensus_compaction to fail with a None metadata value on
peers[1].
Replace the sleep + one-shot assertion with per-peer wait_for polling.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix consensus test port collision across xdist workers
When a peer was started with `get_port()` for each of p2p/grpc/http,
only the OS-allocated p2p_port was guaranteed free. On restart with
`port=p.p2p_port`, the framework derives grpc=port+1, http=port+2 — but
those neighbor ports were never reserved at original startup. Under
`pytest -n auto --dist=loadfile`, another xdist worker could legitimately
bind one of them, causing the restarted peer's REST bind to fail with
EADDRINUSE while the test still talks to that URL and gets a misleading
404 from the unrelated peer.
Add `get_port_triple()` which allocates a base port and probes bind() on
base+1 and base+2 to verify the contiguous slot is free across processes,
then route start_peer / start_first_peer (and their two callers in
test_peer_snapshot_bootstrap / test_cluster_rejoin) through it.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Partition port allocation by xdist worker
Each pytest-xdist worker now draws peer ports from a disjoint slice
(BASE + worker_index * SIZE), so concurrent workers can never compete
for the same triple. Within the slice we still probe-bind() each
candidate to skip ports occupied by unrelated processes, and fall back
to OS allocation if the slice is exhausted.
This eliminates the cross-worker race that prompted the original fix,
and removes the need for the ±2 buffer dance against busy_ports.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [ai] Add config boolean to disable URL based snapshot restore
* Merge if-statement
* Comment-out config option by default
* [ai] Also block partial snapshots from remote URLs
* [ai] Only run clock consistency test when staging feature is present
* [ai] Add integration test
* ci: parallelize consensus tests with pytest-xdist
Enable pytest-xdist for consensus_tests to run tests across multiple
workers in parallel, significantly reducing CI wall time (~20min → ~5-7min).
Changes:
- Add `-n auto --dist=loadfile` to the consensus test pytest invocation
- Remove hardcoded port_seed from tests that don't need fixed ports for
restart/rejoin (test_order_by, test_consensus_compaction,
test_named_vector_crud, test_listener_node)
- Give test_cluster_rejoin its own PORT_SEED=15000 to avoid port
conflicts with auth tests (PORT_SEED=10000)
- Derive restart ports from killed PeerProcess objects instead of
hardcoded arithmetic where possible
- Add xdist_group("auth") marker to auth test files to ensure they
run on the same worker (they share PORT_SEED=10000)
Made-with: Cursor
* fix: remove remaining hardcoded port_seed=20000 causing parallel test conflicts
8 test files were using port_seed=20000 as a positional argument to
start_cluster(), which was missed in the initial change. When running
in parallel with pytest-xdist, multiple workers would try to bind to
the same port range (20000-20x02), causing port conflicts and cascading
test failures.
Also remove port_seed=23000 from test_snapshot_recovery_kill.py since
it doesn't need fixed ports for restart.
Made-with: Cursor
* fix: use saved port for restart in test_two_follower_nodes_down
The test was restarting killed peers on hardcoded ports (20200/20100)
that previously matched port_seed=20000. After switching to random
ports, the restart ports no longer match the original peer ports,
causing raft state URI mismatches and peer startup failures.
Save the p2p_port from the killed PeerProcess and reuse it for restart.
Made-with: Cursor
* Reuse p2p ports when restarting killed peers in consensus tests
When a peer is killed and restarted with random ports, it gets a new
consensus URI. The cluster needs a Raft operation to update this URI,
which under CPU contention from parallel test workers can exceed the
30-second timeout. Fix by capturing each peer's p2p_port before killing
and reusing it on restart, so the URI stays the same and no consensus
update is needed.
Made-with: Cursor
* A few improvements for parallel runs (#8731)
* fix: make auth tests' PORT_SEED per-worker to avoid port collisions
* ci: improve failure visibility for parallel consensus tests
* Three small changes to make hangs, interleaved output, and coverage runs behave predictably under pytest-xdist
* fix: two test bugs surfaced by parallel runs and revert drop PR_SET_PDEATHSIG helper
* fix: wait for count convergence in test_triple_replication
* fix: clean leaked peer processes at test start
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: tellet-q <166374656+tellet-q@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
