mirror of
https://github.com/qdrant/qdrant.git
synced 2026-09-28 00:47:32 -05:00
model-testing-reload-postmortem
4526
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c85eed4773 |
test(shard): regression test for the CoW-destination flush in create_field_index
Reproduces the #10095 loss shape deterministically: a pending copy-on-write move out of a non-appendable source, a destination whose own pre-build flush is skipped by the already_indexed short-circuit, then a holder-wide create_field_index. Verified failing with the dependency-aware flush neutralized (destination stays behind the move while the source flushes past it) and passing with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6bdfca854b |
fix(shard): flush CoW destinations before the payload-index pre-build flush
create_field_index force-flushes each segment before building an index on it (flush-before-build, #9767), one segment at a time, outside flush_all's all-segment lock capture and copy-on-write dependency ordering. That flush durably advances a CoW source past the delete halves of its pending moves. The appendable-first iteration order usually flushes the destination before the source, but not always: a destination proxy-wrapped by a running optimization is classified non-appendable and can skip its flush entirely through the already_indexed short-circuit (the proxy reports the field as present), and a move landing mid-pass is ordered behind nothing. Once the source flushes, the move's WAL entry stops being replayable: the pre-image is durably deleted while the only current copy sits in the unflushed destination, and a graceful close then loses the point. This is the root cause of the nightly model-testing reload divergence (#10095), traced end-to-end in CI runs 31583878492 and 31583871346: cow move op 5197 into a freshly proxied destination, index op ~5252 flushing every source past it while skipping the proxy, destination reloading at 5181, replay declining with 'No point with id'. The fix mirrors flush_all's invariant at the only per-segment flush site: before flushing a segment, flush the destinations of its pending flush_dependency edges (one hop suffices, destinations are appendable and never CoW sources). Destination guards are taken before the flush lock to keep the documented [segment locks -> flush lock] ordering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
081c0a725d |
Revert "test(model_testing): flush synchronously as a discriminating experiment"
This reverts commit
|
||
|
|
83d75439d5 |
chore: keep the too_many_arguments allow attached to its function
The test-hook module was inserted between the attribute and execute_optimization, orphaning it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
874d6f3ed5 |
test(shard): pin-arithmetic regression test for source retirement
Drives a real optimization through execute_optimization with a minimal strategy, injecting operations into the mid-optimization window through a new testing-only after_proxy_wrap hook: a propagated delete raising the destination to 50, and a gap operation raising only the proxy to 99. Asserts the retiring source's drop pin holds through a waterline covering the destination and releases only when the proxy's version is covered. Verified failing with the pre-fix release threshold and passing with the fix. This is an invariant test, not a data-loss reproduction: analysis while designing it showed the gap operations reachable through legitimate apply paths carry no sole-copy data, so the fix is invariant hardening; the observed nightly losses belong to the mid-list flush death still under investigation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8b413948cf |
test(model_testing): mark flush pass boundaries
The flush-exec attribution caught the tear in the act: within one second, a CoW source's flusher executed twice past the fatal operations (captured 2511 then 2618) while the destination's flusher, dirty since the same operations, never executed again - and neither pass reached its decision line nor logged an error. flush_all is the only executor of segment flushers, so those were passes that died mid-list silently. Pair every pass start with its decision so a mid-list death and its caller are visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1bddc7e7aa |
test(shard): regression test for the empty-but-dirty temp drop
Fails on the pre-fix inline drop (the acknowledge jumps past the temp's uncovered range) and passes with the pin. Writing it surfaced a scope correction: a temp that copy-on-write traffic transited is kept, not dropped, because soft-deleted points keep is_empty() false - so the reachable empty-but-dirty case is a temp whose version advanced through schema operations alone. The invariant (a dirty removal must never raise the durable waterline) stands; the fix's causal share in the observed nightly failures is accordingly smaller than first attributed, consistent with the v2 validation wave showing no rate change over v1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
bf8a1d0efc |
test(model_testing): log every segment flush execution
The snapshots-off A/B refuted the snapshot hypothesis: both passes of a disable_snapshots run failed with the identical reload-lost class. Its fully instrumented log then closed every other gap at once: no directory was deleted, the acknowledge window held, both the source's and the destination's directories reloaded - with the source's on-disk version past the CoW operation and the destination's below it, while no flush-pass decision was logged for the shard in the window, and the destination had left the holder dirty via a proxy wrap immediately after receiving the arrivals. Durable state therefore advanced through flush executions that pass-level decision logging does not attribute. Log each execution of a segment's flush closure with its directory and captured version, whichever caller captured it: the flush worker, the snapshot force-flush, or anything else holding a flusher. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
34fc776f3c |
chore: drop an import unused under the all-targets clippy configuration
Local checks ran clippy with the model_testing feature set only; CI's all-targets pass compiles the feature matrix where the import is dead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7fb03f8e5c |
test(model_testing): make every segment-file deletion attributable
The sync-flush experiment refuted the background-flush interleave theory: two of four synchronous-flush runs failed identically, so the loss is flush-mode-independent. The placement/inventory pair then showed a lost point's pre-image directory absent at reload with every known deletion path silent: no pin release logged, no drop_data (now logged at the LockedSegment funnel), no load-time reclaim warning. Two deletion paths remained unlogged: cleanup_cancelled_optimized_segment deletes a built segment's directory outside the funnel, silently on success, on the assumption that cancellation never arrives after the swap; and id-reuse made it impossible to track a directory's custody through swap chains. Log the cancelled-segment cleanup at warn, and tag departures and swap destinations with their directory names so the next failure names its deleter directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
eca686c875 |
test(model_testing): flush synchronously as a discriminating experiment
The reload inventory caught the mechanism red-handed in run 31570040346: a flush pass persisted the CoW source's deletes (segment 11 reloading at 36930/36930, the hops' delete-halves baked in) while leaving the destination of the same moves unflushed (segment 7 reloading at 36808/36808, its arrivals discarded at close). The WAL window contained the moves; their pre-images were durably deleted; ten points vanished. That is the exact destination-before- source hazard flush_all's all-segment locking and dependency ordering exist to prevent, so something interleaves the background flush's captured cut with its execution. Synchronous flush holds every segment lock through execution, closing that split by construction. This commit is the discriminating experiment, not the intended fix: zero failures on this build confirm the capture/execute split as the final layer; the production fix then needs consistent capture without the sync flush's write stall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
bee66e82e1 |
test(model_testing): capture pre-close placement of every point at restarts
The reload inventory resolved the custody fork's first branch: the lost point's last durable holder reloads fine, directory present, expected version, expected point count minus one. What remains unobserved is the pre-close side at segment granularity: which directory's in-memory state held the point at close, under what durability claim. The failing hop's ledger line says the source was clean at version==persisted while its reloaded directory lacks the point, which, if confirmed against a placement snapshot, is a flush whose durable content does not match its claim. Capture the placement of every visible point (segment, directory, point version, deferred flag, segment version/persisted) right before stop_gracefully and print it for every id the reload loses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e498f8d6e3 |
test(model_testing): log the segment inventory at every reload
The custody chain now ends at a binary fork the logs cannot resolve: a lost point's pre-image holder was durable when its final hop ran (source persisted past the point's version, still a holder member at stop, untouched by earlier replayed operations), yet mid-replay no loaded segment knows the id. Either the holder's directory was never loaded at reopen, or its loaded content lacks the point. One inventory line per reload, directory name, version, persisted version, live point count per segment, tells those apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b9ec8a4980 |
test(model_testing): record source durability on every CoW ledger line
The decline postmortem answered its question and disproved two working theories: the failing replay's decline is the only application of the op (the "successful replay CoW" was the live application one second before the restart), and the full-window run's lost points cannot be shard-attributed because the reload postmortem prints every id under both shards. What remains is the custody chain: each hop consumes a pre-image, and whether that pre-image was durable at the moment of the hop is exactly what the ledger does not yet say. Add the source segment's version, persisted version, and the point's pre-move version to each cow line. A lost id's ledger then reads as a custody chain with numbers, and the hop whose pre-image was never durable, or whose durable holder later vanished, identifies itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
159b0e63c9 |
test(model_testing): dump per-segment knowledge when replay declines an op
One v2-validation failure defeats log inference: the ack was correctly pinned, the replay window contained every relevant operation, the containment audit confirmed the point in the optimized destination, that destination's build output covered the point's version range and had flushed past it - and reopen still declined the point's CoW replay for want of a pre-image. Whatever layer lies between "the files should hold it" and "no occurrence found" needs to be observed at the moment of the decline, not reconstructed. On a PointNotFound during WAL replay, log what every loaded segment knows about the id (visibility and point version, WithDeferred). Diagnostic branch only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b87304b36d |
fix(optimizer): pin the waterline when dropping an empty-but-dirty temp segment
Validation of the source-retirement fix produced one failure in six runs (down from at least one in two, and at 236 restart verifications instead of 15), with the ledger showing the residual path: a temporary segment that copy-on-write moves emptied again is dropped immediately, and with it goes the unsaved range that was capping the durable waterline. Upstream source-retirement pins then release while the transit operations are neither durable anywhere nor re-derivable, since their pre-images went down with the retired sources. The replay window in the failing run started past exactly such a transit hop. Any segment leaving the holder dirty must leave a pin behind. Swap evictions got theirs in the previous commit; give the empty-temp drop the same one: register the drop as a post-flush action with ready_at at the segment's version and the acknowledge capped at its persisted version, so the WAL keeps the transit range replayable until the moved copies have flushed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
69734091d0 |
fix(optimizer): retire sources only after their CoW moves are durable
A copy-on-write move during an optimization puts the point's new copy in the shared appendable write segment and records a delete in the proxy. That delete is baked into the optimized segment as a durable tombstone, while the write segment may not flush for a long time. Retiring the source once the waterline covered the destination's version therefore destroyed the last durable pre-image while the only current copy sat in memory: a graceful restart discarded it, and replaying the still-acknowledged CoW operation was declined for want of the pre-image, silently losing the point. This is the nightly model-testing reload divergence (#10095): points missing after close+reopen, unknown to every segment, replay declining their operations. Release the drop pin at max(destination version, proxy version) instead. The proxy's version covers every CoW operation sourced from it (each delete half bumped it), and the durable waterline cannot reach that version until the write segment holding the copies has flushed past it, or has itself been optimized into a built, on-disk destination whose own pin extends the chain. This makes the pin a faithful re-expression of what the proxy enforced while it was a holder member: version above persisted capped the acknowledge. Evidence, from instrumented CI reproductions on this branch: two independent failures showed every lost point CoW-consolidated into a write segment that left the holder with its whole op range unflushed, replay windows containing the operations, and declines on each; a swap-time containment audit stayed silent across ~3000 swaps, ruling out build drops; pin logging showed wrapped sources never above the old release threshold (0/1557), ruling out premature wrapped-data release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
647daf38d7 |
test(model_testing): run the containment audit with zero added lock time
Fourteen instrumented CI passes with the audit inside (or half-inside) the update lock produced zero reproductions against a historical one-in-three run failure rate: the audit was suppressing the race it exists to catch. An evicted proxy is frozen, no writes reach a segment outside the holder and its wrapped files survive until the ack pin releases, so its visible set still equals the swap-time set. Do the whole audit after the swap on the evicted proxies: same containment question, nothing added to the critical section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
5a83939567 |
test(model_testing): move the containment audit out of the swap's critical section
Nine instrumented CI passes with the audit probing inside the update lock produced zero reproductions against a historical one-in-three run failure rate, consistent with the added lock time suppressing the race under investigation. Split the audit: snapshot each source's visible ids and versions under the lock (cheap), probe after the swap outside the critical section. A point absent from the destination is only reported when no holder segment knows it at the snapshot version or above, which excuses CoW moves that legitimately delete from the destination between the swap and the probe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
78d3c62bc5 |
test(model_testing): ledger CoW moves and proxied deletes
A point the model keeps can only leave the engine through a CoW move or a proxied delete, so those two events are a lost point's complete engine history. Log both with the point id and operation number: on a failure, grepping the id reconstructs its last transition and names the segment pair involved, which the containment audit alone cannot do when the loss happens outside the build. Diagnostic branch only; volume is bounded by update rate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f055504960 |
test(model_testing): audit optimized-segment containment at swap time
The fatal window's version arithmetic is fully consistent in every instrumented failure: destinations enter with valid persisted versions, pins release at covered waterlines, the ack never exceeds a surviving segment's claim, and yet points vanish. The one thing none of that arithmetic can express is whether the destination actually contains every live point its sources held: a segment's version is the highest operation it saw, not a containment proof. Check containment directly, under the update lock right before the swap: every point still visible through a proxy was untouched during the optimization, so the build must have carried it. Log an error naming the point, its source version, and the destination's version when it did not. Proxied reads exclude CoW-moved and deleted points, so a hit is a genuine drop, not a propagated delete. Segments in the tester are capped at 10 KB, so the per-swap scan is negligible there. Diagnostic branch only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7551982e7b |
test(model_testing): log acknowledge pins at registration and release
The swap registers an acknowledge pin per evicted proxy at the wrapped source's persisted version, released once the waterline covers the optimized segment's version. The swap log shows 313 of 411 swaps where the departing proxy's version sits above the destination's, so those operations are covered by the destination neither before the release nor pinned after it. Whether that is benign depends on which half of the proxy's version carries them: upserts are rejected on a proxy and land in the write segment, which stays in the holder, but the wrapped source's own version is a different matter. Log both halves at registration alongside the pin and its release condition, and log each release with the waterline that freed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
eb55377969 |
test(model_testing): log segment departures and swap destinations
The acknowledge decision maxes over the segments still in the holder, so data that leaves with a dropped segment is invisible to it. The flush log shows segments leaving while their version is ahead of their persisted version, and the acknowledge then moving past that range, but only as a five-second-stale inference from the previous flush pass, which cannot prove a source was dropped dirty rather than flushed just before. Log the state of every segment at the moment it leaves the holder, and the destination's state on a swap. A source departing with operations the destination's version does not cover is then directly visible, without correlating across flush passes. `remove` is the single funnel for departures, so both swap and replace are covered. Diagnostic branch only: info level. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8c6a34012c |
test(model_testing): log the WAL acknowledge decision per flush
The reload divergence is now pinned to its shape: the acknowledge point advances past an upsert whose data never reached disk, leaving the point unknown to every segment and its write below the replay window. What is still unknown is which input moved the acknowledge point. Log one line per flush pass from `get_max_persisted_version` with the value it chose and which input produced it, the no-op overwrite floor, the maximum segment persisted version, or the lowest unsaved version, alongside every segment's version/persisted pair. Pair it with a line recording what the WAL was actually told after the keep_from cap. A reproduction can then be walked back from the lost point to the flush that acked past its write. Diagnostic branch only: these are info level and fire every flush interval. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
53a5244127 |
test(model_testing): probe segments for points lost across a reload
The reload assert reports which point ids went missing but not why, and the two candidate mechanisms need different fixes. Probe the reopened engine for every lost id and log, per segment, whether it is still known, at which point version, and how the segment's own version compares to its persisted one. An id unknown to every segment means nothing on disk ever recorded the point, so the WAL was acknowledged past a write that only existed in an in-memory pending buffer. An id still known, invisible or at an older version, means the point did reach disk and a later durable write removed or reverted it without its counterpart surviving. The distinction is meaningful because the restart is an in-process close and reopen rather than a crash: the page cache keeps every byte ever written whether or not it was fsynced, so anything genuinely absent was never written at all. Also promote the WAL replay window in `load_from_wal` from debug to info. It is one line per shard load and it is the other half of the postmortem: a point whose write fell below `from` was acknowledged as durable without reaching disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
309c64945a |
[UpdateOnly] wire every component into the appendable segment (#10152)
* [UpdateOnly] wire every component into the appendable segment `AppendableSegment::store_points` sheds its `todo!()`: the id tracker claims a fresh slot per point, every component writes its data at those slots — each named vector storage, the payload storage, the payload indexes — and only then do the versions cover them, the step that makes the points visible to readers. A crash anywhere in between leaves claimed, unpublished slots, which the next writer to open the segment retires. Each vector comes from whichever half of `FullyQualifiedPoint` holds it: the batch's decoded vectors win over the bytes carried from the point's previous slot, and a name in neither still takes its slot as a vector the point does not have. The store components open lazily, on the first `store_points`. A batch that only deletes writes nothing but the mappings log, so it never pays for those opens — and it keeps working against segments whose payload storage was created in mutable mode, which the append-only writers refuse and which is all any leader builds today. The writer now also remembers what it stored, so `tombstone_points` skips a point this very batch wrote instead of retiring its fresh slot; the caller can hand over every slot a stored point used to occupy without holding that rule. `UpdateOnlySegmentEnum::open` takes the segment config, which is where the writer learns which vector storages exist. The end-to-end edge tests now run stores the whole way through: located and resolved through the `LookupSegment`s, appended by the writer, and read back through an ordinary follower — a new point with its payload, a rewrite winning over the old copy, a replayed batch skipping on the published versions, and a second writer resuming every component where the first ended. The leader still writes its payload storage in mutable mode, so the tests recreate it empty in append-only mode, standing in for segment creation wiring that does not exist yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] drop the stored-ids guard from `tombstone_points` The caller already never asks to retire a point its batch stored — it has to hold that rule regardless, since `preview` mirrors it to count outcomes — so the writer-side set was redundant state, and it made `tombstone_points` silently drop requests instead of honoring a stated contract. The contract is now stated: only points the batch deletes go here, because a delete addresses the external id and would take a stored point's fresh slot along with the stale one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] gate the store tests off Windows The leader's writable storage preallocates chunk files, and the append-only writer cuts them back to end at the data — its append offset is a compare-and-swap token, so a file longer than the data would make every append conflict. That cut replaces the file, which Windows refuses while the writer's own `LookupSegment`s hold it memory-mapped; on Linux the old inode simply lives on under the mappings. Nothing to fix in the writer: Windows cannot shrink a mapped file, and the production target is object storage, where neither preallocation nor mmap exists. The delete tests keep running everywhere; the store tests move into a `#[cfg(not(windows))]` module together with the imports and helpers only they use, so the Windows build carries no unused-import warnings. Cross-checked with `--target x86_64-pc-windows-msvc`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] wire the quantized overlay into StoreComponents Opens UpdateOnlyQuantizedVectors alongside each dense, non-multivector, non-Turbo4-datatype vector's raw storage, when the segment's quantization config supports incremental appends (Binary/Turbo). Multivector and Turbo4 combinations are out of scope (see UpdateOnlyQuantizedVectors' own doc comment) — such a vector simply has no quantized overlay entry and stays searchable exactly through its raw storage alone, same as before. store_points keeps the overlay's row count in exact lockstep with the raw storage: every point takes a row in both, in the same order, at the same id (start_slot + offset) — a decoded vector encoded for real, a Raw-bytes-carryover blob decoded back to f32 per its actual storage datatype (mirroring QuantizedVectors::create_impl's use of PrimitiveVectorElement::quantization_preprocess for the same purpose on the non-update-only path), and a Missing vector as an all-zero placeholder. Skipping a row for the latter two cases would silently misalign every later quantized lookup — scoring one point's vector against another's quantized copy — so this mirrors the raw storage's own "every point takes its slot" rule exactly rather than only handling the common decoded case. UpdateOnlyQuantizedVectors now retains its resolved QuantizedVectorsConfig (exposed via quantization_config()/dim()) rather than discarding it after opening storage, since a reopened overlay's persisted config is the source of truth for how to decode carried-over bytes — not necessarily identical to whatever live config the caller has to hand. Its now-unused flusher() is dropped: like every other update-only storage in this stack, a write is already durable when append_many/upsert_vector returns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1a8c9a6397 |
[UpdateOnly] implement the appendable quantized-vector overlay (dense, Binary/Turbo) (#10161)
* [UpdateOnly] implement the appendable quantized-vector overlay (dense, Binary/Turbo) Appendable/plain segments can carry live quantized vectors today: PlainVectorIndex:: update_vector calls quantized_vectors.upsert_vector alongside the raw vector on every insert (lib/segment/src/index/plain_vector_index/lifecycle.rs), auto-created for a fresh segment when appendable_quantization is on and the method supports it (QuantizationConfig::supports_appendable — Binary and Turbo only; Scalar/Product are policy-gated off regardless of storage backend). The update-only vector-storage stack (this PR's base) had no equivalent: UpdateOnlyVectorStorage::open never read quantization_config, and nothing under vector_storage/*/update_only/ mentioned quantization at all — a segment configured with quantization would silently lose it end-to-end once written through this path. This adds UpdateOnlyQuantizedVectors, mirroring QuantizedVectors' auto-create/reopen behavior but scoped to dense (single-vector) Binary/Turbo — the two methods that support incremental appends, matching current capability exactly (multivector support is a follow-up: it needs its own append-only offsets storage, mirroring MultivectorOffsetsStorageChunked the same way this mirrors QuantizedChunkedStorage). The only new machinery is UpdateOnlyQuantizedChunkedStorage, an EncodedStorage backed by UpdateOnlyChunkedVectors (append-only, S: UniversalAppend) instead of ChunkedVectors' positional writes (S: UniversalWrite) — everything else reuses the quantization crate's EncodedVectorsBin::encode/load and EncodedVectorsTQ::encode/load completely unchanged, since both are already generic over the storage backend. It writes files in the exact layout QuantizedChunkedStorage reads, so a promoted segment's quantized data reads through the existing, unmodified reader with no new reading code. UpdateOnlyChunkedVectors gains one addition: a `get` method to read back a single vector, needed because EncodedVectors::load validates the storage's vector size by reading vector 0 (skipped when the store is still empty). Verified: the update-only writer's persisted bytes, read back through the standard (non-update-only) QuantizedChunkedStorage + EncodedVectorsBin/TQ::load, match a RAM-backed reference fed the same vectors one at a time through upsert_vector, byte-for-byte, for both Binary and Turbo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * [UpdateOnly] fix quantized reopen: resume writing shouldn't validate stored reads The previous commit made reopening a non-empty quantized overlay panic (EncodedVectorsBin/TQ::load validates a non-empty store by reading its vector 0, which UpdateOnlyQuantizedChunkedStorage's write-only design cannot serve) and worked around it with a redundant pre-check plus a todo!(), narrowing the tests to single-session-only writes. Both of those were the wrong fix. A writer resuming appends doesn't need `load`'s read-and-validate — it only needs the fitted metadata (encoding, stats) to keep encoding consistently, and that invariant already holds by construction: every vector this writer ever encodes is sized from the same `quantized_vector_size` `load` and the new path both read. Added `EncodedVectorsBin`/`EncodedVectorsTQ::reopen_for_write` to the quantization crate — identical to `load` minus the validating read — and switched `open_existing` to it. `UpdateOnlyQuantizedChunkedStorage` stays write-only as originally designed; no new read capability, no pre-check, no todo. Tests restored to the original two-writer split (write half, drop, reopen, write the rest), now genuinely exercising resume-with-data instead of avoiding it, and still passing byte-for-byte against the reference. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * [UpdateOnly] split EncodedStorage into EncodedStorageWrite + EncodedStorage A write-only storage (the update-only quantized overlay) had to fake a full EncodedStorage impl with unreachable!() read stubs just to satisfy EncodedVectorsBin/TQ's generic bound. Split the trait so a write-only backend only needs to implement EncodedStorageWrite; EncodedStorage adds the read methods on top. The overlay now implements EncodedStorageWrite alone — no panicking stand-ins for methods that don't exist. * [UpdateOnly] remove UpdateOnlyQuantizedVectors::create Nothing in this stack builds the first appendable segment of a collection yet (that's still a todo!() in edge/src/update_only), so create() had no real caller and open() had to guess from file absence whether to invoke it. open() now only reopens an overlay create() already persisted; the bootstrap logic moved into tests.rs as a private fixture helper, since tests still need it to build fixtures. * [UpdateOnly] fix CI: codespell typo and lint dead-code on unwired write path codespell flagged "implementors" (wants "implementers") in two doc comments. Separately, CI's lint job runs clippy without --all-targets, so the update-only quantized write path — genuinely unreachable from any non-test code until #10152 wires it into a segment — trips -D warnings dead-code. Scope #![allow(dead_code)] to the two files that are only exercised by their own tests today, and allow the now test-only UpdateOnlyQuantizedChunkedStorageBuilder re-export. * [UpdateOnly] fix ast-grep: use expect(dead_code) instead of allow * fix CI: remove unused EncodedStorageWrite import in gpu vector storage Left over from splitting EncodedStorage into EncodedStorageWrite + EncodedStorage; only caught under --all-features since gpu is gated behind a feature flag. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: qdrant-cloud-bot <111755117+qdrant-cloud-bot@users.noreply.github.com> |
||
|
|
89f1ae939c |
[UpdateOnly] implement the appendable vector storages (#10151)
* [UpdateOnly] drop the `UniversalWrite` bound from `UpdateOnlyChunkedVectors` Nothing in it needs random-offset writes: the config, the chunk listing and the status file all go through `UniversalReadFs` / `UniversalWriteFileOps`, which `UniversalAppend` already provides. The bound excluded the object-store backend this writer exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] implement `UpdateOnlyDenseVectorStorage` The vectors go into `UpdateOnlyChunkedVectors` and the deleted flags into `UpdateOnlyStoredFlags`, both of which already append; what this adds is the directory layout and the rule for a point with no vector under this name. Such a point still takes its slot, holding a placeholder, and is flagged deleted — slots are shared across every named storage of the segment, so skipping one would shift every later vector of this storage against the id tracker. Only the missing ones are flagged: an unflagged slot reads as present, and the mask is explicitly allowed to be shorter than the vector count, so a batch where every point has a vector rewrites no mask at all. `VectorToStore` is the input, mirroring the two halves of `FullyQualifiedPoint`: vectors the batch decoded, and storage-native bytes carried over from a point's previous slot which are appended without a decode round-trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] implement `UpdateOnlyMultiDenseVectorStorage` Rows here are not indexed by point slot — a point owns a run of them — so this writer tracks where the row space ends and places each run itself, reading the end from the chunked storage on open. A run that would straddle a chunk skips to the next one, as the writable side does, since a read of a multi-vector assumes its rows are contiguous within a chunk. The rows of a batch are therefore not always one span, and each span is appended on its own; the gap a skip leaves is zero-filled by the append that follows it. A point with no multi-vector here owns no rows at all: its offset entry says so. Unlike the single-vector storages there is no row to keep aligned, because the offsets are what map slot to rows. Adds `stored_len` and `remaining_chunk_keys` to `UpdateOnlyChunkedVectors` — the vector count read that #10114 dropped as unused now has a user. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] implement `UpdateOnlySparseVectorStorage` The vectors go into `UpdateOnlyBlobstore` — the sparse storage is the one that was already blob-backed — and the flags into `UpdateOnlyStoredFlags`. A point with no sparse vector stores nothing at all, since the storage is keyed by slot and an unwritten slot is already "no vector"; it is flagged instead. `UpdateOnlyStoredFlags::open` now materializes its directory rather than waiting for the first flag. Storages use that directory as the marker that they exist: `MmapSparseVectorStorage::open_or_create` takes its absence for "not created yet" and starts a fresh storage over the top of the old one. A batch that flags nothing must still leave it behind. Caught by the resume test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] implement the TurboQuant vector storages `UpdateOnlyTurboVectorStorage` and its multivector counterpart. The quantizer is rebuilt from the dimension and distance rather than read back — it carries no learned state, so the two sides encode identically, which the test asserts by comparing the encoded bytes against what the writable storage produces for the same vector. The multivector one places runs of rows exactly as the plain multivector storage does, skipping to the next chunk rather than straddling one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] add `UpdateOnlyVectorStorage`, the dispatch over the five families Selects the writer from the vector config the way the writable side selects the storage, and refuses a storage type an update-only segment cannot have: the mmap ones are built whole rather than appended to, and the empty placeholder has no files. Sparse gets its own opener, since sparse vectors are configured separately from dense ones rather than through `VectorDataConfig`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix clippy under `-D warnings` - `is_multiple_of` in place of the manual remainder checks in the two multivector writers. - Drop the `dead_code` expectations on `UpdateOnlyChunkedVectors`: the vector storages use it now, so the expectation no longer holds. - Drop a `TypedMultiDenseVectorRef::from` that converts to its own type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] fix a stale doc and an inconsistent guard The doc on `UpdateOnlyStoredFlags::open` still said nothing is created until the first flush, from before open started materializing the directory eagerly. And the span-merge guard in the multivector writer hedged with `dim.max(1)` while the same function divides by bare `dim` three lines up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] append a multivector batch's rows once, with the gaps as zero rows The chunk layer packs rows consecutively while runs must not straddle a chunk, so a batch's rows are not gapless. The old bridge grouped them into contiguous spans and appended each on its own, leaning on `ensure_chunk_lengths` — the repair path — to zero-fill the gap before every span, and saving the status once per span. Making the gaps explicit zero rows removes all of that: one append per batch, through the normal write path, one status save. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e95f56861b |
[UpdateOnly] implement UpdateOnlyStructPayloadIndex, the per-segment fan-out (#10150)
* [UpdateOnly] implement `UpdateOnlyStructPayloadIndex`, the per-segment fan-out Every field index of one segment, opened for a batch and dropped with it — the update-only counterpart of `ReadOnlyStructPayloadIndex`, and the level `AppendableSegment` needs: it takes the points a batch stores and leaves every index of every indexed field current. It reads which indexes a field has from the payload config, exactly as the read-only side does, and holds nothing else. No payload storage, no id tracker, no vector storages: those are there to answer queries and to work out what an update means, and by the time a batch reaches here that is settled — each point arrives with the payload it will be stored with. Every field is offered every point, including points whose payload holds nothing under it. An index that stores values per point stores none for those; the null index records that the point has no value there, which is the whole reason it exists. That is simpler than the writable path's add-or-remove split, which is only needed because a slot there may already hold something. A field whose index types the config does not spell out is refused. That config predates those types being recorded, and the writable index repairs it by deriving them from the schema on its next open; this writer builds no indexes, so it cannot, and going on would leave whatever is on disk to rot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] trim the doc comments on the payload index fan-out Keep the guarantees and the non-obvious rationale, drop the restatements. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9d1894d311 |
[UpdateOnly] implement UpdateOnlyFieldIndex for the appendable payload indexes (#10147)
* [UpdateOnly] implement `UpdateOnlyFieldIndex` for the appendable payload indexes The payload index write half for the update-only segment writer, over a backend that only appends. An appendable field index keeps two things: the values it persists per point, and the in-memory structure it answers queries from. Only the first is state — the second is rebuilt from it on every open, by the mutable index and by its read-only counterpart alike. A writer that never answers a query therefore holds nothing: it turns a point's payload into the values its index would persist, appends them at the point's slot, and is done. What differs between index types is only that translation, so that is all `UpdateOnlyIndexKind` captures; `UpdateOnlyValueIndex` is the storage around it, the same for all of them, and each kind lives next to the index it writes for as the read-only counterparts do. The extraction itself is taken from the index types' own `ValueIndexer` and `NumericIndexIntoInnerValue` impls rather than restated, so the two sides cannot drift apart. `UpdateOnlyFieldIndex` dispatches over the nine covered index types, mirroring `ReadOnlyFieldIndex`. What the writer emits is the append-only mode of the very same storage the mutable index writes, and `Blobstore` selects the mode from the persisted config, so the read side needs no change: every test here writes through the update-only writer and reads back through the ordinary appendable index, opened on the directory the writer produced. The boolean and null indexes are not covered and are refused loudly rather than skipped. They keep a bitmask over all points instead of values per point, and persist it through random-offset writes, which an append-only backend does not offer. A skipped index goes stale and then answers queries wrongly, and the null index complements every other index of every indexed field — so a caller that took a silent skip for "nothing to do" would leave every field it touched wrong. Covering them needs an append-only bitmask representation first. That is also why this stops short of the struct-payload-index fan-out: until bool and null can be written, a component that claims to keep a field's indexes current could not. `UpdateOnlyPayloadStorage` moves onto the shared `UpdateOnlyBlobstore` extracted here, which is what it already was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] share the flattening, tokenization and flush guard Cleanup pass over the field-index writers, from a reuse/simplification review. Share what was restated: - `ValueIndexer::flatten_values` is now a provided method holding the loop that `add_point` had inlined. The update-only kinds call it instead of the free `extracted_values`, which built one throwaway `Vec` per input value on top. - `FullTextIndex::tokenize_document` and `serialize_stored_document` hold the sentinel placement and the phrase-matching order-vs-sort decision that the update-only kind had copied out of `MutableFullTextIndex::add_many`. Both sides call them, so a document written by one always matches the phrases the other would. Simplify: - `UpdateOnlyFieldIndex::open` matches on the index type alone and takes the text params via `TextIndexParams::try_from`, as `ReadOnlyFieldIndex::open` does. That drops the schema tuple, the nine-arm mismatch block and the `Option` return. - The `UuidIndex` variant is gone: that discriminant is historically map-backed, and both the writable selector and the read-only mirror already collapse it into `UuidMapIndex` — its `storage_dir` is `map_dir`, so a numeric-kind writer was writing into a directory everyone else opens as a map index. - Why bool and null cannot be written append-only now lives in `PayloadIndexType::is_append_only_writable`, next to `storage_dir`, so that whoever decides a field is update-only-serviceable can ask rather than rediscover it; `open` consults it as a backstop. - Dead `new()` constructors on the two zero-sized kinds. Skip the flush when nothing was buffered, in `UpdateOnlyBlobstore` rather than in one caller: a flush with nothing to write still syncs every page file of the storage, and for a field index an empty batch is the common case — every point that lacks the field, or holds a value the index rejects, stores nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] move each index kind under the appendable index it writes for `numeric_index/update_only.rs` and its three siblings sat at the index-type level, next to the enum over all three storage variants, although each writes for the appendable variant alone. They now live at `<index>/mutable_<index>/update_only/`, beside that variant's `read_only/` counterpart, which is the same split for the same reason. `mutable_text_index` is private, so the text kind is re-exported from `full_text_index` for the dispatch enum to name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] cover the bool and null indexes by rewriting their masks whole These two keep a bitmask over all points rather than values per point, so keeping one current means changing bytes in the middle of it — which an append-only backend cannot do. It can replace a file outright, and that is enough: `UpdateOnlyStoredFlags` reads the mask into memory on open, sets the batch's bits, and writes both files back whole, in the same format `DynamicStoredFlags` uses. A reader cannot tell which side produced them. The mask goes out before the length that publishes it, so a torn batch falls back to the shorter mask rather than to flags that were never written, and the whole-file write is charged to the hardware counter at flush — it is the write that actually happened, not the handful of bits the batch touched. `UpdateOnlyBoolIndex` and `UpdateOnlyNullIndex` sit on that, next to their mutable index like the other kinds. The null classification (which values count as present, which as null) moves into `classify_payload`, shared with `MutableNullIndex::add_point`, and the boolean one reuses that index's own `ValueIndexer`. Both are recorded for every point of a batch, including those whose field holds nothing: "this point has no value here" is precisely what these indexes are asked. With that, `UpdateOnlyFieldIndex` covers every index type `ReadOnlyFieldIndex` does, so the refusal and `PayloadIndexType::is_append_only_writable` are gone. The cost is that a batch rewrites the entire mask however few bits it touched — about 1.2 MiB per flag set for a segment of ten million points. Documented on the writer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] trim the doc comments on the field index writers Keep the guarantees and the non-obvious rationale, drop the restatements and the comments that narrate the next line. One code change: `values.contains(&true)` in place of `values.iter().any(|value| *value)` on the boolean index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c8261beaec |
[UpdateOnly] implement UpdateOnlyPayloadStorage (#10146)
* [UpdateOnly] implement `UpdateOnlyPayloadStorage` The payload write half for the update-only segment writer: a short-lived storage opened for one batch and dropped with it, over a backend that only appends. Backed by a `Logstore` — the append-only mode of the same storage the writable `PayloadStorageImpl` uses — so a slot's payload is written once and never rewritten. `append_many` takes one payload per point at the slot the ID tracker claimed for it and flushes, so a batch is durable when the call returns and nothing is buffered across calls. Puts only buffer, so the flush is what touches the files: one append per touched page file plus one to the tracker, regardless of how many points the batch holds. A point with an empty payload is skipped, since an unwritten slot already reads back as an empty payload, and so is any gap between slots, which the tracker materializes as unmapped entries. `Logstore` had to leave the `Blobstore` facade for this: `Blobstore`'s type is bound at `UniversalWrite + UniversalAppend` for the sake of its `Gridstore` variant, so it cannot be named on a backend that only appends. Its cross-crate surface is `open_or_create`, `put_value` and `flusher`, nothing more; the new `open_or_create` mirrors `Blobstore`'s and rejects a storage created in mutable mode rather than opening it. Not wired into `AppendableSegment` yet — `store_points` stays `todo!()` until the vector storages and field indexes exist, as with `UpdateOnlyChunkedVectors`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] trim the doc comments on the payload storage writer Keep the guarantees and the non-obvious rationale, drop the restatements — the merged-baseline style of `UpdateOnlyChunkedVectors` and `AppendableSegment`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c5f32f1315 |
build(deps): bump num-derive from 0.4.2 to 0.5.1 (#10174)
Bumps [num-derive](https://github.com/rust-num/num-derive) from 0.4.2 to 0.5.1. - [Changelog](https://github.com/rust-num/num-derive/blob/main/RELEASES.md) - [Commits](https://github.com/rust-num/num-derive/compare/num-derive-0.4.2...num-derive-0.5.1) --- updated-dependencies: - dependency-name: num-derive dependency-version: 0.5.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
f206bbca68 |
build(deps): bump serial_test from 3.5.0 to 4.0.1 (#10169)
Bumps [serial_test](https://github.com/palfrey/serial_test) from 3.5.0 to 4.0.1. - [Release notes](https://github.com/palfrey/serial_test/releases) - [Commits](https://github.com/palfrey/serial_test/compare/v3.5.0...v4.0.1) --- updated-dependencies: - dependency-name: serial_test dependency-version: 4.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
911d6a2afa | perf(common): add zero copy fast path for multi-block disk cache hits (#10108) | ||
|
|
6a4a1fc4d4 | test(model_testing): log run stages and fix nightly failure reporting (#10163) | ||
|
|
4c9d124ab4 | Move deferred iter into PointMappings. (#10156) | ||
|
|
1d18e46551 |
[UpdateOnly] split UpdateOnlySegment into its lookup and writer phases (#10142)
* [UpdateOnly] split UpdateOnlySegment into its lookup and writer phases Applying a batch runs in two phases that agree on almost nothing, and `UpdateOnlySegment` was both: `resolve.rs` used every field, `append.rs` used none of them and could not — a `ReadOnlyPayloadStorage` has no append path. The `fs` field existed only for the writes that were never wired up. Split along that line: * `LookupSegment` (was `UpdateOnlySegment`) is the read phase. Every segment of a shard is opened as one, on read-only bounds, and the phase above them aggregates. Loses the dead `fs` field. * `DeleteOnlySegment` and `AppendableSegment` are the write phase, one segment each, `UpdateOnlySegmentEnum` over the two. Opened for one batch and dropped with it, matching the append-only components, which buffer nothing across calls. The phases meet at `SegmentWriterState`, produced by `LookupSegment::writer_state` and consumed by `UpdateOnlySegmentEnum::open`. It carries the mappings-log tail an appendable writer resumes from, which `UpdateOnlyAppendableIdTracker::new` requires to come from one and the same read of that log. The writer kind follows the id-tracker format that was loaded, not the segment config: the format decides how a point is retired. That difference makes `tombstone_points` take both ids, `(external, slot)`; an immutable segment marks the slot in its deleted-points bitmask, an appendable one records a retirement for the id in its mappings log. The appendable half is implemented — deletes now run end-to-end. `store_points` and the immutable bitmask remain `todo!()`, still waiting on the append-only storages and field indexes. Two bugs surfaced while wiring it up: * A point stored into the write target must not have its old slot retired there: appending records a mapping that supersedes it, and retiring the id on top would take the new slot with it. * A second `apply_batch` through one writer resurrected deleted points. It resumed the log from the `mappings_end` its own first batch had moved past, and appending there cut that batch's entries off. Refused now; lifting it means reloading the segments after a batch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] one batch per writer, enforced by the type system Cleanup pass over the phase split. `apply_batch` now takes `self`. It could only ever serve one batch — the segments are read when the writer opens, and that read is both what a batch resolves against and what its writers resume from — and the runtime guard enforcing that cost a flag, its doc, two imports, a hand-maintained `writes_anything` condition, an error branch and a test. Consuming the writer makes the second call a compile error instead. Also: * drop `LookupSegment::uuid`, which nothing ever read, along with the two parameters and the argument that fed it; * `AppendableSegment::tombstone_points` was a copy of the tracker's own `retire_pending_inserts`; both now go through `delete_points`; * fold the duplicated "segment disappeared mid-batch" error into `LookupSegmentHolder::get`, and restore `write_target_uuid` as an `Option`, which is what two of its three callers wanted; * one fixture helper for the writer tests instead of three copies; * state the mappings-log co-read invariant once, with pointers, instead of three times. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] cut the writer surface down to what it does * `flush()` is gone from both writers and the enum. Both bodies were `Ok(())` and would stay that way: the id tracker persists what it writes before returning, and the deleted-points bitmask writer does not exist yet. The ordering it looked like it enforced — new slots durable before the tombstones retiring the old ones — falls out of call order, since every write is durable when it returns. Bring it back with the first storage that buffers. * `SegmentWriterState` was an enum of one unit variant and one payload, which is `Option`. `writer_state()` returns `Option<AppendableIdTrackerState>`, and `None` reads as what it means: no mappings log to resume, so a delete-only writer. * `LookupVectorData` wrapped a single `Arc<AtomicRefCell<_>>`; the map holds it directly now. * `appendable` joins the five `pub` fields around it, and `is_appendable()` goes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
2805fe4e2a |
fix: stop underestimating is_empty / not-null cardinality by 1/3 (re-introduce #10128) (#10141)
* fix: stop underestimating is_empty / not-null cardinality by 1/3 Re-introduce #10128 after its revert so CI can exercise payload_index_test::test_read_operations / test_is_empty_conditions. * test: stop requiring is_empty struct exp to beat plain NullIndex complement estimates use an indexed upper bound (may include soft-deletes); that is not guaranteed to be closer to truth than plain's available/2 guess. Assert upper-bound semantics instead. * test: drop is_empty exp==max assertion That locked in NullIndex implementation detail. Keep result parity and min/max bounds only; document why exp-vs-plain is not checked. |
||
|
|
2c024ba037 |
[UpdateOnly] implement UpdateOnlyChunkedVectors (#10114)
* AI + manual: impl `UpdateOnlyChunkedVectors` * AI: simplify * graceful handling of unexpected file lengths fix test * incorporate updates from #10119 * drop the unused status read on open The vector count loaded at open was never consulted: every batch carries the offset it starts at, and the chunks are reconciled against that offset. Drop the field and the read, and fold both watermark writes into `save_len`. A corrupt status file no longer blocks opening the writer — the first batch overwrites it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix clippy: ensure_chunk_lengths no longer needs &mut self Dropping the status field left it with nothing to mutate. `append_many` keeps `&mut self` — nothing in this module is exported, so the lint reaches it too, but the exclusive borrow is what enforces the single-writer contract the appends rest on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: generall <andrey@vasnetsov.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
34d3f35fd7 |
Revert "fix: stop underestimating is_empty / not-null cardinality by 1/3 (#10128)" (#10140)
This reverts commit
|
||
|
|
50fe2e8139 |
fix: stop underestimating is_empty / not-null cardinality by 1/3 (#10128)
NullIndex used an arbitrary `exp = 2/3 * estimated` heuristic for complement conditions (`is_empty=true`, `is_null=false`), which caused steady-state approximate counts to under-report by ~35% even with no deletes (see #10120). Use the indexed upper bound as the expected count instead. |
||
|
|
b60f298dea |
Add UpdateOnlyAppendableIdTracker, the append-only ID tracker writer (#10093)
* Add UpdateOnlyAppendableIdTracker, the append-only writer The write counterpart of `ReadOnlyAppendableIdTracker`, producing the two files that tracker already consumes — `mutable_id_tracker.mappings`, an append-only log of mapping changes, and `mutable_id_tracker.versions`, a dense array of one version per slot — through `UniversalAppend`, so the same code drives a local file and an object store. `insert_operations` records a batch of `MappingOperation`s in order: an insert claims the next slot above the highest one in use and reports it, a delete retires an external id and claims nothing. Nothing is rewritten in place, so re-inserting a live id moves it to a fresh slot and supersedes the old one — the update-only shape of an update. `set_internal_versions` extends the versions array. Ids may come in any order but must be exactly the slots the array does not cover yet: a slot below the end would need an in-place overwrite, and a hole would have to be zero-filled — and since "covered by the versions file" *is* the commit signal for readers, that would publish a slot as a live point of version 0 before its data exists. Both are rejected rather than written. Both methods append at an offset they probed for, never at an implicit end: the offset is a compare-and-swap token, so a file that has moved on since the probe is rejected instead of being written twice or in the wrong place. Both have persisted what they wrote when they return `Ok` — append, then run the handle's flusher — and nothing is buffered across calls. The order of the two calls, claim the slot then commit the version, is what makes a crash in between safe: readers ignore slots the versions array does not cover. Cleaning up the slots such a crash abandons is left to the opener, along with repairing a torn tail; the writer fails loudly rather than guessing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Share the versions file format between both write paths `set_internal_versions` was reimplementing what `versions_storage` already knew: entries are `SeqNumberType`-sized, slot `n` lives at `n * VERSION_ELEMENT_SIZE`, a file length that is not a whole number of entries has a torn tail. Every one of those facts existed in two to four places, spelled as inline `/`, `%` and `write_u64::<FileEndianess>`, so the append-only writer could drift away from the in-place one silently. Move them into `versions_storage`, which now owns the format for both writers and both readers: - `write_version` / `read_version`, the entry codec, with a static assertion tying its `u64` to `VERSION_ELEMENT_SIZE` so a change to `SeqNumberType` cannot silently shrink every offset; - `version_offset` and `versions_byte_len`, the slot arithmetic; - `VersionsLayout`, which splits a file length into committed entries and a partial tail. The two writers still react differently — the in-place one truncates the tail, the append-only one refuses it, because an append cannot — but they no longer each work out what the tail is. `store_version_changes`, `load_versions`, `set_internal_versions` and the read-only tracker's live reload all go through it. The write loops themselves stay separate: one seeks to sparse offsets, the other emits a validated consecutive run, and merging them would obscure both. What they share is where the bytes go, which is the part that must not diverge — and a new test pins it down by writing the same versions through both writers and comparing the files byte for byte. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Append mapping and version entries as batches Both writers built one concatenated buffer and handed it to `append`. `append_batch` takes the entries as separate buffers and places them in a single operation — a vectored write locally, one request on an object store — so the entry boundaries reach the backend instead of being flattened away first. Versions are fixed-size, so the entries are the payload's `chunks_exact`. Mapping changes are variable-length, so their bounds are recorded as they are serialized. Both keep the compare-and-swap offset, which `append_batch` validates the same way `append` does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test the writer against MutableIdTracker end to end The suite leaned on round-trips through the storage loaders, which only restated what the writer had just written. Four such tests are replaced by one that checks the property actually worth having: drive the append-only writer and `MutableIdTracker` with the same points, versions and deletes, open each segment through `ReadOnlyAppendableIdTracker`, and require the two views to be indistinguishable — counts, deleted state, external ids, live points' versions, and id resolution. Versions are compared for live points only. `MutableIdTracker::drop` overwrites the slot with `DELETED_POINT_VERSION`, which an append cannot do, so the append-only writer leaves the point's original version there; neither is observable for a point that is gone. The remaining tests keep what a round-trip cannot show: slot allocation across calls, instances and deletes (three tests folded into one), the rejection of holes and rewrites, and the byte-for-byte agreement of the two writers on the versions file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * better buffer names * fmt * Heal a torn versions tail instead of refusing to write A writer that dies mid-entry leaves the versions file ending inside a slot. The in-place writer already truncated that tail before writing; the append-only writer refused, which left the file unwritable forever since an append cannot truncate. Share the decision — what counts as torn, the healthy length, the warning — in `heal_versions_tail`, and let each writer supply the shrink its backend can do: `set_len` in place, or reading the committed prefix back and putting it in place as a whole file where there is no truncate. Dropping the tail loses nothing: the array covers a slot only once its whole entry is there, so a partial entry belongs to a slot no reader ever saw and no writer counted as committed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Append mappings at the end of the log, not the end of the file Mapping entries vary in length, so no length tells you whether the log ends on an entry boundary. Appending at the file's end therefore could not fail: a torn entry was silently appended after, and every entry from there on was framed off the stray bytes. Carry the boundary instead. `new` takes the offset just past the last complete entry — `ReadOnlyAppendableIdTracker::mappings_read_to`, from the same view that supplies `max_internal_id` — and appends there, which turns a file ending elsewhere into an append offset conflict. On that conflict `heal_mappings` cuts the file back to the log's end, the same read-prefix-and-rewrite the versions file heals with, and writes the batch again. A torn entry and a batch that landed unacknowledged are indistinguishable without parsing, and need not be told apart: neither `max_internal_id` nor `mappings_end` moves before an append is durable, so the retry writes the same bytes at the same offset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move the healing functions into their own file `update_only/mod.rs` had grown to hold the writer, its two public write paths and the two repair routines they fall back on. Split the latter out: `heal_versions` and `heal_mappings` move verbatim into `update_only/heal.rs`, as a second impl block, following the layout the read-only half already uses (`lifecycle.rs`, `live_reload.rs`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Retire inherited pending inserts when opening the writer A slot is spoken for from the moment the mappings log claims it — components may already have written data at it — so the writer must resume above every slot the log ever handed out. Deriving that bound from a reader's point set undercounts it twice over: a claimed slot whose version was never committed is not in the mapping, and one whose external id was deleted afterwards is not among the pending inserts either. Track it in the log instead, as `ReadOnlyAppendableIdTracker::max_claimed_internal_id`, bumped on every insert entry regardless of what becomes of the point, and take it as `UpdateOnlyAppendableIdTracker::new`'s bound. The points on those claimed-but-unversioned slots are the other half. They cannot be adopted: a writer stopped partway through them, so some components hold their data and others do not, and which is unknowable here. They cannot be left alone either, the versions array being dense — covering any slot above one of them publishes it, half-written. So `new` now takes the pending inserts explicitly and retires them, recording a `Delete` per id before the writer can be used at all, which is what makes it fallible. Doing it at construction rather than lazily on the first write means no write path can be added later that forgets to. `set_internal_versions` accordingly stops rejecting holes: it writes the whole run from the end of the array through the highest id given, covering skipped slots with `DELETED_POINT_VERSION` as the in-place writer's seek already does. It gains an upper bound in exchange — publishing a slot means covering every slot below it, so an id the log never claimed is refused. Live-reload semantics are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Skip UpdateOnlyAppendableIdTracker heal tests on Windows Healing replaces the file via atomic_save while an mmap handle is still open, which Windows denies with os error 5. * Drop mmap handles before healing ID tracker files atomic_save cannot replace a path while an mmap is still open on Windows. Copy the committed prefix, drop the handle, then rewrite and reopen. * Trim ID tracker docs and drop redundant helpers Condense the doc comments on the update-only tracker to the style of the sibling modules, merge a duplicate impl block, and remove `read_version` and a debug assert that restates its own operands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop the versions layout helpers and inline the arithmetic VersionsLayout, versions_byte_len and version_offset wrapped one divmod and one multiplication between them, and adopting the struct made the live-reload hunk longer than the line it replaced. Compute the committed length where it is needed instead, and let heal_versions_tail and heal_versions return unit, since no caller used the layout they handed back. Also drop the writer's unused max_claimed_internal_id accessor, and fold retires_inherited_pending_inserts_at_construction into retires_inherited_pending_inserts: the merged test asserts the retirement happened before the writer did anything, and commits a real version to the retired slot rather than letting it take the filler, so it still shows the Delete is what hides the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: qdrant-cloud-bot <111755117+qdrant-cloud-bot@users.noreply.github.com> |
||
|
|
7c0948628d |
Batched deletion checks in full-scan/exact search (10-35% faster Turbo4) (#10042)
* Batched bitmap checks in full-scan/exact search Instead of walking the deleted bitmap bit by bit and re-checking every point, the new peek_top_visible reads all bitmaps one u64 at a time and handles 64 points in one step. About 1.5x QPS on unfiltered full scans. * Remove debug statement * Extract parallel bitmap scaning into separate helper * Optimizing `BatchedBitmapScan` * Also apply to HNSW full-scan fallback |
||
|
|
2ef887bae1 | fix(edge): skip pool install for single search thread (#10118) | ||
|
|
c8e7ef80a8 |
Batched HNSW: preliminary refactoring (#10052)
* [1] refactor: EntryPoint: derive Copy
* [2] refactor: extract GraphLayers::probe_links_format
* [3] refactor: merge …/graph_links/{links, storage}.rs
* [4] refactor: GraphLinks: inline GraphLinksEnum methods
* [5] refactor: graph_links/view.rs split into view_utils.rs
Later these utils would be used in links_file.rs.
* [6] spelling: clarify error_size
* [7] refactor: TestGraphLinksVectors::{assert_base_vector, assert_link_vector}
* [8] refactor: extract entry-point selection out of GraphLayers::search
* [9] refactor: Introduce GraphWithVectorScorers
* [10] refactor: extract load_or_derive_config
|
||
|
|
f4ad4f4c25 |
chore(deps): drop dead dependencies in edge-path crates (#10109)
- blobstore: move `dataset` to dev-dependencies (test/bench only) - shard: remove unused `fs4` - segment: move `tap` to dev-dependencies (test/bench only) - sparse: move `tempfile` to dev-dependencies (test only) Removes the `dataset -> reqwest -> hyper/tower/h2` root from the `edge` dependency graph. |
||
|
|
0a6cb3b4cf |
[Raw payloads]: read payload as stored bytes in retrieve_raw (#10040)
* segment: read payload as stored in retrieve_raw `retrieve_raw` already hands back vectors as stored; let the caller ask for the payload the same way, so a reader that only relocates a point parses nothing. `RawPayloadFormat` states what the caller wants — no payload, parsed, or as stored — and replaces the `WithPayload` argument, which could express a key selection that a raw read cannot serve anyway. [`MaybeRawPayload`] states what came back, which can differ from the request in one direction only: a payload storage that keeps payloads parsed cannot answer `Raw` with a blob, and now says so instead of encoding a payload for a reader that would parse it straight back. The raw path reaches the blobstore through `read_payloads_maybe_raw`, mirroring `read_payloads` down the payload storage and payload index traits, so it keeps the batched read. Every caller asks for `Parsed`, so this changes no behaviour: the copy-on-write move and the sync comparison need the parsed payload anyway, and the shard transfer switches over with the feature flag that ships the blob to another node. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * segment: always hand out the stored payload blob from retrieve_raw Review follow-up: instead of telling `retrieve_raw` in which form to return the payload, it always returns it as stored and a caller that needs the parsed form decodes it itself. - Drop `RawPayloadFormat` and the payload parameter it replaced: no production caller ever asked for anything but the whole payload, and a selector cannot be applied to an opaque blob anyway. - Drop `MaybeRawPayload` / `MaybeRawPayloadRef`: only `InMemoryPayloadStorage` could produce the parsed variant, and no segment can be built with that storage (`PayloadStorageType` is `Mmap` or `InRamMmap`, both blobstore-backed). `SegmentRecordRaw` carries a plain `Option<RawPayload>`. - `PayloadStorageRead::read_payloads_maybe_raw` becomes `read_payloads_raw` and hands out `Option<&[u8]>`. The in-memory storage keeps payloads parsed, so it encodes on read, producing the bytes an on-disk storage would have written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * api: decode a received raw payload with the shared decoder `decode_payload` at the gRPC boundary matched on the encoding and parsed the blob itself, duplicating `RawPayload::decode`. Add the inbound conversion from the wire type and let the one decoder do the reading, so another encoding has a single place to be taught. The conversion also rejects an encoding number no variant maps to, which prost would otherwise hand out as the default encoding — a blob from a node that writes payloads some other way must not be read as JSON. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * simplification --------- Co-authored-by: Ivan Pleshkov <ivan.pleshkov@qdrant.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0805d3f422 |
Add /profiler/consensus_lag to measure apply lag between peers (#10090)
* Add /profiler/consensus_lag to measure apply lag between peers Raft commit index advances on a peer whose apply loop is stalled, so the existing signals - `raft_info.commit` and the `all_nodes_have_same_commit` test helper - report a stuck peer as healthy. Nothing exposes how long a peer has been behind at *applying* entries, which is what shard transfer's `await_consensus_sync` barrier actually waits on. Each peer now keeps a ring of the last 32 entries it applied, stamped with its own wall clock and the time that entry took to apply. The ring is in memory on ConsensusManager, not in Persistent, so the on-disk format is untouched. `/profiler/consensus_lag` collects those rings from every peer over a new internal RPC and lines them up on the entry indices they share. Each entry is measured from whichever peer applied it first, so a lag is never negative; the peer that is first can differ per entry, so the baseline is per entry rather than a single chosen peer. Entries only one peer still remembers are excluded, otherwise a peer would be measured against itself. A peer stalled part-way through an entry keeps healthy lag statistics - everything it did apply, it applied on time - so the report carries `behind_entries` and `newest_applied_age_ms` alongside, which is what actually exposes the stall. Peers that fail or time out are listed rather than failing the request: a partial answer is more useful than none when the point is to find a peer that stopped answering. The endpoint follows `/profiler/slow_requests`: manage access, and outside OpenAPI, so no endpoint-count or ACTION_ACCESS guard applies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move applied-entry log into its own module Keeps the new code out of files that are already large. The ring, its entry type and the snapshot served over RPC move to `content_manager/consensus/applied_log.rs`, alongside the other consensus internals; `ConsensusManager` is left with a field, an accessor and the one `record` call in the apply loop. The grpc encoding moves next to the decoding it mirrors, in `common/consensus_lag.rs`, leaving the internal service handler three lines instead of thirty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test that a consensus stall is still in the report after the peer catches up * Take each peer's applied index from consensus state, not its ring --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: tellet-q <elena.dubrovina@qdrant.com> |
||
|
|
7364cc42ef |
feat(edge): add query_batch for batched planned queries (#10100)
* feat(edge): add query_batch for batched planned queries Expose the planned-query batch path as a public API so multiple independent queries can share one planning pass over leaf searches and scrolls. Wired through EdgeShardRead, FFI, and Python bindings. Co-authored-by: Cursor <cursoragent@cursor.com> * perf(edge): push batched query vectors down to segments `query_batch` planned the whole batch at once but then executed every leaf search on its own: one query context, one fan-out over all segments, and one single-vector `Segment::search_batch` call per leaf. Execute the batch as a batch instead: - `EdgeReadView::search_batch` builds the query context once, visits the segments once, and hands each segment the leaves that agree on everything but their query vector as a single multi-vector `search_batch` call. `search` is now a thin wrapper over a one-element batch. - Move `SearchType`/`BatchSearchParams` from `collection`'s segments searcher into `shard`, next to `CoreSearchRequest`, and add `group_search_batches` so both the collection and the edge read path share one grouping implementation. Edge computes the grouping once and reuses it per segment. - `search_matrix` now issues its per-sample nearest queries through `query_batch`; they share filter, limit and vector name, so the whole sample is scored in one batched search per segment instead of one full segment pass per sampled point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: generall <andrey@vasnetsov.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
908cd2f10a |
[UIO] Pin the append filesystem to its handle (#10092)
`UniversalRead::Fs` was reachable from an append handle and known to be
a `UniversalWriteFileOps`, but not pinned: `fs.open_append(..)` on it
returned some `Fs::AppendFile`, not the handle type in hand. Pin it the
way the read side is pinned, so `S::Fs` both opens `S` for reading and
hands `S` out as its append handle:
UniversalRead<Fs: UniversalWriteFileOps<AppendFile = Self>>
Every append handle's canonical filesystem already produced itself
(`MmapFs → MmapFile`, `IoUringFs → IoUringFile`, `BlobFs<A> →
BlobFile<A>`), so this only writes down what held — the workspace
compiles unchanged.
Generic-over-`<S: UniversalAppend>` code now reaches its file-creating,
append-opening filesystem as `S::Fs` with no second associated type, and
`&impl UniversalWriteFileOps<AppendFile = S>` accepts any other producer
— the mirror of `&impl UniversalReadFs<File = S>`.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|