mirror of
https://github.com/qdrant/qdrant.git
synced 2026-07-31 07:00:56 -05:00
386e222b42
* fix: serialize optimization proxy install against shard updates execute_optimization captures `target_config` from the optimizer's frozen config and then wraps source segments in proxies. Between those two points, `CollectionUpdater::update` can apply a `CreateVectorName(V)` to the source segments via `apply_segments`, leaving the optimizer with sources that have V but a target_config that does not. The optimization then produces a merged segment without V, and a follow-up optimization (running with the refreshed config that includes V) fails to use that segment as a source: "Cannot update from other segment because it is missing vector name X". Close the race by extending the scope of the existing `LockedSegmentHolder::acquire_updates_lock` to cover the proxy install window. `CollectionUpdater::update` already takes this lock before processing any shard update, so concurrent writers wait until proxies are in place — at which point further mutations hit the proxies (recorded as intent and propagated to the merged segment in `finish_optimization`) instead of the originals. The guard is dropped right after proxy install so the slow build phase does not extend it. Tests: - Three `SegmentBuilder::update` tests document the precondition the lock now guarantees: with a target schema that adds a named vector the source lacks, update errors with "missing vector name X". Quantized and mixed-source variants exercise the same error path. - `test_optimize_blocks_proxy_install_on_updates_lock` asserts the invariant directly: while the updates lock is held, proxies are not yet installed. Verified to fail when the new guard is removed (otherwise it passes because `finish_optimization` also takes the same lock, so a naive "did optimize finish?" check would not catch a missing proxy-install guard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(optimize): finish_optimization lock order; drop redundant tests Address review of #9110: 1. `finish_optimization` was acquiring `upgradable_read` before `acquire_updates_lock`, while the new guard at the start of `execute_optimization` acquires them in the reverse order. With two optimizer threads in flight, thread A in `finish_optimization` could hold `upgradable_read` and wait on `updates_lock` while thread B at the top of `execute_optimization` held `updates_lock` and waited on `upgradable_read` (parking_lot allows only one upgradable reader), deadlocking. Swap `finish_optimization` to take `updates_lock` first so both halves agree. 2. Drop the quantized and mixed-source variants of the inverted `SegmentBuilder` unit test — all three asserted the same error path (the mismatch check fires before quantization training or per-source branching), so only one is useful as documentation of the precondition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: reject source-superset schema mismatch at SegmentBuilder Drop the lock approach (deadlocked test_continuous_snapshot) and fix the bug at the merge layer instead. Snapshot's proxy_all_segments_and_apply acquires the segment_holder upgradable_read first and then takes acquire_updates_lock tactically inside the snapshot operation. The previous commits' lock-extension acquired updates_lock before upgradable_read, so a snapshot in flight and an optimization just entering execute_optimization could deadlock holding each other's required next lock. Snapshot cannot easily reverse its order — that would hold updates_lock for the entire snapshot duration, blocking all writes. Move the fix to where the actual harm happens: SegmentBuilder::update iterates the target's vector_data and silently drops source vectors that aren't in target. That silent drop is what produces the broken merged segment in the CreateVectorName-vs-optimizer race. Add a check that every source vector name is in the target schema; the optimization aborts cleanly on mismatch and the next round (with refreshed config) merges correctly. This is strictly stronger than the lock: the lock only closed the window where V arrived *during* the proxy-install region. The schema check catches both that window and the window where V's apply_segments completed before the optimizer's lock acquisition. Diff is contained to lib/segment; no locking changes, no cross-crate plumbing. test_continuous_snapshot passes again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(segment_builder): use Cancelled instead of ServiceError for schema mismatch ServiceError flips the shard to RED status (via `report_optimizer_error` → `segments.optimizer_errors`) and stays sticky until the next `recreate_optimizers_blocking` clears it. That's the right shape for hardware/IO failures but wrong for the schema-mismatch case here, which is an expected, recoverable race outcome — the next optimizer round with a refreshed target_config merges the same originals cleanly. `Cancelled` is the variant the optimization worker treats as a recoverable cancellation: logged at debug, tracker marked Cancelled, no `report_optimizer_error` call, no RED status. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(segment_builder): also use Cancelled for the existing target-superset error The existing "missing vector name" check at the start of the merge loop also fires during a race — specifically the optimizer-vs-DeleteVectorName shape, where V is removed from originals before J wraps proxies but J's frozen target_config still has V. Like the new source-superset check, this is an expected, recoverable race outcome, so use Cancelled instead of ServiceError to avoid flipping the shard to RED. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>