mirror of
https://github.com/qdrant/qdrant.git
synced 2026-09-21 13:37:46 -05:00
5939b0a271a63a7dbfa4bcc4f5c7e5feac711c39
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
9d71591d3f |
feat: ReadOnlySegment read view + ReadSegmentEntry impl (#9398)
* feat: ReadOnlySegment read view + ReadSegmentEntry impl Add the read_only segment module: ReadOnlySegment / ReadOnlyVectorData, a `with_view` builder mirroring `Segment::with_view` (with a `ReadOnlySegmentReadViewFor` alias and a `VectorDataRead` impl for `ReadOnlyVectorData`), and a `ReadSegmentEntry` impl that delegates to the shared `SegmentReadView`. A read-only segment is never appendable, so `is_appendable()` is `false` and the view builders receive `false`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Merge pull request #9400 * fix: pass multi_vector_config in multi read-only live_reload test * feat: make VectorIndexReadEnum usable for ReadSegment * feat: use ReadOnlyStructPayloadIndex in ReadOnlySegment Replace the placeholder in-memory StructPayloadIndex with the storage-generic ReadOnlyStructPayloadIndex<S>, mirroring how the read-only HNSW index already wires its payload index. The segment read view now nests the read-only payload-index view generics (ReadOnlyPayloadStorage / ReadOnlyIdTrackerEnum / VectorStorageReadEnum / ReadOnlyFieldIndex). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Daniel Boros <56868953+dancixx@users.noreply.github.com> |
||
|
Andrey Vasnetsov
|
a52983615e | implement ReadOnlyStructPayloadIndex and patch places where it should be used (#9403) | ||
|
generallandClaude Opus 4.8
|
2f4b39cc6b |
fix: post-rebase trait integration for batched_reader and TQDT
Import the *Read traits in batched_reader (get_dense/get_multi/ get_sparse_opt moved onto them) alongside the write traits it still needs for update_from, and drop DenseTQVectorStorage's duplicate size_of_available_vectors_in_bytes default now that VectorStorageRead mandates it (it was ambiguous for TurboVectorStorage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
2217749384 |
fix: drop resurrected quantized_vector_layout stub from rebase
The rebase conflict resolution kept TurboVectorStorage::quantized_vector_layout,
a dead unimplemented! stub that dev removed in
|
||
|
generall
|
1e6810bd4a | renaming | ||
|
generallandClaude Opus 4.8
|
24d7a72d49 |
feat: split Multi/SparseVectorStorage read traits, score read-only multi/sparse
Split MultiVectorStorage and SparseVectorStorage into read-only supertraits (MultiVectorStorageRead / SparseVectorStorageRead, holding the scoring methods) plus the write-only update_from, mirroring the DenseVectorStorage split. Read-only storages can now implement the read traits, which they could not before because of update_from. Re-bound raw_multi_scorer_impl / raw_sparse_scorer_impl and the four multi/sparse query scorers on the *Read traits, and implement the read traits for ReadOnlyChunkedMultiDenseVectorStorage and ReadOnlySparseVectorStorage. The read-only multi storage now carries its MultiVectorConfig (threaded through open()). VectorStorageReadEnum::build_raw_scorer now scores the multi and sparse variants via raw_multi_scorer_impl / raw_sparse_scorer_impl instead of returning a service_error, so read-only multi and sparse storages are fully scorable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
974ab87fcc |
refactor: hoist size_of onto VectorStorageRead, generalize plain read view search
Move size_of_available_vectors_in_bytes onto the VectorStorageRead trait (it was split across DenseVectorStorageRead / MultiVectorStorage / the enum inherent impls), implementing it for every VectorStorageRead impl. This makes per-vector size queryable through a generic V: VectorStorageRead, behavior-preserving. With that, move the plain read view search/is_small_enough impl from the concrete PlainVectorIndexReadViewEnum alias to the generic PlainVectorIndexReadView<'a, I, V, Q, P>, mirroring the HNSW read view, so PlainVectorIndex and ReadOnlyPlainVectorIndex share one search impl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
da1a18230b |
feat: split DenseVectorStorage, generic scorers, read-only HNSW search
Split DenseVectorStorage into a read-only supertrait DenseVectorStorageRead (vector_dim / get_dense / scoring helpers) and the write-only update_from. Read-only storages can now implement the read trait, which they could not before because of update_from. Make the dense query scorers (Metric/CustomQueryScorer, raw_scorer_impl, new_scorer_with_metric) and the FilteredScorer / BatchFilteredSearcher / postprocess_search_result / is_quantized_search / get_oversampled_top constructors generic over VectorStorageRead + RawScorerBuilder and QuantizedVectorsReadAccess instead of the concrete VectorStorageEnum / QuantizedVectors. Add RawScorerBuilder, implemented for VectorStorageEnum and VectorStorageReadEnum<S>; the read-only enum scores all dense variants (incl. chunked) via raw_scorer_impl. Add size_of_available_vectors_in_bytes to VectorStorageReadEnum. Move the HNSW read view search impl from the concrete HNSWIndexReadViewEnum to the generic HNSWIndexReadView, so HNSWIndex and ReadOnlyHNSWIndex share one search implementation; wire VectorIndexRead for ReadOnlyHNSWIndex. Remaining: read-only multi / sparse storages still return a service_error from build_raw_scorer (they need the analogous MultiVectorStorage / SparseVectorStorage split); read-only multi size_of is approximate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
ec6d6d1faa |
wip: decouple read view type params (mirror plain index)
Restructure HNSWIndexReadView along the lines of the refactored
PlainVectorIndexReadView: bound I on IdTrackerRead (not IdTracker) and
take the payload index view as its own decoupled `P: PayloadIndexRead`
param instead of reusing I/V. Add a ReadView<'a, S> alias and switch
ReadOnlyHNSWIndex::with_view to it.
This fixes the two view type-param blockers (6 -> 2 compile errors);
with_view now type-checks for the read-only backends. Remaining (still
does not compile):
- search is implemented only on the concrete in-RAM HNSWIndexReadViewEnum;
reusing it for read-only storages needs the scorer constructors
generalized over VectorStorageRead (no raw-scorer path over
VectorStorageReadEnum<S> yet)
- size_of_searchable_vectors_in_bytes has no VectorStorageRead source
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
generallandClaude Opus 4.8
|
fe0aa15cb8 |
wip: read-only HNSW index scaffold (does not compile)
Extract HNSWIndex read logic into a reusable HNSWIndexReadView
(read_view/{mod,search,dispatch}.rs); HNSWIndex::search now delegates
via with_view.
Add a ReadOnlyHNSWIndex<S> scaffold with with_view + VectorIndexRead
in read_only/{mod,read.rs} that naively reuses the shared view search.
This intentionally does NOT compile yet; it documents the shape and the
remaining blockers:
- view bounded on IdTracker, but the read-only tracker only impls
IdTrackerRead
- view couples payload-view type params to the standalone storage
params (payload index is always in-RAM)
- scorer constructors are hardcoded to VectorStorageEnum/QuantizedVectors;
no raw-scorer path over VectorStorageReadEnum<S>
- size_of_searchable_vectors_in_bytes has no VectorStorageRead source
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
generallandClaude Opus 4.8
|
f45578cb92 |
feat: with_view for ReadOnlyPlainVectorIndex
Decouple the payload index view from the top-level backends in PlainVectorIndexReadView (generic P: PayloadIndexRead, relax I to IdTrackerRead) so a read-only id tracker / vector storage / quantized vectors can be combined with a payload index view built over the still-mutable StructPayloadIndex enums. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
b0f3de5bb8 |
refactor: QuantizedVectorsReadAccess trait, generic quantized backend in plain read view
Introduce `QuantizedVectorsReadAccess` (in its own `read_access.rs`), a shared read/scoring interface implemented by both the read-write `QuantizedVectors` and the read-only `QuantizedVectorsRead<S>`. The duplicated `raw_scorer` body is extracted into a shared `build_quantized_raw_scorer` helper. `PlainVectorIndexReadView` is now generic over `Q: QuantizedVectorsReadAccess`, holding the quantized vectors as `Option<&'a Q>` (symmetric with the id-tracker and vector-storage references) instead of cloning the `Arc<AtomicRefCell<...>>`. The concrete `PlainVectorIndexReadViewEnum` alias pins `Q = QuantizedVectors`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
ded653a7be |
fix: add missing std imports in TurboVectorStorage scaffold
`update_from`'s signature references `AtomicBool` and `Range` without importing them, leaving the segment crate uncompilable. Add the missing `use`s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
generall
|
31c77e7df8 | pub | ||
|
generallandClaude Opus 4.8
|
56d5430b1b |
refactor: inline trivial VectorIndexRead methods, slim down the view
Only `search` and `is_small_enough_for_unindexed_search` genuinely need the borrowed view, so the constant/single-field `VectorIndexRead` methods (`indexed_vector_count`, `is_index`, `fill_idf_statistics`, `size_of_searchable_vectors_in_bytes`, `get_telemetry_data`) are now implemented directly on `PlainVectorIndex` instead of routing through `with_view`. The view consequently no longer implements the full `VectorIndexRead` trait -- it exposes just the two inherent methods that are used. Renamed `read_view/vector_index_read.rs` to `read_view/search.rs` to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
e38181c72a |
refactor: introduce PlainVectorIndexReadView and move read logic into it
Add a borrowed read-only view `PlainVectorIndexReadView` and a `PlainVectorIndex::with_view` accessor that collapses the per-method `AtomicRefCell::borrow()` calls into a single up-front borrow, mirroring `StructPayloadIndex::with_view`. The actual `VectorIndexRead` implementation (search, telemetry, etc.) and `is_small_enough_for_unindexed_search` now live on the view in `read_view/vector_index_read.rs`. `PlainVectorIndex` keeps only thin delegations through `with_view` in `read.rs`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.8
|
b15cb408a0 |
refactor: split plain_vector_index.rs into a module
Split `plain_vector_index.rs` into a `plain_vector_index/` module: - `mod.rs`: the `PlainVectorIndex` struct and its `new` constructor - `read.rs`: `is_small_enough_for_unindexed_search` and the `VectorIndexRead` impl (search, telemetry, etc.) - `lifecycle.rs`: the `VectorIndex` impl (`files`, `update_vector`) Code was moved verbatim; only the per-file `use` lists were adjusted. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
04e38c6621 |
feat(id_tracker): deferred-aware mutable id tracker (#9249)
* feat(debug): QDRANT_APPEND_ONLY_MUTATIONS env override Debug-only escape hatch so newly built segments default to append-only mutation routing when QDRANT_APPEND_ONLY_MUTATIONS=1 (or true/yes) is set in the environment. Lets us run the existing test suites against the append-only path without wiring a collection-level config knob first. Release builds compile this out — the function is a const false. Logs a single warn-level message the first time the override fires. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(append-only): skip deleted vectors on snapshot, always write payload Two correctness fixes to clone_and_mutate_point uncovered by running the openapi suite with QDRANT_APPEND_ONLY_MUTATIONS=1: 1. The snapshot loop read every named vector via get_vector_opt, which returns slot bytes even when the per-vector deletion bit is set. For sparse-only points (or any point that had update_vector(_, None) applied) this materialised the default-zero vector as if it were real data, wrote it at new_id, and never re-tombstoned the slot — so dense search started scoring phantom vectors. Now we check is_deleted_vector(old_id) and skip the read, letting the writer loop emit update_vector(new_id, None) and re-mark the slot deleted. 2. The payload write was skipped when the snapshot ended up empty. That dropped two side effects the field indexes rely on: payload_storage.overwrite(new_id, empty), and the remove_point fan-out across configured field indexes that bumps each index's total_point_count to cover new_id. Without the bump the null index doesn't see new_id, so is_empty / is_null filters lose the point even though its mapping is live in the id tracker. The skip was an optimisation, not a contract; remove it so the field indexes get the same registration they'd get from the standard clear_payload path. Also collapses the debug env override to an inline cfg!()-gated check at the struct literal — the helper with one-time logging and multi-value matching was disproportionate for a debug-only escape hatch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(id_tracker): split active vs deferred maps in PointMappings (PR A) First step of moving the mutable id tracker to a "two-track" model so append-only mutations into a deferred segment can keep both the visible (active) version and the latest mutated (deferred) version of a point at the same time. This PR is purely structural. On-disk format is unchanged: the loader still produces a single combined map, and `PointMappings::new` partitions it at construction time based on `deferred_internal_id`. Every observable behaviour at the existing API surface is preserved. Concretely: - New fields: `external_to_internal_num_deferred`, `external_to_internal_uuid_deferred`, and a `shadowed: BitVec` for future use by PR B (lazy-grown, default-false). - `internal_id(ext)` checks active first, falls through to deferred — matches the pre-split "any matching id" contract for ext ids whose internal id sat above the cutoff. - `set_link(ext, new_id)` now routes by cutoff: writes below the cutoff land in active, writes at or above land in deferred. Any prior head in the other track is tombstoned, so each ext still owns exactly one slot — same observable result as the pre-split single-map insert. PR B replaces the cross-track tombstone with a shadow-bit flip; PR A keeps current semantics on purpose. - `drop(ext)` clears entries from both tracks and tombstones each one, again matching the prior single-map behaviour. - `iter_external` and `iter_from` merge the active and deferred BTreeMap views into one sorted-by-key stream (dedup'd in case an ext exists in both tracks). - `available_point_count` counts distinct external ids across both tracks — preserves the prior observable count for segments where some entries used to sit above the cutoff in the single map. No write-path or read-path behaviour change. Reads still filter `internal_id >= cutoff` exactly as before; mutations still tombstone prior heads. The shadow bit and the deferred-aware lookup wiring land in PR B and PR C. All existing id_tracker tests pass against the split layout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(id_tracker): shadow active head on deferred writes (PR B) Step 2 of the deferred-aware mutable id tracker. Replaces PR A's "tombstone the other track" cross-track cleanup with shadow-bit logic so a deferred mutation no longer hides the visible active version. Behaviour by case: - Active write (`internal_id < cutoff`, or no cutoff): unchanged semantically. Any prior deferred head for the same ext is dropped and tombstoned (the new active is the single visible head). Same observable result as PR A. - Deferred write (`internal_id >= cutoff`): the prior deferred head (if any) is dropped and tombstoned. The active head, if it exists, is **shadowed** — its bit is set in `shadowed: BitVec` but its slot stays alive in the active map. Read paths in `Exclude` mode continue to return that active version; PR C will teach `IncludeAll` paths (the optimiser) to skip shadowed actives and prefer the deferred head. Also adds `is_shadowed(internal_id)` and `shadowed_bitslice()` accessors (the latter for PR C's filter pipeline). New unit tests cover the routing matrix: - no cutoff — active replacement, no shadow, - below-cutoff replacement — active path, no shadow, - deferred-on-top-of-active — shadow set, active retained, - two deferred writes — prior deferred tombstoned, shadow persists, - fresh insert above cutoff — no shadow, - `drop(ext)` clears both tracks plus the shadow bit. WAL replay reuses the same `set_link`, so deferred routing on replay falls out of this change with no extra wiring. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(id_tracker): IncludeAll skips shadowed actives, deferred-aware lookups (PR C) Step 3 of the deferred-aware id tracker stack. PR B introduced the shadow bit on `set_link`; this PR teaches the read paths to honour it so the optimiser (and any other `DeferredBehavior::IncludeAll` consumer) sees each external id exactly once, with the deferred head winning over the now-shadowed active. Concretely: - `PointMappings::internal_id_with_behavior(ext, behavior)`: * `Exclude` returns the active head only — `None` for a deferred-only ext, so query paths never see a deferred mutation; * `IncludeAll` prefers the deferred head and falls back to active for points that never crossed the cutoff. Yields at most one internal id per ext. - `IdTrackerRead::internal_id_with_behavior` mirrors the new method with a default impl that delegates to `internal_id` for trackers that don't carry deferred mutations. - `PointMappingsRefEnum::iter_internal_with_behavior(IncludeAll)` now filters shadowed actives via the new `PointMappings::shadowed_bitslice()` accessor. - `PointMappingsRefEnum::filter_deferred_and_deleted(IncludeAll)` also filters shadowed actives — same single-yield-per-external guarantee for external iterator sources like field-index outputs. - `IdTrackerRead::resolve_external_ids` switches to the deferred-aware lookup. No more post-lookup `id >= cutoff` filter — the behaviour enum lookup gets it right at the source. New unit tests cover the two new entry points: - IncludeAll prefers the deferred head when an active is shadowed; - Exclude returns None for deferred-only ext ids; - `filter_deferred_and_deleted` over a mixed candidate list yields the expected per-mode result (Exclude: actives below cutoff; IncludeAll: every visible head, no shadowed actives). No queries observable behaviour change today — production Exclude paths still resolve via `internal_id` and the active head. The optimiser will start using `IncludeAll` (and reap the dedup) in follow-up work. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(id_tracker): expose shadowed_point_count in SegmentInfo (PR D) Final step of the deferred-aware id tracker stack. Adds a single new counter — number of active heads currently shadowed by a deferred mutation — and plumbs it through the id-tracker trait, the segment read-view helper, and `SegmentInfo` for telemetry. Preserves existing semantics: - `available_point_count` is unchanged. Each distinct external id still counts once, regardless of which track holds its head. - `deferred_point_count`, `deferred_internal_id`, `num_deleted_deferred_points` keep their current values. - Non-appendable trackers default `shadowed_point_count()` to `0`, so the new `SegmentInfo.num_shadowed_points` is `None` for them. Concrete changes: - `PointMappings::shadowed_count()` — popcount of the shadowed bitslice. - `IdTrackerRead::shadowed_point_count()` trait method with a `0` default; wired through `MutableIdTracker`, `InMemoryIdTracker`, the mutable read-only tracker, and both enum dispatchers. - `SegmentReadView::shadowed_point_count()` helper. - New `SegmentInfo.num_shadowed_points: Option<usize>`, populated with `Some(_)` for appendable segments and proxied through `ProxySegment` from the wrapped segment's value. New unit test covers the counter lifecycle: - active-only writes don't grow it, - a deferred write over an active adds one shadow, - a second deferred write supersedes the prior deferred head but the shadow stays put (still one active being shadowed), - `drop(ext)` clears the shadow bit, - a fresh deferred insert with no active prior doesn't add a shadow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(id_tracker): rename DeferredBehavior, push behavior down to PointMappings Three connected pieces of polish on top of the deferred-aware id tracker stack: 1. Rename `DeferredBehavior::Exclude` → `VisibleOnly` and `IncludeAll` → `WithDeferred`. The pre-PR-C semantics of "include/exclude the deferred cutoff" became misleading once `IncludeAll` started skipping shadowed actives — it doesn't "include all" anymore, it yields one slot per external (deferred head preferred, active fallback). New names describe what each variant returns instead of how it relates to the cutoff. The helper method `include_all_points` becomes `with_deferred_points`. Updates ~90 call sites across the workspace; behaviour is unchanged. 2. Push `iter_internal_with_behavior` down from `PointMappingsRefEnum` into `PointMappings`. The per-mode logic (cutoff `take_while`, shadowed `filter`) now lives next to the data it consults; the enum layer becomes a two-arm `Either` dispatcher. `CompressedPointMappings` short-circuits to `iter_internal()` since compressed mappings can't carry deferred mutations. 3. Add a short docstring on `internal_to_external` describing the two-track model: no active-vs-deferred bias, shadowed pairs occupy two slots with the same value, and reads must gate on `deleted` because `set_link`'s same-track replacement leaves a real-looking stale ext id in place. Returning `impl Iterator` instead of `Box<dyn Iterator>` for `iter_internal`, `iter_internal_excluding`, `iter_internal_visible`, and `iter_internal_with_behavior` removes the double-Box at the enum boundary. The original branching structure is preserved with `itertools::Either` instead of restructured into one big filter chain. All existing `id_tracker` tests still pass (47 total). `cargo check --all-targets` + `cargo clippy --all-targets` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(id_tracker): drop shadowed API surface, unbox iter_from/iter_random Cleanup pass on top of the deferred-aware id tracker stack. API surface: - Drop SegmentInfo.num_shadowed_points + its trait method, read-view helper, tracker impls and ProxySegment passthrough. Cross-segment shadow already exists in the appendable update flow (the source segment keeps its copy when the new write lands above the cutoff, see SegmentHolder::apply_points_with_conditional_move) and is handled at the aggregation layer, not reported per-tracker. - Keep PointMappings::shadowed_count() as a private popcount helper guarded by expect(dead_code) so a future caller can opt back into the dedup'd count without re-plumbing the trait. available_point_count: - Replace the cross-track dedup (active + deferred-only via contains_key filter) with a straight 4-term sum. A shadowed ext contributes two slots, matching the two non-tombstoned internal ids it actually occupies. Old dedup broke the invariant that deleted_point_count == deleted_bitslice.count_ones(): for each shadow it overcounted deletions by one without any tombstone actually being set. DeferredBehavior pushdown: - iter_random_with_behavior: caps the sampling range at the deferred threshold in VisibleOnly mode (no wasted samples above cutoff), filters shadowed actives via the bit in WithDeferred. - iter_from_with_behavior: VisibleOnly walks the active maps only (no merge with deferred); WithDeferred delegates to iter_from's existing merge. - scroll.rs read_by_id_stream / filtered_read_by_id_stream collapse their manual if-deferred-behavior branching into a single iter_from_with_behavior call. - Old iter_random (no behavior) at PointMappings + ref enum was unused after the migration, deleted. Unboxing iter_from / iter_random / iter_from_with_behavior: - PointMappings::iter_from returns impl Iterator + '_ via Either inside the merged_num/merged_uuid closures (BTreeMap::iter vs range), Either at the outer match (num+uuid chain vs uuid-only). - PointMappings::iter_from_with_behavior unboxed with a triple Either (behavior, external-id arm, closure start). - CompressedPointMappings::iter_from unboxed (Either over None/Some). - PointMappingsRefEnum::{iter_from, iter_from_with_behavior, iter_from_visible} all return impl Iterator + 'a via Either on the Plain/Compressed dispatch. Other: - Update outdated PR-A/PR-B comment on PointMappings::drop. - Make the max_internal match in iter_random_with_behavior exhaustive (VisibleOnly+None | WithDeferred+_ instead of `_`). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(id_tracker): unbox iter_external Return impl Iterator from PointMappings::iter_external, CompressedPointMappings::iter_external and the PointMappingsRefEnum wrapper, matching the style of the other iter_* helpers. The wrapper dispatches via Either. The remaining Box::new at Segment::iter_points stays because self_cell's BoxedPointIdIterator alias needs a sized type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(id_tracker): count set_link tombstones in deferred_deleted_count PointMappings::set_link tombstones the prior slot at two sites — when an active write supersedes a deferred head (cross-track) and when any write replaces a same-track head — using a bare deleted.set(old, true). Neither path updated deferred_deleted_count, so tombstones above the cutoff weren't reflected in the counter. The append-only flow exposes this constantly: every set_full_payload after upsert_point routes through clone_and_mutate_point, which re-issues set_link with a fresh internal id and tombstones the prior one. Most tombstones land above the cutoff, so deferred_point_count (total - cutoff - deferred_deleted_count) over-reports by the missing count. The openapi test_deferred_points integration test caught this as `num_points - num_deferred_points = -1800` across two segments. Extracted the tombstone bookkeeping into PointMappings::tombstone_slot and routed both set_link sites + drop's loop through it. Behaviour on drop is unchanged; set_link now bumps the counter on the live → tombstoned transition for slots at or above the cutoff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(id_tracker): collapse set_link tombstone if-let chains Clippy's collapsible_if on the two if-let blocks added by the deferred_deleted_count fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update lib/segment/src/id_tracker/point_mappings.rs Co-authored-by: Tim Visée <tim+github@visee.me> * refactor(id_tracker): size shadowed BitVec once instead of growing lazily Collect the shadowed active ids up front and allocate the BitVec to the highest offset in a single resize, avoiding repeated reallocations while marking shadows. Addresses review feedback. Co-authored-by: Cursor <cursoragent@cursor.com> * revert: restore lazily-grown shadowed BitVec Roll back the up-front sizing of the shadowed BitVec; last_entry doesn't fit here and swapping one allocation for another isn't worthwhile. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(id_tracker): prefer deferred head in iter_from merge When an external id has both an active and a deferred head, iter_from's merge collapsed the pair to the active (stale) offset. That contradicts the WithDeferred contract used everywhere else: internal_id_with_behavior and iter_random_with_behavior both surface the deferred head (the latest mutation) over the shadowed active. Consumers that use the returned internal id (payload-filter checks, the optimizer's version merge, HNSW old->new mapping) therefore saw the stale copy. Flip the Both arm to take the deferred operand and document the rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(id_tracker): preserve active+deferred heads when loading mappings read_mappings replayed the persisted change log into a single flat external -> internal map per id-type, then split it by the deferred cutoff in PointMappings::new. A flat map holds one head per external id, so it collapsed the active+deferred coexistence case (an external id linked first to an active slot, then to a deferred one via sequential set_link) down to the last write — silently dropping the other head, orphaning its slot, and leaving the shadowed bit unset. The split in new() could not recover what was already lost before it. Replay the log through the canonical set_link/drop mutators on a PointMappings seeded with the cutoff instead. The log is the sequence of set_link/drop calls that produced the live in-memory state, so this reconstructs that state exactly — both heads, the shadowed bit, and deferred_deleted_count — with no logic duplication. Drops the debug_assert-guarded corruption-recovery branch (subsumed by set_link's re-link handling) and the now-unused Uuid/PointIdType imports. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(id_tracker): make deferred behavior explicit at point resolution Replace the ambiguous "any matching head" point resolution with an explicit DeferredBehavior at every resolution boundary, so callers no longer rely on a hidden active-vs-deferred policy. - Remove PointMappings::internal_id and the bare IdTrackerRead::internal_id (the active-first-else-deferred hybrid). internal_id_with_behavior is now the single required resolution method; immutable/compressed trackers implement it by ignoring the behavior (they never carry deferred heads). - Migrate every caller to an explicit behavior, audit-driven: - writes (upsert/delete/payload/vectors), point_version, point_is_deferred, get_internal_id, drop, consistency + builder dedup -> WithDeferred (the latest/live head); - single-point payload/vector retrieval and formula rescore -> VisibleOnly; - HasId/CustomIdChecker/cardinality resolution -> the request's behavior, threaded through the filter chain from iter_filtered_points (other entry points default to VisibleOnly). - lookup_internal_id takes an explicit DeferredBehavior instead of assuming VisibleOnly internally. - has_point takes an explicit DeferredBehavior (drop the has_point_with_behavior wrapper). Thread it through read_points/_read_points/read_points_locked so retrieve_blocking passes its request behavior to the existence filter; all other existence/dedup callers pass WithDeferred (unchanged behavior). - set_link now detaches a stale live occupant of a reused internal id, keeping the forward and reverse maps consistent when recovering a corrupted log. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(id_tracker): failing tests for two correctness findings in #9249 (#9311) * test(id_tracker): failing tests for two review findings Two intentionally-failing tests pinning correctness gaps in the deferred-aware id tracker (#9249). Both fail on the final assertion; the earlier assertions establish the expected/consistent behavior. 1. iter_from_with_behavior(WithDeferred) resolves an active/deferred shadow collision to the stale ACTIVE internal id, while its siblings internal_id_with_behavior and iter_internal_with_behavior correctly surface the DEFERRED (latest) head. Consumers that use the yielded internal id (optimizer merge via for_each_unique_point, filtered_read_by_id_stream) therefore observe the pre-mutation version. left: [(NumId(7), 2)] right: [(NumId(7), 9)] 2. The PR-B shadow/visible invariant is not durable: the on-disk single combined map cannot represent a shadowed ext, so a plain mappings flush + reload collapses the shadow to deferred-only and the visible (active) head is lost (VisibleOnly resolves None where it resolved Some(2) live). Restoration then depends entirely on WAL replay, i.e. on flush-vs-WAL-truncate ordering. left: None right: Some(2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(id_tracker): strengthen shadow tests + merge-primitive proof Follow-up to the two failing tests, addressing self-review: - Add for_each_unique_point_keeps_deferred_head_for_shadowed_point: the optimizer merge primitive (used by segment_builder::update_from) yields the stale active copy (internal 2, version 5) for a shadowed point and drops the deferred latest (internal 9, version 8). This directly exercises the data-loss consequence of finding #1 at the merge layer. left: [(NumId(7), 2, 5)] right: [(NumId(7), 9, 8)] - Tighten shadow_visible_head_survives_mapping_flush_reload: pin the exact reload failure mode. After flush+reload the mapping collapses to deferred-only (internal_id == Some(9)) and the active slot survives as a live orphan in the inverse map (external_id(2) == Some(7), not deleted) — a torn state where a VisibleOnly scroll still surfaces the stale copy while by-id VisibleOnly resolution breaks. Reframe as the live-vs-reload divergence the PR introduces (Some(2) live -> None reload; dev is consistently None). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix test --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: generall <andrey@vasnetsov.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Tim Visée <tim+github@visee.me> Co-authored-by: root <111755117+qdrant-cloud-bot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Arnaud Gourlay <arnaud.gourlay@gmail.com> |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
c90fdb9a93 |
feat: read-only QuantizedVectors generic over UniversalRead (#9346)
* feat: add read-only QuantizedVectors generic over UniversalRead Introduce `QuantizedVectorsRead<S>` / `QuantizedVectorStorageRead<S>`, a read-only counterpart of `QuantizedVectors` organized like `VectorStorageReadEnum`: generic over the `UniversalRead` backend `S`, opened from existing on-disk data, with no create/upsert/builder path and no disk writes. Highlights: - Keep both in-RAM (`*Ram`) and read-only mmap (`*Mmap`) variants; drop the appendable `*ChunkedMmap` variants (the only mutable ones). - All bulk reads go through `S`: add `QuantizedRamStorage::from_universal_read` and `MultivectorOffsetsStorageRam::open`, and make `MultivectorOffsetsStorageMmap` generic over `S` (default `MmapFile`). - Share scorer construction between the read-write and read-only enums via a `QuantizedScorerDispatch` trait, so only the per-variant match is duplicated while the datatype/distance and per-query dispatch live once in the builder. Tested: read-only vs read-write scorer parity (scalar/binary/product, single and multivector, RAM and mmap), covering both `raw_scorer` and `raw_internal_scorer`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: route quantized RAM loading through UniversalRead Follow-up to the read-only quantized work, removing direct-filesystem reads and load-path duplication: - `EncodedVectors{U8,PQ,Bin,TQ}::load` now take a `UniversalRead` filesystem and read metadata via `read_json_via` instead of `fs::read_to_string` — every read flows through universal IO. Single-vector `load` returns `common::universal_io::Result`; `validate_storage_vector_size` stays `std::io::Result`. - Add `common::universal_io::OneshotFile<S>`: a thin RAII wrapper over any `UniversalRead` handle that evicts the data from cache via `clear_ram_cache` on drop (the universal-IO counterpart of `fs::OneshotFile`). - Collapse `QuantizedRamStorage::{from_file, from_universal_read}` into one `from_file<S: UniversalRead>`. It reads the whole file in a single access (no separate `len()` round-trip — cheaper on S3-like backends) and loads via the new `VolatileChunkedVectors::extend`, which inserts one chunk per `copy_from_slice` instead of one vector at a time. - RW callers pass the local `READ_FS` (mmap) backend; the read-only loader passes its `S`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: load appendable (chunked) quantization format read-only `storage_type` only selects the on-disk layout, so the read-only quantized storage must be able to load both — the immutable flat format and the appendable chunked format (produced only by Binary/TurboQuant). Previously the read-only loader rejected the Mutable layout outright. - Add `QuantizedChunkedStorageRead<S>` and `MultivectorOffsetsStorageChunkedRead<S>`, read-only wrappers over the existing `ChunkedVectorsRead<_, S>` primitive (mirrors the dense read-view's chunked read storage). Generic over the `UniversalRead` backend, on-disk, no write path. - Add `BinaryChunked`/`TQChunked` (+ multi) variants to `QuantizedVectorStorageRead` and wire them through every accessor and the scorer dispatch. - Route `storage_type == Mutable` to the chunked read variants in the loader and drop the blanket rejection. - Tests: parametrize the read-only/read-write parity tests over `storage_type` and add chunked (Mutable) cases for binary/turbo, single and multivector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: split quantized chunked & multivector storage into modules `quantized_chunked_mmap_storage.rs` and `quantized_multivector_storage.rs` had grown to hold several unrelated structures, making them hard to navigate. Convert each into a module (pure code movement, no behavior change): - quantized_chunked_mmap_storage/{read_write,read_only}.rs — the appendable mmap storage + builder vs. the read-only chunked storage. - quantized_multivector_storage/{mod,offsets}.rs — the core `QuantizedMultivectorStorage` + offset traits stay in mod.rs; the four `MultivectorOffsetsStorage*` backends move to offsets.rs. Public paths are unchanged via re-exports. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: single source of truth for quantized vector size The on-disk stride (bytes per quantized vector), including the binary `u8`(multi)/`u128`(single) word-type choice, was independently re-derived in every load/open site with no compile-time link — the kind of value-level duplication that silently diverges (it already caused the binary multi `u8`/`u128` bug). Extract it into one place: - `QuantizedVectors::quantized_vector_size(quantization_config, vector_parameters, is_multi)` and the `QuantizedVectorsConfig::quantized_vector_size(is_multi)` convenience. Route every reader through it: - read-only `open_single`/`open_multi` and the read-write `{scalar,pq,binary,turbo}` loaders now hoist `config.quantized_vector_size(is_multi)` once instead of recomputing the per-method formula per branch. The create (write) path keeps its own computation for now; it stays guarded by `validate_storage_vector_size` and the read-only/read-write parity tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: flat-match quantized loaders on a shared QuantizedStorageKind The loaders chose the concrete storage variant via a nested `method × (storage_type / is_ram)` match, re-derived independently in the read-only and read-write paths — hard to follow and easy to drift. - Add `QuantizedStorageKind` + `QuantizedVectorsConfig::storage_kind(on_disk)` (and `is_ram(on_disk)`): the single place the method × backend decision lives. - Both loaders now compute the kind once and use a flat 10-arm match: - read-only `open_single`/`open_multi`; - read-write `load_single`/`load_multi` (consolidating the four per-method `{scalar,pq,binary,turbo}/load.rs` files, which are removed). Both matches are exhaustive over the same enum, so the read and read-write variants can no longer fall out of sync without a compile error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: genericize write-capable chunked quantized storages over Fs Drop the hardcoded `MmapFile` specialization from `QuantizedChunkedStorage`, `QuantizedChunkedStorageBuilder`, and `MultivectorOffsetsStorageChunked`. They now expose the `Fs` backend (defaulting to `MmapFile`) and accept the fs handle as a parameter, matching the read-only variants. Introduce a single shared `ReadFile` type alias and `READ_FS` value handle in the `quantized_vectors` module root, used by the create, load, and storage enum paths so the local-file backend is named in one place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: move is_in_ram_or_mmap match into UniversalKind Deduplicate the identical kind-to-residency match in the read-only and write-capable chunked quantized storages by adding UniversalKind::is_in_ram_or_mmap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: adapt TurboVectorStorage to quantized rename + update_from move Integration fix after rebasing onto dev's TurboQuant work: - QuantizedChunkedMmapStorage -> QuantizedChunkedStorage<MmapFile> - update_from moved off the VectorStorage trait onto an inherent method, matching the per-kind sub-trait refactor; TurboVectorStorage implements neither DenseVectorStorage<T> nor the other kind traits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
ae5a0fd706 |
refactor: split quantized_vectors.rs into a module (#9345)
`quantized_vectors.rs` had grown to ~2.6k lines. Split it into a `quantized_vectors/` module: - `config.rs`: path constants, `QuantizedVectorsConfig`, `QuantizedVectorsStorageType` - `storage.rs`: `QuantizedVectorStorage` enum, type aliases and its `is_on_disk`/`heap_size_bytes`/`Debug` impls - `accessors.rs`: per-variant dispatch accessors (`files`, `populate`, `clear_cache`, `flusher`, `upsert_vector`, ...) - `create.rs` / `load.rs`: cross-type creation/loading dispatch - `scalar/`, `pq/`, `binary/`, `turbo/`: per quantization type, each with its own `create.rs` and `load.rs` The module root keeps the `QuantizedVectors` struct, scorer methods, path/conversion helpers and the `MemoryReporter` impl, and re-exports the public types so the external API path is unchanged. Code was moved verbatim; no behavior change. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
d61784c270 |
feat: lifecycle for ReadOnlyAppendableIdTracker (open + live reload) (#9301)
* feat: lifecycle for ReadOnlyAppendableIdTracker (open + live reload) Implement `open` and `live_reload` for `ReadOnlyAppendableIdTracker`, the read-only view over the appendable (mutable) ID tracker storage. - Generic over `S: UniversalRead`; all IO goes through the universal-io abstraction (mmap/io_uring/...), file handles refreshed via `reopen`. - `open` requires both mapping and versions files to exist (errors otherwise, never creates an empty tracker). - `live_reload` returns newly inserted/deleted point offsets. Inserts are driven by the versions file (version = commit marker, flushed last); deletes are driven by the mapping. Versions are loaded as an append-only delta and only fully-written entries are loaded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: only expose committed points in ReadOnlyAppendableIdTracker A point's version is flushed last (mappings -> data -> versions), so an insert is only fully written once its version exists. Defer linking an insert into the mapping until its offset is covered by the versions file: buffer pending inserts and apply them on the reload that observes their version, dropping any whose delete arrives first. A versionless point is therefore absent from the mapping entirely, not just withheld from the result. `open` now loads via the same reconciliation from an empty tracker, so the gating logic lives in one place. Handles upsert re-links to a new offset (old offset reported deleted) and partial/torn version tails. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Update lib/segment/src/id_tracker/mutable_id_tracker/read_only/live_reload.rs Co-authored-by: xzfc <5121426+xzfc@users.noreply.github.com> * review: simplify file load error handelling * fmt * fix: remove unused imports in read-only appendable id tracker Clears `cargo clippy --all-targets` warnings (unused `Path`, `OkNotFound`, `OperationError`, `PointIdType` imports). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: work around Windows mmap resize limit in torn-version test `test_live_reload_withholds_partially_written_version` completes a torn version flush, which truncates the stray bytes via `set_len`. On Windows a file with an open memory mapping cannot be resized (os error 1224), and the read-only tracker keeps the versions file mapped. Drop the read-only view (releasing its mmap) before the writer resizes the file, then reopen it to observe the completed version. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: work around Windows mmap resize limit in torn-mapping test `test_live_reload_ignores_partial_trailing_mapping_entry` completes a torn mapping append; the writer's flush truncates the stray bytes via `set_len` (the `Greater` branch in `store_mapping_changes`). On Windows a file with an open memory mapping cannot be resized, and the read-only tracker keeps the mappings file mapped. The partial-entry handling (read position not advanced) is already asserted on the live tracker; drop the read-only view before the writer truncates, then reopen to confirm the appended point is visible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: keep live_reload coverage in torn-entry tests; sort inserted offsets The Windows mmap-resize workaround replaced the completion step of both torn-entry tests with a fresh `open()`, which dropped the point of the tests: that a subsequent `live_reload` resumes from the preserved position and picks up the completed entry. Restore the `live_reload` resume path and gate only the truncation-completion (a `set_len` Windows forbids while the file is mapped) behind `cfg(not(windows))`. The partial-ignore assertions still run on all platforms. Also sort `inserted` in `live_reload`: `extract_if` drains in arbitrary hash order, but `LiveReloadResult` documents both lists as sorted ascending (test_live_reload_reports_inserts_and_deletes was flaky without it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: xzfc <5121426+xzfc@users.noreply.github.com> |
||
|
Andrey Vasnetsov
|
cc8d1696a9 |
feat/readonly map index with live reload (#9264)
* live reload function for map index * fmt |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
634d6c5bee |
Log slow operations during local shard WAL recovery (#9282)
* feat: log slow operations during local shard WAL recovery Warn when applying a single WAL operation during recovery of a local shard takes longer than 30s, including the operation type (e.g. PointOperation::UpsertPoints) so slow recoveries can be diagnosed. Adds CollectionUpdateOperations::label() returning a human-readable label including the inner variant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: reuse audit operation names for slow WAL recovery logging Move the canonical operation-name mapping next to the CollectionUpdateOperations definition in the shard crate as an inherent operation_name() method, and have the audit AuditableOperation impl delegate to it. The slow WAL recovery warning now reuses these same names (e.g. upsert_points) instead of a duplicated label mapping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
7900faf789 |
Add timeout to streaming shard snapshot writer (#9239)
* Add block level timeout to snapshot stream writer * Add tooling to exercise streaming snapshot stalls - tests/manual/slow_snapshot_download.py: slowly / partially download a streaming shard snapshot from a URL to exercise sender-side backpressure. Supports hold / rst / fin / blackhole termination to simulate a stalled, killed, or offline (network-partitioned) consumer against a remote node. Stdlib only; read-only against the target. - tests/consensus_tests/test_streaming_snapshot_receiver_kill.py: throttled receiver killed mid-flight + a second receiver, to observe whether the sender releases the SegmentHolder lock and recovers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: timvisee <tim@visee.me> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
4efb78545d |
feat: append-only mutation mode for mutable segments (#9245)
* feat: append-only mutation mode for mutable segments Adds an opt-in per-segment switch so every mutating op tombstones the old internal_id and writes the result at a fresh one, instead of overwriting the underlying vector storage / payload row / field index in place. Intended for S3-backed storages that prefer pure appends — the on-disk structures are never rewritten. Runtime flag, not persisted: `Segment::append_only_mutations` lives next to `appendable_flag` on the struct, defaults to false in the builder, and `Segment::is_append_only()` only returns true when the segment is also appendable (clone-and-tombstone needs growable storages). The core helper is `Segment::clone_and_mutate_point`: snapshots the point at `old_id` into owned `NamedVectors` + `Payload`, hands them to a closure for op-specific in-memory modification, allocates `new_id`, writes all configured vector storages and the payload at `new_id`, and repoints the id tracker via `set_link` (which auto-tombstones `old_id` in the deleted bitslice). Stale slots and field-index postings keyed by `old_id` are filtered by readers via `filter_deferred_and_deleted` and reclaimed at optimization. The routing policy concentrates in one new dispatcher, `Segment::handle_point_mutate`, which takes two closures (in-place and snapshot-mutate) and picks the path based on `is_append_only()`. All six existing entry points in `SegmentEntry` (`upsert_point`'s existing-pid branch, `update_vectors`, `delete_vector`, `set_full_payload`, `set_payload`, `delete_payload`, `clear_payload`) collapse to this dispatcher; `upsert_point`'s insert path stays on the non-mutating fast path. `delete_point` gets a tombstone-only variant (`delete_point_tombstone_only`) that only flips the id-tracker deleted bit, leaving the payload row and field-index postings at `internal_id` in place. The append-only path skips the in-place `clear_payload` call entirely, matching the "id tracker is the only thing we write" intent. `NamedVectors::into_owned()` is added so the snapshot closure can take ownership of a borrowed input wholesale. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(debug): QDRANT_APPEND_ONLY_MUTATIONS env override Debug-only escape hatch so newly built segments default to append-only mutation routing when QDRANT_APPEND_ONLY_MUTATIONS=1 (or true/yes) is set in the environment. Lets us run the existing test suites against the append-only path without wiring a collection-level config knob first. Release builds compile this out — the function is a const false. Logs a single warn-level message the first time the override fires. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(append-only): skip deleted vectors on snapshot, always write payload Two correctness fixes to clone_and_mutate_point uncovered by running the openapi suite with QDRANT_APPEND_ONLY_MUTATIONS=1: 1. The snapshot loop read every named vector via get_vector_opt, which returns slot bytes even when the per-vector deletion bit is set. For sparse-only points (or any point that had update_vector(_, None) applied) this materialised the default-zero vector as if it were real data, wrote it at new_id, and never re-tombstoned the slot — so dense search started scoring phantom vectors. Now we check is_deleted_vector(old_id) and skip the read, letting the writer loop emit update_vector(new_id, None) and re-mark the slot deleted. 2. The payload write was skipped when the snapshot ended up empty. That dropped two side effects the field indexes rely on: payload_storage.overwrite(new_id, empty), and the remove_point fan-out across configured field indexes that bumps each index's total_point_count to cover new_id. Without the bump the null index doesn't see new_id, so is_empty / is_null filters lose the point even though its mapping is live in the id tracker. The skip was an optimisation, not a contract; remove it so the field indexes get the same registration they'd get from the standard clear_payload path. Also collapses the debug env override to an inline cfg!()-gated check at the struct literal — the helper with one-time logging and multi-value matching was disproportionate for a debug-only escape hatch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * make test_move_points_to_copy_on_write work with copy-on-write --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
bfa9c182d9 |
test(consensus): batch initial upsert in snapshot-transfer missing-point test (#9261)
The initial 20k-point insert was sent as a single HTTP request (no batch_size), saturating all cores long enough to starve the consensus thread (cascading leader elections) and to exceed the 2000ms per-shard update healthcheck deadline, returning a flaky 408 before the test's actual snapshot-transfer logic even started. Batch the insert into 1k-point requests, matching the convention used by every other large initial upsert in the suite. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
790ce443a9 |
Batch initial load in resharding abort-crash test to fix flakiness (#9240)
The test fired a single 20k-point `wait=true` upsert into a fresh 3-shard x 2-replica collection. Under CI load that one op keeps a replica busy past the hardcoded 2s inter-node health-check (transport_channel_pool HEALTH_CHECK_TIMEOUT), so the coordinator fails the forward with a transient "Healthcheck timeout 2000ms exceeded" 408 before resharding even starts. It's the only resharding test doing a 20k single-shot upsert (others do ~1k), which is why it flakes and they don't. Batch the load at 1000 points so each op stays well under the health-check window, matching the proven-stable pattern. Data and transfer behavior are unchanged. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
739a984da5 |
Fix stale snapshot transfer recovery comment on failure/cancellation (#9237)
* Fix stale snapshot transfer recovery comment on failure/cancellation The transfer status "comment" prefers the destination-side recovery progress over the sender task status. Recovery progress was tracked in `active_recoveries` and only removed via `finish_shard_recovery`, which was called on the success path only. If `recover_shard_snapshot` returned early - clearing the local shard, downloading the snapshot, checksum mismatch, or cancellation (the future is spawned with `spawn_cancel_on_drop`) - the entry leaked. A leaked entry keeps reporting its last stage with an ever-growing elapsed time (computed live from a fixed `Instant`), so subsequent transfers for the same shard show stale timings until the next recovery overwrites the entry or the node restarts. Replace the manual start/finish pair with an RAII `ShardRecoveryGuard` that removes the progress entry on drop, covering every exit path including early returns and cancellation. The guard only removes its own entry (checked via `Arc::ptr_eq`) so it never clobbers a newer recovery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Extract recovery tracking into dedicated recovery_guard module Move ShardRecoveryGuard out of the large shard_holder/mod.rs into a dedicated recovery_guard.rs, and replace the verbose `Arc<Mutex<HashMap<ShardId, Arc<Mutex<RecoveryProgress>>>>>` type with a dedicated `ActiveRecoveries` struct that encapsulates start/comment/ set_stage/remove operations. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Bind recovery stage updates to the guard instance, not shard id `ActiveRecoveries::remove` is pointer-guarded so an unwinding recovery cannot delete a newer recovery's entry, but stage updates still resolved the progress entry by shard id. If recovery A is still unwinding after recovery B started for the same shard, A's late `set_stage` would mutate B's progress and resurrect incorrect transfer comments. Thread the guard's own progress handle (`RecoveryProgressHandle`) through `recover_shard_snapshot_impl` -> `Collection::restore_shard_snapshot` -> `ShardHolder::restore_shard_snapshot`, so the Unpacking/Restoring stages (and Downloading, via `ShardRecoveryGuard::set_stage`) update that exact recovery's progress. Direct API recoveries, which are not tracked as transfer-side recoveries, pass `None`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Allow too_many_arguments on recover_shard_snapshot_impl Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: root <111755117+qdrant-cloud-bot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
102c0dd3ed |
fix(tests): wait for abort target to apply transfer-start before aborting (#9238)
test_resharding_down_abort_converges_when_killed_mid_abort fired the fire-and-forget abort_transfer request at alive_uri after waiting only for the *victim* to apply the transfer-start. When alive_uri lagged in replicating that consensus entry, the abort handler's local check_transfer_exists returned 404 (swallowed by the fire-and-forget thread); the transfer then completed naturally, resharding was never aborted, and _victim_in_window never opened, timing out after 30s. Wait until both alive_uri (the abort target that gates the 404) and the victim have applied the transfer-start before firing the abort. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.8
|
2c257314a2 |
wal: fix WAL lock on Android by using fs4 flock instead of std try_lock (#9226)
`dir.try_lock()` resolves to the inherent `fs_err`/`std` `File::try_lock`,
which Rust's stdlib gates to a fixed target list that excludes
`target_os = "android"` (1.89+). On Android it returns
`ErrorKind::Unsupported` ("try_lock() not supported"), so opening a WAL —
and thus creating/loading a shard via qdrant-edge — fails.
Dispatch explicitly to `fs4::FileExt::try_lock` on the underlying
`std::fs::File`, which issues a direct `flock(LOCK_EX | LOCK_NB)` syscall
that Android supports. This restores the pre-#8770 behavior. UFCS is needed
because fs4's trait method collides by name with the inherent one (which
otherwise wins method resolution).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
|
4b0d4ab76c |
[UIO] Split UniversalReadFileOps into filesystem + file traits (#9151)
* [UIO] Split UniversalReadFileOps into filesystem + file traits
`UniversalReadFileOps` is now an instance-based trait describing a
filesystem handle (list/exists with `&self`, plus `from_context`). A new
`UniversalReadFs: UniversalReadFileOps` subtrait adds the
`open(&self, path, options) -> Self::File` capability with `type File:
UniversalRead`. `UniversalRead` no longer extends `UniversalReadFileOps`
and is purely a file-handle trait.
This separates "filesystem instance" from "file handle". Backends that
need per-instance configuration (S3 bucket name + credentials, mmap
default advice, io_uring runtime, block-cache controller `Arc`) gain a
typed home in `Self::ContextConfig`, and `list_files`/`exists`/`open`
become `&self` methods on the filesystem handle.
Concrete filesystem handles introduced for the three existing backends:
- `MmapFs` — unit struct; `ContextConfig = ()`; produces `MmapFile`.
- `IoUringFs` — carries `prevent_caching`; `ContextConfig =
IoUringConfigContext`; produces `IoUringFile`. `IoUringConfigContext`
becomes the construction input rather than a per-open argument.
- `BlockCacheFs` — carries `Arc<CacheController>`; `ContextConfig =
BlockCacheConfigContext`; produces `CachedSlice`.
`TConfigContext` (universal builder methods) is kept so generic-over-`Fs`
code can still set cross-backend knobs via
`Fs::ContextConfig::default().with_prevent_caching(true)`.
Wrappers (`ReadOnly<S>`, `TypedStorage<S, T>`, `StoredStruct<S, T>`),
higher-level storages (`StoredBitSlice<S>`, `UniversalHashMap<K, V, S>`)
keep `<S: UniversalRead>` parameterization but their `open(...)`
constructors now grow a `fs: &Fs` argument bound by
`Fs: UniversalReadFs<File = S>`. `read_json_via` becomes
`read_json_via(fs: &Fs, path)`.
All test code and benches in `common` updated to construct
`MmapFs`/`IoUringFs` inline as needed. `common` compiles cleanly with
tests and benches. `gridstore`, `segment`, and `tonic` caller updates
are in flight in subsequent commits.
* WIP: gridstore + segment caller sweep (partial)
Threads `fs: &Fs` through gridstore's `BitmaskGaps`, `Bitmask`, `Pages`,
and `Gridstore::new`/`open`/`create_new_page`. Most segment callers
have `OpenOptions { extra: ... }` removed and `S::open(path, opts, ctx)`
sites updated mechanically but the trait change is not yet propagated.
Does NOT compile yet. Tracker still has static `S::open` calls, segment
generic constructors (`MmapInvertedIndex<S>`, `UniversalMapIndex`,
`StoredGeoMapIndex`, etc.) still call `S::open`/`S::list_files`/`S::exists`
statically — they need an `fs: &Fs` parameter added. Tonic API
`StorageReadService<S>` also unconverted.
Committed as branch checkpoint; cascade continues in subsequent work.
* gridstore: thread `fs: &Fs` through Bitmask, BitmaskGaps, Pages, Tracker
Per the new `UniversalReadFs` shape, every constructor/method that opens
files takes an `fs: &Fs` parameter. Gridstore is currently mmap-only,
so the top-level `Gridstore` / `GridstoreReader::open` callers in the
crate pass `&MmapFs` inline. Tests do the same.
gridstore lib + tests now compile cleanly. Segment + tonic cascade
still pending.
* WIP: segment caller sweep — dynamic_stored_flags first
* WIP: segment cascade - id_tracker partial
* common benches: update to new UniversalReadFs::open shape (clippy clean)
* segment flags: thread Fs through BufferedDynamicFlags / Bitvec / Roaring
DynamicStoredFlags::set_len now takes `fs: &Fs`. BufferedDynamicFlags
stores an `Arc<Fs>` so the flusher closure can call `set_len` on resize.
BitvecFlags and RoaringFlags expose a new `Fs` type parameter and the
flag tests now pass `Fs::default()` (MmapFs/IoUringFs via duplicate_item).
Concrete consumers (bool/null index, mmap dense/multi/sparse storages)
pin `Fs = MmapFs` and pass `&MmapFs` to inner opens.
* segment: thread Fs through field-index lifecycle methods
Apply the new UniversalReadFs::open shape across:
- full_text_index (MmapInvertedIndex, MmapFullTextIndex, UniversalPostings)
- geo_index (StoredGeoMapIndex build/open + tests + builders)
- numeric_index lifecycle (UniversalNumericIndex build/open)
- map_index lifecycle (UniversalMapIndex build/open)
- stored_point_to_values (open / from_iter)
Concrete consumers pin Fs = MmapFs and pass &MmapFs inline; generic
open paths thread `fs: &Fs` where Fs: UniversalReadFs<File = S>.
* segment: thread Fs through chunked vectors and id-tracker callers
- ChunkedVectors gains `Fs` generic so add_chunk can call create_chunk
after open. ChunkedVectorsRead/load_config and chunks::{read_chunks,
create_chunk} take `fs: &Fs`. Concrete callers (dense / multi-dense /
sparse / quantized) pass MmapFs inline.
- DenseVectorStorageImpl stores `fs: Fs` so `update_from` can reopen
ImmutableDenseVectors. ImmutableDenseVectors::open takes `fs: &Fs`.
- VectorStorageEnum DenseUring* variants thread IoUringFs alongside
IoUringFile.
- segment_builder + segment_constructor_base pass MmapFs to
ImmutableIdTracker::{new, open}.
- QuantizedStorage::from_file takes `fs: &Fs`; quantized_vectors callers
pass &MmapFs.
* segment: apply nightly rustfmt after Fs refactor
cargo +nightly fmt --all over the segment crate after the
UniversalReadFs cascade. No semantic changes.
* segment: thread Fs through benches and id-tracker tests
Update the dynamic-mmap-flags and buffered-update-bitslice benches to
the new UniversalReadFs::open shape (pass `&MmapFs`). Update the
immutable-id-tracker test suite to forward `&MmapFs` to
`from_in_memory_tracker` / `open`.
* uio: pin Fs via UniversalRead::Fs assoc type; per-call OpenExtra
Two design changes that fall out of the per-instance Fs refactor:
1. Bidirectional Fs ↔ File pinning. `UniversalRead::Fs:
UniversalReadFs<File = Self>` lets generic-over-`S` code refer to
`S::Fs` directly instead of carrying an extra `<Fs: UniversalReadFs<File = S>>`
generic param. `ReadOnly<S>` wraps a file but has no natural
filesystem; a phantom `ReadOnlyFs<S::Fs>` satisfies the constraint
while inherent `ReadOnly::open` keeps taking `&S::Fs` directly.
2. `prevent_caching` moves from filesystem-instance state to per-call
`UniversalReadFs::OpenExtra: Default`. Was previously a knob on
`IoUringConfigContext` / `IoUringFs`, conflating "how this fs is
built" with "how this file is opened." Now `IoUringFs::OpenExtra =
IoUringOpenExtra { prevent_caching }`; mmap and block-cache use `()`.
`IoUringConfigContext` is gone, `TConfigContext` slims to a `Default`
marker.
Tonic StorageReadService holds `Arc<S::Fs>` (was `PhantomData<S>`); its
`new()` builds via `S::Fs::from_context(default)` and the spawn_blocking
closures clone the Arc to call instance methods.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* uio: cfg-gate IoUringOpenExtra import for non-linux builds
The IoUringOpenExtra reexport from `universal_io` is gated on
`target_os = "linux"`. The previous commit left an unconditional import
in `persisted_hashmap/tests.rs`, breaking macOS/Windows CI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* uio: migrate simple_disk_cache to per-instance Fs API
PR #9097 (merged into dev concurrently with this branch) introduced a
`DiskCache<R: UniversalRead>` using the pre-refactor trait shape:
file-handle-as-filesystem (`R::open`, `R::list_files`), an
`OpenOptionsExtra` field on `OpenOptions`, and trait methods without
`&self`. The Fs-instance refactor on this branch removed all three.
Reshape `simple_disk_cache` to match the new design without breaking the
lazy mirror semantics:
- New `DiskCacheFs<R>` is the filesystem handle. Holds a clone of the
remote `R::Fs`; `list_files`/`exists` delegate; `from_context`
forwards to the inner Fs context. `open` constructs a `DiskCache<R>`
via the global `DiskCacheConfig`.
- `DiskCache<R>` now stores `remote_fs: R::Fs` + `remote_extra:
<R::Fs as UniversalReadFs>::OpenExtra`, so lazy remote opens go
through `self.remote_fs.open(path, options, extra)` instead of the
removed `R::open`. `open_with_config` takes the remote Fs + extra
explicitly (no more hard-coded `prevent_caching: true`; callers pass
the appropriate `OpenExtra`).
- `UniversalRead for DiskCache<R>` now declares `type Fs =
DiskCacheFs<R>` (no more `open` method on the file trait).
- Propagate the necessary bounds (`R::Fs: Clone`,
`<R::Fs as UniversalReadFs>::OpenExtra: Clone`,
`R::OwnedReadPipeline<u8, Range<u32>>: Send`) through `pipeline.rs`
free functions and impl blocks that reach into `DiskCache::remote` /
`local_state`.
- Drop the now-removed `extra: _` destructure in `LocalState::new`.
- Tests construct the remote Fs via `R::Fs::from_context(Default::default())`
and exercise `DiskCache::open_with_config`. 17 simple_disk_cache
tests pass; the 3 `empty_read_does_not_materialize_local_file`
failures pre-exist on dev (verified) and are unrelated.
`mold -run cargo clippy --all-targets` clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: nightly fmt on simple_disk_cache migration
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: nightly fmt for local_state imports after rebase
Co-authored-by: Cursor <cursoragent@cursor.com>
* uio: split DiskCacheFs into its own module
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* uio: replace TConfigContext with OpenExtra trait; move DiskCacheConfig onto DiskCacheFs
- Drop the empty TConfigContext marker; ContextConfig is now unconstrained
so backends can require explicit construction.
- Add OpenExtra trait with with_prevent_caching for backend-agnostic
per-call knobs; impl for () (no-op) and IoUringOpenExtra.
- DiskCacheFs now carries Arc<DiskCacheConfig> via the new
DiskCacheFsContext<C>; the prefill flow moves from the deleted
open_with_config into DiskCacheFs::open so Populate::Blocking /
PreferBackground work through the trait API.
- Remove the DiskCacheConfig global; callers must construct the context
explicitly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: nightly fmt after OpenExtra refactor
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: fix spelling — Implementors → Implementers
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: possible panic insetad of error propagation
* fix: missing cfg annotation
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Daniel Boros <dancixx@gmail.com>
|
||
|
Andrey VasnetsovandClaude Opus 4.7
|
840f6aebf7 |
Append-only mutable segment: groundwork (filter fix + point_id refactor) (#9156)
* refactor: pass point_id into handle_point_version_and_failure Drop the external_id reverse lookup used for error_status correlation and take the external point_id as an explicit argument instead. All callers already have it in scope, and decoupling it from op_point_offset keeps error correlation correct in flows where the old internal_id is tombstoned before the recovery check runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: filter_deferred_and_deleted also consults the deleted bitslice Field-index primary-clause iterators (and the analogous plain/sparse vector-index paths) are routed through PointMappingsRefEnum to apply the deferred-threshold cutoff. The old `filter_deferred` only applied that threshold, so any soft-deleted internal id sitting below the cutoff slipped through whenever its field-index posting was still live. This was fine while the only source of mid-range tombstones was the deferred-tail design, but it breaks the moment a tombstone can land anywhere in the id range. Add an unconditional deleted-bitslice check (single bit test per element) and rename to `filter_deferred_and_deleted` so the contract is visible at the call site. The Either split is preserved so the no-threshold path still avoids the cutoff comparison. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: cargo +nightly fmt Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * style: collapse error-status recovery match arm Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
7aeb31d9c5 |
tests: use uniform from .utils import * in consensus tests (#9155)
* 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> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
9b9e1034d7 |
[UIO] Require T: Send on UniversalRead (#9147)
Introduce a `pub trait Item: bytemuck::Pod + Send` marker (mirroring the existing `UserData` pattern) and use it in place of `T: bytemuck::Pod` on the read side of `UniversalRead`, its pipeline traits, and all impls/wrappers. Some implementations buffer `Vec<MaybeUninit<T>>` that may be transferred across threads in future backends; tightening the bound makes that explicit at the trait level instead of leaving callers to add `+ Send` ad-hoc. Write paths keep the looser `bytemuck::Pod` bound since they only borrow `&[T]`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
98ca1dc0c5 |
Clear id tracker page cache after building a segment (#9137)
The segment builder evicts the page cache of vector storage, quantized vectors, payload, payload index and the vector index after a build to avoid cache pollution, but the id tracker was never cleared. Its on-disk files (mappings, versions, deleted bitslice) are written during the build and stay resident in the page cache, so after each optimization the id tracker files linger as cache even though they are meant to be on-disk only (expected_cache_bytes == 0). Add `IdTracker::clear_cache` (default no-op) plus a `clear_cache_if_on_disk` policy wrapper, and implement the eviction for the mutable and immutable trackers: - ImmutableIdTracker pages out its two mmap-backed storages via madvise and drops the RAM-loaded mappings file via fadvise(DONTNEED). - MutableIdTracker drops its append-only log files via fadvise(DONTNEED). The builder calls `clear_cache_if_on_disk`, mirroring the payload index. The id tracker has no on-disk mode yet, so this always clears for now; a TODO marks where to gate it once an on-disk mode exists. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
b8f501f3a8 |
test(consensus): snapshot transfer with set-payload for missing points (#9125)
* 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> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
45d320310b |
Fix TurboQuant heap memory under-reporting (#9099)
EncodedVectorsTQ was the only quantizer that did not override the EncodedVectors::heap_size_bytes() trait method, so it fell back to the default of 0. For the RAM-backed variants (TQRam/TQRamMulti) this meant the entire resident quantized dataset was reported as 0 bytes and misclassified as fully on-disk by the MemoryReporter; the always-resident quantizer tables (rotation + TQ+ error-correction vectors) and encoding buffer were also uncounted for every variant. Make heap_size_bytes() a required trait method (remove the default impl) so every quantizer must account for its own heap explicitly, then add the missing TurboQuant implementation: storage backend + quantizer tables + encoding buffer. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
03e15b5ed4 |
refactor: move deferred-point ownership into ID tracker (#9062)
* refactor: move deferred-point ownership into the ID tracker Re-implements the idea from #8512 against current `dev`. Deferred-point state (`deferred_internal_id` + `deferred_deleted_count`) moves out of `Segment.deferred_point_status` and the cached `SparseVectorIndex.deferred_internal_id` field into `PointMappings`, exposed through `IdTrackerRead`. The threshold is set once at `MutableIdTracker::open` time; `PointMappings::drop` now maintains the deleted counter inline (with double-delete protection), removing the manual increment in `delete_point_internal` and the `calculate_deleted_deferred_point_count` rescan. Read paths consume the threshold through the id tracker: - The segment read view drops the `deferred_point_status` field and `with_view` no longer threads it in; `read_view/{deferred,info}.rs` call `self.id_tracker.deferred_*()` directly. - `SparseVectorIndex` no longer stores its own copy and its `update_vector` / search debug-assert read from `self.id_tracker.borrow().deferred_internal_id()`. - `VectorQueryContext.deferred_internal_id` and the `SegmentQueryContext::get_vector_context` parameter are gone; the three downstream readers (`plain_vector_index`, sparse search, sparse `update_vector`) consult their own id tracker. `PointMappingsRefEnum` centralises the dispatch: - `iter_internal_with_behavior(DeferredBehavior)` replaces ad-hoc branches in `iter_filtered_points` impls. - `external_iter_cutoff(DeferredBehavior)` covers iterators sourced outside the mapping (field-index outputs in `struct_payload_index::iter_filtered_points`). - The internal `deferred_internal_id()` accessor is private; the raw threshold no longer leaks to consumers. - `iter_from_visible` / `iter_random_visible` read the mapping's own threshold; callers that previously passed `DeferredBehavior::apply(...)` now branch on `deferred_behavior.include_all_points()` (scroll / order_by) or simply drop the argument (sampling / facet). `PayloadIndexRead::query_points` drops the now-redundant `deferred_internal_id` parameter; `iter_filtered_points` takes `DeferredBehavior` directly so HNSW build/search can request `IncludeAll` while normal reads request `Exclude`. RocksDB-related parts of the original PR are skipped — that tracker is already gone from `dev`. Tests adapted: sites that mutated `segment.deferred_point_status` directly now construct a parallel non-deferred segment via `create_deferred_segment(..., 0)` for comparison; `test_deleted_deferred_point_count` reads counters through the id tracker. See `docs/plans/deferred-points-owned-by-id-tracker.md` for the design write-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(benches): drop stale deferred_internal_id arg from query_points calls The boolean / range / conditional bench files weren't built by `cargo test -p segment`, so they slipped through. `cargo clippy --workspace --all-targets` catches them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: drop id_tracker / point_mappings args from iter_filtered_points Both impls already hold an id tracker on `self`: - `StructPayloadIndexReadView` carries `id_tracker: &'a I`, so `self.id_tracker.point_mappings()` borrows from `'a` and the lazy iterator chain keeps working unchanged. - `PlainPayloadIndex` carries `id_tracker: Arc<AtomicRefCell<...>>`, where the mapping borrow is local; collect into a `Vec` and return `into_iter()`. PlainPayloadIndex::iter_filtered_points has no direct callers — only `query_points` was using it — so eager collection is a non-issue. While here, take `self` by value on `iter_internal_visible`, `iter_from_visible`, `iter_random_visible`, `iter_internal_with_behavior`, and `external_iter_cutoff`. `PointMappingsRefEnum` is `Copy`; this matches the existing `iter_internal` / `iter_from` / `iter_random` shape and lets the iterator outlive a local `let point_mappings = ...;` binding. The HNSW `condition_points` helper drops its now-unused `id_tracker` parameter. All callers (sampling, scroll, order_by, facet ×2, hnsw build/search) just drop the two arguments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: ignore /docs/plans/ and untrack the previously-committed plan `docs/plans/` is a scratch directory for per-feature planning notes — not something we want under source control. Add it to `.gitignore` and drop the deferred-points plan that slipped into history; the design is captured in the PR description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: replace external_iter_cutoff with filter_deferred iterator wrapper Instead of exposing a raw `Option<PointOffsetType>` cutoff that every caller has to apply with their own `.filter(...)`, give `PointMappingsRefEnum` an iterator wrapper: fn filter_deferred<I: Iterator<Item = PointOffsetType>>( self, iter: I, deferred_behavior: DeferredBehavior, ) -> impl Iterator<Item = PointOffsetType> It returns the iterator unchanged for `IncludeAll` (or when the mapping has no threshold) and otherwise wraps it in a cutoff `.filter`, dispatched via `itertools::Either` so the no-cutoff path stays allocation-free. The struct payload index's `iter_filtered_points` swaps its open-coded filter for a single `point_mappings.filter_deferred(...)` call. The deferred threshold no longer leaks out of `PointMappingsRefEnum`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: move deferred wrapping out of peek_top_all, gate it as test-only `BatchFilteredSearcher::peek_top_all` baked the deferred cutoff into its iterator construction, which was the last place outside `PointMappingsRefEnum` that knew about the threshold. Split the deleted-iteration concern out into a new accessor: fn iter_not_deleted(&self) -> impl Iterator<Item = PointOffsetType> + 'a It borrows `&'a BitSlice` directly (not via `&self`), so callers can chain `filter_deferred` and then move `self` into `peek_top_iter` without lifetime conflicts. Sparse + plain vector index call sites now do: let iter = id_tracker .point_mappings() .filter_deferred(searcher.iter_not_deleted(), DeferredBehavior::Exclude); searcher.peek_top_iter(iter, &is_stopped) leaving `BatchFilteredSearcher` completely ignorant of deferred state. With deferred handling lifted out, `peek_top_all` itself is now used only by tests (3 inline `#[cfg(test)] mod tests`, 1 integration test, 1 bench) — gate it under `#[cfg(feature = "testing")]` to match `new_for_test`. Production code goes through the `iter_not_deleted` + `filter_deferred` + `peek_top_iter` composition. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: optimize Segment::retrieve and thread user data through read_vectors Two interlocking changes that together collapse the per-point lookups and intermediate allocations in `Segment::retrieve` down to one external-to-internal pass. ## `IdTrackerRead::resolve_external_ids` (new default trait method) Single-pass translation of a `&[PointIdType]` slice into two parallel vectors `(Vec<PointIdType>, Vec<PointOffsetType>)`. Folds deferred filtering (compare offset against the threshold inline — no separate `point_is_deferred` lookup) and missing-id errors (eager `PointIdError`) into resolution. Lives on the trait so the deferred threshold never leaks out of the id tracker; the parallel-vector shape lets a future batched payload / vector fetcher consume `&offsets` straight without unzipping. The `appendable_flag` guard previously in `point_is_deferred` is gone: non-appendable trackers always carry `deferred_internal_id() == None` (set only via `MutableIdTracker::open`, guarded by the segment constructor), so the check was load-bearing nowhere. ## User-data threading through `read_vectors` `VectorStorageRead::read_vectors` now takes `IntoIterator<Item = (U, PointOffsetType)>` and yields `(U, PointOffsetType, CowVector)`. The user-data tag rides alongside each offset all the way through, so callers can map results back into a parallel array without keeping a separate `offset → ...` lookup table. - Default trait impl: one-line per-key loop. - Dense impl: `unzip()` into parallel `(Vec<U>, Vec<PointOffsetType>)` in a single pass — same allocation count as before, just U riding alongside. - Enum delegations (`VectorStorageEnum`, `VectorStorageReadEnum`) forward unchanged. - `for_each_in_batch` and below stay untouched. `SegmentReadView::vectors_by_offsets<U: Copy>` becomes a lazy filter chain — no parallel `Vec<(orig_idx, offset)>` allocation. The dead `SegmentReadView::read_vectors` helper is removed. ## `Segment::retrieve` end-to-end Per N points / V vectors / payload: | Operation | Before | After | |----------------------------|---------------------|-------| | `id_tracker.internal_id` | N × (1 + V + 1) | N | | `id_tracker.external_id` | N × V | 0 | | `point_is_deferred` | N (when applicable) | 0 | | `offset_to_id` HashMap | N entries | none | | `Vec` in `vectors_by_offsets` | 1 | 0 | The vectors stage passes the external id as `read_vectors`'s user data — the callback gets `id` directly without any index lookup. The payload stage uses `payload_by_offset` against the already-resolved offsets. The shape is also batch-friendly: swapping in a future `IdTrackerRead::batch_internal_id` or `payload_index.batch_get_payload` needs no changes outside the two call sites. ## Behavioural notes - Missing-id now errors eagerly inside resolution, instead of in the vectors stage (`WithVector::Bool(true)` / `Selector`) or payload stage (`with_payload.enable`). The previous `WithVector::Bool(false)` + no-payload path silently inserted an empty record; that is now also an error. None of the existing callers (search post-processing, external retrieve API, the deferred-points test on tests/mod.rs:1179) pass non-existent ids. - Added a per-payload `check_stopped`; the vectors stage already had `stop_if` on its iterator chain. - `vector_by_offset` (the single-element helper) passes `()` as the no-op user data. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: apply rustfmt to optimised retrieve / read_vectors paths Pre-push hook failure on the previous commit was rustfmt. Same content, formatted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * do not error out on missing points in retrieve --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
62594253c2 |
expose universal read in read only id tracker (#9057)
* Expose Universal Read in read-only ID tracker * fmt |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
fd027ca4a2 |
refactor: read-only numeric index (#9038)
* refactor: split numeric index variants into dedicated modules Move MutableNumericIndex, ImmutableNumericIndex, and MmapNumericIndex into their own directories, each split into mod.rs (struct definitions), lifecycle.rs (open/build/wipe/mutations), and read_ops.rs (accessors), mirroring the map_index layout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: drop single-variant Storage enums in mutable/immutable numeric index Replace `Storage<T>` wrappers around the only backing store with the store types directly: `Gridstore<Vec<T>>` for `MutableNumericIndex` and `Box<MmapNumericIndex<T>>` for `ImmutableNumericIndex`. Collapses the trivial single-arm matches into direct method calls. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: introduce NumericIndexRead trait Mirror `MapIndexRead` from the map_index refactor: define a `NumericIndexRead<T>` trait in `numeric_index/read_ops.rs` and implement it on each of the three storage variants (`MutableNumericIndex`, `ImmutableNumericIndex`, `MmapNumericIndex`). Trait signatures are unified across variants — in-memory variants accept and ignore the `hw_counter` argument that the mmap-backed variant uses for IO tracking, and `total_unique_values_count`, `values_range`, and `orderable_values_range` return `OperationResult` everywhere so the dispatcher in `NumericIndexInner` can call them generically. Variant-specific helpers that don't fit the shared shape stay as inherent methods: `MutableNumericIndex::map()`, `ImmutableNumericIndex::values_range_size()`, and `MmapNumericIndex::{values_range_size, is_on_disk}`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: add ReadOnlyAppendableNumericIndex Counterpart to `MutableNumericIndex`, mirroring `ReadOnlyAppendableMapIndex` from the map_index refactor. It reuses the shared `InMemoryNumericIndex` in-memory state but is backed by a `GridstoreReader` over generic `UniversalRead` instead of a writable `Gridstore`, and implements `NumericIndexRead` by forwarding to the in-memory index — no mutation surface. Loading / lifecycle (constructor, files, populate, clear_cache) will follow in a separate change; the storage field is held only to pin the on-disk layout for now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: rename MmapNumericIndex to UniversalNumericIndex, expose UniversalRead param Mirror `UniversalMapIndex`: the type is now generic over `S: UniversalRead` with a `MmapFile` default, so the index can be served from any `UniversalRead` backend (io_uring, disk-cache wrappers, …) rather than the hard-coded `MmapFile`. The `NumericIndexRead` impl and read-side helpers are generic over `S`; `build` / `open` and the other lifecycle methods stay `MmapFile`-only since they construct mmap-backed storage from a path. The `NumericIndexInner::Mmap` enum variant keeps its name and uses the default `S = MmapFile`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: split numeric_index/storage/mod.rs into lifecycle and read_ops `storage/mod.rs` now holds only the `NumericIndexInner` enum and module wiring. The variant-dispatch impls are split into sibling modules matching the layout of the individual storage variants: - `lifecycle.rs`: construction, persistence, file listing, cache control, and `remove_point`. - `read_ops.rs`: read-path forwarding — value lookups, telemetry, RAM accounting, `is_on_disk`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: implement NumericIndexRead for NumericIndexInner The enum-level read dispatch was a set of inherent methods scattered across storage/read_ops.rs and storage/statistics.rs with signatures that drifted from the variant trait (`values_count` returned `usize`, `max_values_per_point` vs `get_max_values_per_point`, a hand-rolled `get_telemetry_data`). Make `NumericIndexInner` implement `NumericIndexRead` directly so it shares one interface with the three storage variants. - All 12 trait methods are forwarded via match dispatch in storage/read_ops.rs; `values_range` / `orderable_values_range` box the per-variant iterators. - `get_histogram`, `get_points_count`, `total_unique_values_count` move out of statistics.rs into the trait impl; `values_is_empty` and `get_telemetry_data` now come from the trait defaults. - `point_ids_by_value` and `is_on_disk` stay as enum-only inherent helpers (not part of the shared trait). - Callers updated: `NumericIndex::values_count` unwraps the now `Option`-returning trait method; `filter` boxes `point_ids_by_value`; `field_index.rs` and `numeric_field_index.rs` import the trait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: add ReadOnlyNumericIndexInner Read-only counterpart to `NumericIndexInner`, mirroring `ReadOnlyMapIndex` from the map_index refactor. Lives under `numeric_index/storage/read_only` and selects across the two read-only storage backends: - `Appendable(ReadOnlyAppendableNumericIndex<T, S>)` — loaded into RAM from the appendable Gridstore format. - `Immutable(UniversalNumericIndex<T, S>)` — served directly from the immutable stored format. Implements `NumericIndexRead` by forwarding each method to the active variant; `values_is_empty` / `get_telemetry_data` come from the trait defaults. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: rename numeric_index read_ops to numeric_index_read, split mod.rs Two changes: - Rename `numeric_index/read_ops.rs` (the `NumericIndexRead` trait definition) to `numeric_index_read.rs`, freeing the `read_ops` name. - Split the leftover content of `numeric_index/mod.rs` into sibling modules, matching the per-variant layout: - `lifecycle.rs`: the `Encodable` key-format trait + impls and the `HISTOGRAM_*` construction constants. - `read_ops.rs`: the `StreamRange` trait and the `Range` → index-key-bounds conversion. `mod.rs` now only wires modules and re-exports. `Encodable` and `StreamRange` keep their public paths via re-export; `tests.rs` gains explicit imports for the symbols it previously picked up through the `mod.rs` glob. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: dissolve numeric_index/index.rs into mod, lifecycle, read_ops `index.rs` only held the `NumericIndex` wrapper and its two impl blocks; spread them to match the per-module layout used elsewhere: - `NumericIndex` struct + `NumericIndexIntoInnerValue` trait → `mod.rs` (type definitions live with the module wiring). - The inherent `impl NumericIndex` (open / build / cache control / storage introspection) → `lifecycle.rs`, alongside the `HISTOGRAM_*` seed constants. - The `PayloadFieldIndexRead` impl → `read_ops.rs`. Also move the `Encodable` key-format trait out of `lifecycle.rs` into its own `encodable.rs`. `mod.rs` keeps re-exporting `Encodable` so its public path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: add ReadOnlyNumericIndex with NumericIndexRead + PayloadFieldIndexRead Read-only counterpart to `NumericIndex`, wrapping `ReadOnlyNumericIndexInner` plus the payload value type parameter `P`. Implements both `NumericIndexRead` and `PayloadFieldIndexRead` by forwarding to the inner storage-variant enum. To support `PayloadFieldIndexRead` without duplicating the query logic, the cardinality/filter/payload-block/condition-checker code is extracted into a new `query` module of generic free functions over `NumericIndexRead<T>`. `ReadOnlyNumericIndexInner` implements `PayloadFieldIndexRead` by plugging into those helpers; `ReadOnlyNumericIndex` delegates to its inner. The writable `NumericIndexInner` path is left untouched — its existing variant-specialized `estimate_points` heuristic stays in `storage`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: make query.rs the single source of truth for numeric index queries The generic `query` helpers and the per-`NumericIndexInner` impls in `storage/{trait_impls,statistics}.rs` had duplicated cardinality / filter / payload-block / condition-checker logic. Collapse them onto the shared `query` helpers: - `storage/trait_impls.rs`: `PayloadFieldIndexRead for NumericIndexInner` now forwards each method to `query::*` instead of carrying its own copy. - `storage/statistics.rs`: deleted — `range_cardinality` and `estimate_points` were duplicates of the `query` versions. - `estimate_points` needs a range size; add `values_range_size` to the `NumericIndexRead` trait with a default that counts `values_range`, overridden by the `Immutable` / `Mmap` variants with their `O(log n)` boundary search. The `MutableNumericIndex::map()` accessor (its only caller was the old `estimate_points`) is removed. - `values_range_size` takes `hw_counter` and threads it into `values_range` rather than fabricating a disposable counter. `tests.rs` calls `query::range_cardinality` directly now that the inherent method is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: integrate read-only numeric index into ReadOnlyFieldIndex Wire the four numeric variants (`IntIndex`, `DatetimeIndex`, `FloatIndex`, `UuidIndex`) into `ReadOnlyFieldIndex`, mirroring `FieldIndex`: - `PayloadFieldIndexRead` / `FieldIndexRead` dispatch covers the new variants — telemetry, value counts, value retrievers, `as_numeric`. - `ReadOnlyNumericFieldIndex` is the read-only counterpart of `NumericFieldIndex` (Int/Float order-by erasure over `ReadOnlyNumericIndexInner`); `as_numeric` returns it for the Int/Datetime/Float variants (UUIDs aren't numerically order-by-able, matching `FieldIndex`). - `ReadOnlyNumericIndex` gains per-`(T, P)` `value_retriever` methods (in `read_only/value_retriever.rs`) and an `inner()` accessor. `StreamRange` is now backed by a shared generic `query::stream_range` helper over `NumericIndexRead`, implemented for both `NumericIndexInner` and `ReadOnlyNumericIndexInner` — replacing the bespoke `EitherVariant` dispatch in `storage/trait_impls.rs`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: collapse ReadOnlyNumericIndex value retrievers onto one generic method The four per-`(T, P)` `value_retriever` methods were identical except for the per-value `T -> Value` conversion. Extract that conversion into a `NumericValueToJson` trait (one tiny impl per `(T, P)`) and keep a single generic `value_retriever` that builds the retriever closure once. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fmt * refactor: dedup NumericFieldIndex / ReadOnlyNumericFieldIndex The two enums were structurally identical — same `StreamRange`, `get_ordering_values`, and `NumericFieldIndexRead` bodies — differing only in the backing storage type. Collapse them onto one generic `NumericFieldIndexView<'a, I, F>` with a single set of impls (over `I: NumericIndexRead<i64> + StreamRange<i64>` and the `f64` counterpart). `NumericFieldIndex` and `ReadOnlyNumericFieldIndex` are now type aliases of the generic view, so every existing call site is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
f926c6009d |
test: fix race in test_corrupted_snapshot_recovery (#9013)
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> |
||
|
Andrey Vasnetsov
|
dc2a6df244 |
read only map index (#9025)
* rename mmap -> universal * [AI] Introduce `ReadOnlyMapIndex` and implement `MapIndexRead` for it * add missing file * [AI] Implement read-only PayloadFieldIndexRead / FieldIndexRead (#9026) Wires `ReadOnlyFieldIndex` into the read-only field-index surface that `ReadOnlyMapIndex` and `ReadOnlyNullIndex` already provide, sharing the non-trivial bodies with the existing `MapIndex` impls. - Extracted per-`N` `PayloadFieldIndexRead` bodies (filter, estimate_cardinality, for_each_payload_block, condition_checker) in `payload_index_impl/{str,int,uuid}.rs` into free functions over `T: MapIndexRead<N>`. Both `MapIndex<N>` and `ReadOnlyMapIndex<N, S>` now impl `PayloadFieldIndexRead` via thin delegation to those bodies. - Lifted `value_retriever` bodies in `value_indexer_impl.rs` the same way; `ReadOnlyMapIndex<N, S>` gets the per-K closure for free. - Added `telemetry_index_type` (required) + `get_telemetry_data` (default) to `MapIndexRead`, mirroring the `NullIndexRead` pattern; every map-index variant now reports telemetry without an inherent method on the enum. - Promoted `MapIndexRead` + its module to `pub` so `field_index_base/read_only/` can name them. - Implemented `FacetIndex for ReadOnlyMapIndex<N, S>` and extended `FacetIndexEnum` with `S: UniversalRead = MmapFile` plus the three read-only variants. `FieldIndex::as_facet_index` pins `S = MmapFile` via turbofish to keep inference happy. - `ReadOnlyFieldIndex::as_numeric` returns `None` for now; no concrete read-only numeric type exists yet (the doc on `NumericFieldIndexRead` already notes this is intended for a future `ReadOnlySegment`). |
||
|
Andrey Vasnetsov
|
452473099e |
refactor(map_index): extract inner state; add ReadOnlyAppendableMapIndex skeleton (#9020)
* refactor(map_index): extract MutableMapIndexInner; add ReadOnlyAppendableMapIndex
Extract the four in-memory fields (`map`, `point_to_values`, `indexed_points`,
`values_count`) of `MutableMapIndex<N>` into a shared inner struct
`MutableMapIndexInner<N>` under `mutable_map_index/inner.rs`. The
`MapIndexRead<N>` impl moves wholesale onto the inner. `MutableMapIndex<N>`
becomes a thin wrapper `{ inner, storage: Gridstore<...> }`; its
`MapIndexRead<N>` impl forwards to the inner. The Gridstore load loop in
`open_gridstore` collapses to `MutableMapIndexInner::empty()` +
`inner.ingest(idx, value)` per row, sharing the per-value ingestion path with
the future read-only loader.
Add a new sibling `mutable_map_index/read_only/` module containing
`ReadOnlyAppendableMapIndex<N>` (inner + `GridstoreReader<Vec<...>, MmapFile>`),
with its `MapIndexRead<N>` impl forwarding to the same inner. No constructor
yet — loading and lifecycle for the read-only variant will land in a
follow-up. Field `storage` is held to pin the on-disk type and tagged
`#[allow(dead_code)]` until then.
No behavioural change. The 17 existing map_index tests pass unchanged.
* expose storage generic
|
||
|
Andrey Vasnetsov
|
67295ef87f |
refactor(map_index): split mod.rs into read_ops, lifecycle, tests (#9015)
* refactor(map_index): split mod.rs into read_ops, lifecycle, tests
mod.rs is now ~40 lines containing only the MapIndex enum, type
aliases, and module declarations.
- read_ops.rs: read-only inherent methods (get_values, get_iterator,
for_each_*, except_cardinality, except_set, telemetry, ram_usage,
mutability/storage type)
- lifecycle.rs: open/builder/flush/wipe/remove_point/files/populate/
clear_cache
- tests.rs: all #[cfg(test)] tests
Also folded payload_index_impl_{int,str,uuid}.rs into a dedicated
payload_index_impl/ submodule.
* refactor(map_index): split storage submodules into dedicated dirs (#9016)
* refactor(map_index): split storage submodules into dedicated module dirs
Turn each of the three storage implementation files into a directory
module split into read_ops + lifecycle, mirroring the parent module
layout introduced in #9015.
- mutable_map_index/{mod,lifecycle,read_ops}.rs
- immutable_map_index/{mod,lifecycle,read_ops}.rs
- mmap_map_index/{mod,lifecycle,read_ops}.rs
Each mod.rs holds only the struct definitions, internal Storage type,
and config/constants. read_ops.rs holds the read-only query methods;
lifecycle.rs holds open/build/flush/wipe/files/remove_point and
internal mutation helpers.
Pure refactor — no behavior change.
* refactor(map_index): introduce MapIndexRead trait
Define a unified read-only trait `MapIndexRead<N>` describing the
methods every storage variant exposes (check_values_any, get_values,
get_iterator, for_each_*, storage_type, ram_usage_bytes, etc.).
Each storage variant's read_ops.rs now contains a trait impl instead
of inherent methods. Signatures are unified across variants:
- `hw_counter` is accepted by every method that needs it for the mmap
variant; mutable / immutable accept and ignore it.
- `check_values_any` returns `bool` (mmap absorbs IO errors internally
with the existing FIXME, matching the parent's prior `.unwrap_or`).
- `for_each_count_per_value` takes `deferred_internal_id` uniformly;
the immutable variant `debug_assert!`s it is `None`.
`for_points_values` keeps its variant-specific callback signatures
and stays as an inherent method — it's only used by FacetIndex with
explicit pattern matching.
Pure refactor — no behavior change.
* refactor(mutable_map_index): drop single-variant Storage enum
The `Storage<T>` enum had only one variant (`Gridstore`), so every
`match &self.storage { Storage::Gridstore(s) => ... }` was just
unwrapping the same path. Replace the field with `Gridstore<Vec<...>>`
directly and inline every match.
|
||
|
|
89c488a497 |
refactor(field_index): make FieldIndexRead a supertrait of PayloadFieldIndexRead (#8996)
* refactor(field_index): make FieldIndexRead a supertrait of PayloadFieldIndexRead Removes the `get_payload_field_index_read() -> &dyn PayloadFieldIndexRead` bridge from `FieldIndexRead` and the five default impls that forwarded through it. The overlapping read methods now come from the supertrait directly, eliminating one layer of dynamic dispatch on the hot read path: `FieldIndex::filter` → variant match → concrete typed-index method. `FieldIndex` gains a direct `impl PayloadFieldIndexRead` block with per-method match arms, mirroring the existing dispatch shape used by `get_telemetry_data`, `values_count`, etc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: apply nightly rustfmt Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(numeric_index): split mod.rs into focused submodules (#8997) * refactor(numeric_index): split mod.rs into focused submodules `numeric_index/mod.rs` was 1308 lines mixing the storage-dispatch enum, the public `NumericIndex<T, P>` wrapper, three builders, per-(T, P) `ValueIndexer` impls, and the `Encodable`/`StreamRange` traits — hard to navigate. Split into: - `mod.rs` keeps the shared traits (`Encodable`, `StreamRange`), `Range<T>::as_index_key_bounds`, and re-exports. - `wrapper.rs` — `NumericIndex<T, P>` + inherent impl + `NumericIndexIntoInnerValue` trait. - `builders.rs` — `NumericIndexBuilder`, `NumericIndexMmapBuilder`, `NumericIndexGridstoreBuilder`. - `value_indexer.rs` — `ValueIndexer` and per-(T, P) `value_retriever` inherent impls. - `storage/` — the `NumericIndexInner` dispatch enum: - `storage/mod.rs` — enum + simple match-and-forward (constructors, lifecycle, telemetry, per-point access). - `storage/statistics.rs` — histogram-driven cardinality and point-count helpers. - `storage/trait_impls.rs` — `PayloadFieldIndex`, `PayloadFieldIndexRead`, `StreamRange` impls. Pure code reorganization — no behavior change. A few inherent methods on `NumericIndexInner` had to widen from private to `pub(super)` / `pub(in crate::index::field_index::numeric_index)` to remain reachable across the new module boundaries (and from `tests.rs`); the new constructors (`NumericIndexMmapBuilder::new`, `NumericIndexGridstoreBuilder::new`) replace direct field construction across files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(numeric_index): rename wrapper.rs -> numeric_index.rs, move point_ids_by_value out of statistics - Renamed `wrapper.rs` to `numeric_index.rs`, matching the central `NumericIndex` type and the surrounding module name. - Moved `point_ids_by_value` from `storage/statistics.rs` to `storage/mod.rs` next to `get_values`. It is an exact value->points lookup primitive, not a cardinality estimate; the statistics module is left to the histogram-driven helpers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(clippy): rename numeric_index submodule to index to fix module_inception lint Clippy's module_inception rule disallows a module with the same name as its containing module. Rename numeric_index.rs -> index.rs and update the three internal references. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(field_index): push PayloadFieldIndexRead through NumericIndex and move special_check_condition per-variant (#8998) Two related cleanups that share a theme — drop enum-level read dispatch in favor of per-variant trait impls: 1. `NumericIndex<T, P>` now implements `PayloadFieldIndexRead` directly (forwarding to its inner storage enum). The four numeric arms in `FieldIndex`'s `impl PayloadFieldIndexRead` drop their `.inner()` calls, so all eleven variants now use a uniform `idx.<method>(...)` form. 2. `special_check_condition` moves from `FieldIndexRead` to `PayloadFieldIndexRead` with a default `Ok(None)` body. `FullTextIndex` (the only variant with non-trivial logic) overrides it. `NumericIndex<T, P>` forwards through to the inner enum, and the `FieldIndex` enum's match dispatch moves out of `FieldIndexRead` into `PayloadFieldIndexRead` for consistency with the other five trait methods. Removed the now-redundant declaration from `FieldIndexRead` (inherited via the supertrait). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <agent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <agent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Andrey Vasnetsovandxzfc
|
b12b090ae5 |
refactor mmap hashmap (#8405)
* AlignedBuf * UniversalHashMap * Update wording --------- Co-authored-by: xzfc <xzfcpw@gmail.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
a6934da495 |
refactor(field_index): extract FieldIndexRead impl into separate file (#8995)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
ca757cd13a |
refactor(read_view): make StructPayloadIndexReadView generic over F: FieldIndexRead (#8994)
The read view's `field_indexes` was concretely typed as `&IndexesMap = &HashMap<PayloadKeyType, Vec<FieldIndex>>`. Replace the FieldIndex with a generic parameter F: FieldIndexRead, so the view advertises a dependency on the trait surface rather than the concrete enum. `StructPayloadIndex::with_view` instantiates `F = FieldIndex`; any future consumer that implements FieldIndexRead (e.g. a read-only segment with a narrower field-index representation) can plug in directly. Changes: 1. StructPayloadIndexReadView gains a fourth generic parameter F bound by FieldIndexRead. `field_indexes` is now `&HashMap<PayloadKeyType, Vec<F>>`. All five impl blocks across read_view/ propagate the F generic. 2. `check_field_condition` / `select_nested_indexes` / `check_payload` in payload_storage/query_checker.rs become generic over `FI: FieldIndexRead` (and `R: AsRef<Vec<FI>>`). The bodies only call `special_check_condition` which is on FieldIndexRead, so the generalization is free. 3. `variable_retriever` in read_view/value_retriever/helpers.rs gains an F generic; the body already only uses FieldIndexRead methods after the previous commits in this PR. 4. `with_view` and `SegmentReadViewFor` instantiate F = FieldIndex. No external consumer breaks. 5. Tests construct the view with an inferred F. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generall
|
795fe4bd7e |
Revert "refactor(read_view): make StructPayloadIndexReadView generic over F: FieldIndexRead"
This reverts commit
|
||
|
generallandClaude Opus 4.7
|
0b01e70e4a |
refactor(read_view): make StructPayloadIndexReadView generic over F: FieldIndexRead
The read view's `field_indexes` was concretely typed as `&IndexesMap = &HashMap<PayloadKeyType, Vec<FieldIndex>>`. Replace the FieldIndex with a generic parameter F: FieldIndexRead, so the view advertises a dependency on the trait surface rather than the concrete enum. `StructPayloadIndex::with_view` instantiates `F = FieldIndex`; any future consumer that implements FieldIndexRead (e.g. a read-only segment with a narrower field-index representation) can plug in directly. Changes: 1. StructPayloadIndexReadView gains a fourth generic parameter F bound by FieldIndexRead. `field_indexes` is now `&HashMap<PayloadKeyType, Vec<F>>`. All five impl blocks across read_view/ propagate the F generic. 2. `check_field_condition` / `select_nested_indexes` / `check_payload` in payload_storage/query_checker.rs become generic over `FI: FieldIndexRead` (and `R: AsRef<Vec<FI>>`). The bodies only call `special_check_condition` which is on FieldIndexRead, so the generalization is free. 3. `variable_retriever` in read_view/value_retriever/helpers.rs gains an F generic; the body already only uses FieldIndexRead methods after the previous commits in this PR. 4. `with_view` and `SegmentReadViewFor` instantiate F = FieldIndex. No external consumer breaks. 5. Tests construct the view with an inferred F. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
91653b6aea |
refactor(field_index): add condition_checker trait method (foundation) (#8978)
* refactor(field_index): add condition_checker trait method (foundation) First PR in the condition-checker-migration sub-plan (docs/plans/field-index-read-trait/condition-checker-migration/). Purely additive: every existing typed index keeps the default `None` impl, so the legacy match in `field_condition_index` handles every condition. Behaviour unchanged. Changes: 1. PayloadFieldIndexRead::condition_checker — new method with default `None` body. Lets each typed index advertise the conditions it can serve. 2. FieldIndexRead::get_payload_field_index_read — new required method returning `&dyn PayloadFieldIndexRead`. Distinct from the existing inherent `get_payload_field_index` (which returns `&dyn PayloadFieldIndex`, exposing write-side methods) — the read path now hands back a handle that can't accidentally call wipe / flusher / files. 3. Five pure-delegation methods on FieldIndexRead get default impls using the new getter (count_indexed_points, filter, estimate_cardinality, for_each_payload_block, condition_checker). `impl FieldIndexRead for FieldIndex` drops their explicit bodies. 4. field_condition_index in read_view/condition_converter/helpers.rs tries `index.condition_checker(...)` first, then falls through to the existing legacy match. The match handles everything until per-variant overrides land in the follow-up PRs. Why `&dyn` rather than `impl PayloadFieldIndexRead + '_`: RPITIT requires a single concrete return type, but the per-variant arms produce `&NumericIndexInner<T>`, `&MapIndex<K>`, `&GeoMapIndex`, etc. — different types. A unifying borrowed-enum (à la FacetIndexEnum) would be more code than the trait-object indirection saves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate GeoMapIndex into condition_checker (#8979) * refactor(field_index): migrate GeoMapIndex condition checking into condition_checker Sub-plan PR 1 of the condition-checker-migration. GeoMapIndex now serves geo conditions (geo_radius, geo_bounding_box, geo_polygon) directly through PayloadFieldIndexRead::condition_checker. The corresponding match arms and three helper functions in the read view's helpers.rs are gone — the polymorphic dispatch in field_condition_index handles them. Changes: 1. impl PayloadFieldIndexRead for GeoMapIndex gains a condition_checker override. Bodies copied from get_geo_radius_checkers / get_geo_bounding_box_checkers / get_geo_polygon_checkers; the `&'a self` capture replaces the &FieldIndex match arm. 2. The override destructures FieldCondition explicitly (no `..`, every field named with either a binding or `_:`). This forces a compile error if a new field is added to FieldCondition, so each index variant must explicitly decide whether to handle it. Same pattern is mandated for the remaining variant migrations — see docs/plans/field-index-read-trait/condition-checker-migration/00-overview.md. 3. read_view/condition_converter/helpers.rs drops the three Geo FieldCondition arms from field_condition_index's legacy match and deletes the three helper functions. The catch-all is relaxed to `geo_radius: _ | geo_bounding_box: _ | geo_polygon: _` since the trait method handles all geo cases now. No behavioural change: non-Geo variants continue to return None for geo conditions (the trait default returns None), and the read view's catch-all still falls back to the payload check. Stacked on #8978 (condition-checker-trait foundation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate NullIndex into condition_checker (#8980) * refactor(field_index): migrate NullIndex condition checking into condition_checker Sub-plan PR 2 of the condition-checker-migration. NullIndex now serves is_empty / is_null conditions directly through PayloadFieldIndexRead::condition_checker. The dispatch in the read view's condition_converter/mod.rs collapses to mirror the Condition::Field arm: try every index for the field, fall back to the payload check on unwrap_or_else. Changes: 1. impl PayloadFieldIndexRead for NullIndex gains a condition_checker override. Explicit FieldCondition destructure (no `..`) — binds is_empty and is_null, every other field uses `_:` to document intentional non-handling. 2. read_view/condition_converter/helpers.rs drops the is_empty / is_null arms from field_condition_index's legacy match and deletes five orphaned helpers: get_is_empty_checker, get_is_null_checker, get_is_empty_indexes, get_null_index_is_empty_checker, get_fallback_is_empty_checker. 3. read_view/condition_converter/mod.rs rewrites Condition::IsEmpty and Condition::IsNull to use field_condition_index with a constructed FieldCondition, same shape as Condition::Field. Behavioural note (decision locked during review): The legacy values_is_empty fast-path for "field has indexes but no NullIndex" is dropped. NullIndex was introduced in v1.13.5 (#6088); we're on v1.18.0, so any collection touched in the past ~5 minor releases has a NullIndex. Segments untouched since pre-v1.13.5 fall through to the payload check on is_empty queries — no longer accelerated by another index's values_is_empty. A re-index recovers the original perf. Stacked on #8979 (GeoMapIndex) → #8978 (foundation) → dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate NumericIndex into condition_checker (#8981) * refactor(field_index): migrate NumericIndex condition checking into condition_checker Sub-plan PR 3 of the condition-checker-migration. NumericIndexInner<T> now serves Range conditions directly through PayloadFieldIndexRead::condition_checker. The generic impl handles all four FieldIndex variants (IntIndex, FloatIndex, DatetimeIndex, UuidIndex) with one trait override. Changes: 1. impl PayloadFieldIndexRead for NumericIndexInner<T> gains a condition_checker override. Explicit FieldCondition destructure (no `..`) binds `range`. Body converts the RangeInterface bounds into T via T::from_f64 / T::from_u128 — same conversion `filter` already uses. 2. read_view/condition_converter/helpers.rs drops the range arm from field_condition_index's legacy match and deletes get_range_checkers / get_float_range_checkers / get_datetime_range_checkers (3 helpers, ~75 lines). Six unused imports go with them. Behavioural note: The legacy helpers were stricter than `filter` — only IntIndex/FloatIndex served Float ranges, only DatetimeIndex served DateTime ranges; UuidIndex returned None. `filter` already handles every numeric variant uniformly via T::from_f64 / T::from_u128, so there was a pre-existing inconsistency between filter (primary- clause iteration) and condition_checker (post-filter validation). This PR makes condition_checker match filter — every NumericIndex variant can now serve any RangeInterface. The schema layer normally prevents cross-type range queries from reaching here. Stacked on #8980 (NullIndex) → #8979 (GeoMapIndex) → #8978 (foundation) → dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate MapIndex into condition_checker (#8982) * refactor(field_index): migrate MapIndex condition checking into condition_checker Sub-plan PR 4 of the condition-checker-migration. MapIndex<K> now serves Match::Value / Match::Any / Match::Except conditions directly through PayloadFieldIndexRead::condition_checker, one override per K (MapIndex<str>, MapIndex<UuidIntType>, MapIndex<IntPayloadType>). Changes: 1. Three impl blocks gain condition_checker overrides — one per K. Each uses an explicit FieldCondition destructure (no `..`) and an exhaustive `Match` variant match (no `_ => None`); a new variant in FieldCondition, Match, ValueVariants, or AnyVariants forces a compile error in every relevant place. INDEXSET_ITER_THRESHOLD small/large branching preserved. 2. read_view/condition_converter/match_converter.rs sheds the Map arms. get_match_value_checker keeps (Bool, BoolIndex) plus the explicit 30-tuple None list. get_match_any_checker becomes an explicit 22-tuple None match (no Bool/FullText/Numeric/Geo/Null variant ever served Any). get_match_except_checker reduces to the unconditional values_count > 0 fallback — when no typed index handles Except (via condition_checker), match any point with at least one value, since the value can't be in a type-mismatched list. Unused IndexSet/Uuid/INDEXSET_ITER_THRESHOLD imports drop. Stacked on #8981 (NumericIndex) → #8980 (NullIndex) → #8979 (GeoMapIndex) → #8978 (foundation) → dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate BoolIndex into condition_checker (#8983) * refactor(field_index): migrate BoolIndex condition checking into condition_checker Sub-plan PR 5 of the condition-checker-migration. BoolIndex now serves `Match::Value(Bool)` directly through PayloadFieldIndexRead::condition_checker. Changes: 1. impl PayloadFieldIndexRead for BoolIndex gains a condition_checker override. Explicit FieldCondition destructure (no `..`) plus an exhaustive `Match` variant match (no `_ => None`). Match::Any / Match::Except over booleans return None — matches legacy match_converter behaviour. 2. read_view/condition_converter/match_converter.rs's get_match_value_checker drops the `(Bool, BoolIndex)` arm; the function now returns None unconditionally for every (value, index) pair, with the explicit 33-tuple list preserved so adding a new ValueVariants or FieldIndex variant forces a compile error. Stacked on #8982 (MapIndex) → #8981 → #8980 → #8979 → #8978 → dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate FullTextIndex into condition_checker (#8984) * refactor(field_index): migrate FullTextIndex condition checking into condition_checker Sub-plan PR 6 of the condition-checker-migration. FullTextIndex now serves Match::Text / Match::TextAny / Match::Phrase directly through PayloadFieldIndexRead::condition_checker. Changes: 1. impl PayloadFieldIndexRead for FullTextIndex gains a condition_checker override. Explicit FieldCondition destructure (no `..`) plus exhaustive Match variant matching (no `_ => None`). Match::Value / Match::Any / Match::Except all return None explicitly. Body copies parse_text_query / parse_phrase_query / parse_text_any_query dispatch and the check_match closure from the legacy helper, preserving the FIXME-marked error swallowing. 2. read_view/condition_converter/match_converter.rs loses get_match_text_checker and the TextQueryType enum (both unused after this PR). get_match_checkers' Text/TextAny/Phrase arms collapse to a single explicit `None` arm. Stacked on #8983 (BoolIndex) → #8982 (MapIndex) → #8981 → #8980 → Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): remove legacy condition_checker dispatch helpers (#8985) * refactor(field_index): remove legacy condition_checker dispatch helpers Final PR (7) of the condition-checker-migration sub-plan. With every typed FieldIndex variant now serving its conditions via PayloadFieldIndexRead::condition_checker (PRs 1-6), the wrapper `field_condition_index` and the legacy match dispatch are no longer needed. Changes: 1. read_view/condition_converter/mod.rs's three call sites (Condition::Field, Condition::IsEmpty, Condition::IsNull) call `index.condition_checker(...)` directly instead of going through the `field_condition_index` wrapper. 2. helpers.rs (49 lines) and match_converter.rs (119 lines) deleted — `field_condition_index`, `get_match_checkers`, `get_match_value_checker`, `get_match_any_checker`, `get_match_except_checker` are all gone. 3. The directory `condition_converter/` collapses to a flat file `condition_converter.rs` (mod.rs renamed via git). Behavioural note: the legacy `Match::Except` `values_count > 0` fallback for type-mismatched Except queries is no longer applied — those now fall through to the payload check via `check_field_condition`. Schema-level type checking prevents cross-type Match::Except queries from reaching here in practice, so the fallback was only useful for degenerate inputs; payload check is semantically equivalent. Audit grep on `lib/segment/src/`: $ grep -rn "field_condition_index\|get_match_checkers\|...\|TextQueryType" (no matches) All legacy helper machinery is gone. Stacked on #8984 (FullTextIndex) → #8983 → #8982 → #8981 → #8980 → Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): migrate indexed_variable_retriever into FieldIndexRead (#8986) * refactor(field_index): migrate indexed_variable_retriever into FieldIndexRead Follow-up to the condition-checker-migration sub-plan. The read view's `value_retriever/helpers.rs` previously destructured `FieldIndex` variants inside `indexed_variable_retriever` to build per-point value extractors. This PR turns that into a `value_retriever` trait method on `FieldIndexRead`, so the helper now depends only on the trait — no variant destructuring outside `FieldIndex`'s own impl. Changes: 1. FieldIndexRead gains `value_retriever(&self, hw_counter)`. Lives on the enum-level trait (rather than `PayloadFieldIndexRead`) because the value→Value conversion depends on the payload type `U` in `NumericIndex<T, U>`, and only `FieldIndex` knows the `(T, U)` pairing. 2. impl FieldIndexRead for FieldIndex implements value_retriever with an 11-arm variant match. Bodies copied verbatim from the old `indexed_variable_retriever`. FullTextIndex and NullIndex return None (caller falls back to payload). 3. read_view/value_retriever/helpers.rs: - `variable_retriever` now calls `index.value_retriever(hw_counter)` directly via trait dispatch - `indexed_variable_retriever` deleted - Unused imports dropped (Number, DateTimePayloadType, UuidPayloadType, MultiValue) Helpers.rs shrinks from 189 lines to 73 lines (the test module stays). Total: +134 / -122 across 3 files, but the structural change is moving the dispatch into the trait. Stacked on #8985 (condition-checker-cleanup) → #8984 → … → #8978 → dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): push value_retriever conversion logic into typed indexes `FieldIndex::value_retriever` previously inlined the per-variant value→JSON `Value` conversion (Number, String, DateTime, UUID, Bool, serde_json::to_value for geo) as a 100-line variant match. Move each variant's conversion into an inherent `value_retriever` method on the corresponding typed index; `FieldIndex` is left with a thin 11-arm dispatch that just delegates. Changes: 1. NumericIndex<T, U> gains 4 per-(T, U) inherent value_retriever impls (Int/Datetime/Float/Uuid). The conversion is U-specific (timestamp formatting, UUID string, Number::from_f64 etc.), so each (T, U) pair has its own impl block. 2. MapIndex<K> gains 3 per-K inherent value_retriever impls (Keyword/Int/Uuid). 3. GeoMapIndex and BoolIndex each gain a single inherent value_retriever. 4. FieldIndex::value_retriever collapses to a 11-arm match that delegates to the typed index's inherent method. FullTextIndex and NullIndex still return None at the FieldIndex level — they don't expose value retrieval and the caller falls back to payload. 5. field_index.rs sheds the Number, MultiValue and Box::new noise — about 100 lines smaller. Same shape as how `wipe`, `add_point`, `get_telemetry_data` already delegate from FieldIndex to typed indexes. Adding a new FieldIndex variant now forces compile errors in (a) the dispatch in FieldIndex and (b) the new typed index's inherent impl — both correct places. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(field_index): push BoolIndex/NullIndex condition_checker into storage variants The `BoolIndex` and `NullIndex` wrappers were special-cased among typed indexes: their `PayloadFieldIndexRead::condition_checker` impls did the variant-specific work directly on `self` (matching `Match::Value(Bool)`, `is_empty`, `is_null`) and called `self.check_values_any` / `self.values_is_empty` / `self.values_is_null`, each of which dispatched `match self { Mmap(...) | Immutable(...) }` per point. The inner storage variants (`Immutable/MutableBoolIndex`, `Immutable/MutableNullIndex`) implemented `PayloadFieldIndexRead` solely for `filter` / `count_indexed_points` / `estimate_cardinality` / `for_each_payload_block` — their `condition_checker` was unreachable behind the trait's default `None` body. Removing the trait default surfaced this asymmetry. Rather than stub the inner-type `condition_checker` as dead `None`, push the real logic down to where the storage variants live — same shape as `filter` already uses: inner: explicit FieldCondition destructure + exhaustive Match arms; per-point closure captures `&'a self` of the inner type directly. outer: 2-arm `match self { ... => inner.condition_checker(...) }`. Result: per-point closure body no longer hits a `match self { ... }` on the wrapper. The wrapper dispatch happens once during setup when the wrapper picks which inner closure to return. Matches the pattern the other 5 trait methods (filter/count/estimate/for_each_block) on these wrappers already use. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * CI fix * upd docstring --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
144fb01a09 |
fix: notify pending consensus ops on snapshot apply (#8990)
When an `AddPeer` / `RemovePeer` / `UpdatePeerMetadata` /
`UpdateClusterMetadata` operation is proposed locally and then committed
remotely, the resulting log entry can be delivered back to us as part of
a raft snapshot rather than as a regular log entry. In that case the
operation never flows through `apply_conf_change_entry` /
`apply_normal_entry`, so the awaiter registered by
`propose_consensus_op_with_await` was never woken and timed out after
10s — manifesting as flaky failures of `test_peer_snapshot_bootstrap`
("Failed to add peer: ... Waiting for consensus operation commit failed").
Resolve awaiters in `apply_snapshot` whenever their effect is directly
observable in the new persistent state.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Andrey VasnetsovandClaude Opus 4.7
|
b68435bb4d |
refactor: relocate read-view-exclusive helpers under read_view/ (#8977)
The helpers in query_optimization/condition_converter{,/match_converter}.rs
and the bulk of query_optimization/rescore_formula/value_retriever.rs were
only ever called from struct_payload_index/read_view/. Move them next to
their callers.
The trait split work (#8975, #8976) showed why these helpers can't migrate
to a uniform FieldIndexRead bound: every branch in field_condition_index,
match_converter, get_range_*, get_geo_*, indexed_variable_retriever
destructures FieldIndex variants and reaches into variant-specific
predicates on the underlying typed indexes (MapIndex::check_values_any,
FullTextIndex::parse_*_query, NumericIndex::get_values, …). The variant
match is the whole point of these helpers; co-locating them with the read
view is the right shape.
New layout:
- read_view/condition_converter/{mod.rs, helpers.rs, match_converter.rs}
(was a flat read_view/condition_converter.rs + scattered query_optimization
files)
- read_view/value_retriever/{mod.rs, helpers.rs}
(was a flat read_view/value_retriever.rs + query_optimization/rescore_formula/
value_retriever.rs)
VariableRetrieverFn type alias stays in query_optimization/rescore_formula/
value_retriever.rs (now a 6-line file): formula_scorer.rs in the same module
uses it as a struct field, so the type belongs there. Only the helper
functions and their tests move.
Visibility tightened where natural: cross-file entry points are
pub(super), purely-local helpers are private, public-API surface is
unchanged.
No behavioural change. cargo test -p segment: 666 unit + 120 integration
tests pass.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Andrey VasnetsovandClaude Opus 4.7
|
869af1303a |
refactor: implement FieldIndexRead for FieldIndex (#8976)
* refactor: implement FieldIndexRead for FieldIndex
Move the 10 read-only inherent methods on FieldIndex (special_check_condition,
count_indexed_points, filter, estimate_cardinality, for_each_payload_block,
get_telemetry_data, values_count, values_is_empty, as_numeric, as_facet_index)
into impl FieldIndexRead for FieldIndex. Bodies are unchanged.
Write/lifecycle methods (add_point, remove_point, wipe, flusher, files,
immutable_files, ram_usage_bytes, is_on_disk, populate, clear_cache,
get_full_index_type) stay inherent on FieldIndex.
Call sites that hold &FieldIndex now resolve through the trait — added
`use ... FieldIndexRead;` imports where the compiler asked
(read_view/{payload_index_read,filtering}.rs, query_optimization/
condition_converter.rs and match_converter.rs, payload_storage/
query_checker.rs, two integration tests, full_text_index tests module).
StructPayloadIndex::get_facet_index keeps its concrete
OperationResult<FacetIndexEnum<'_>> return: the trait method as_facet_index
returns Option<impl FacetIndex + '_> (RPITIT) which can't be downcast back
to FacetIndexEnum, so the function inlines the variant match. Same shape as
before, just expressed locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: remove unused StructPayloadIndex::get_facet_index
Confirmed dead code — the function had no callers anywhere in the workspace.
It was preserved across recent refactors (most recently #8967) but was never
referenced. Removing it also drops the now-unused JsonPath and FacetIndexEnum
imports in struct_payload_index/mod.rs.
This obsoletes the inline variant match introduced in the previous commit:
that match existed solely to keep get_facet_index returning the concrete
FacetIndexEnum<'_> when as_facet_index moved to the FieldIndexRead trait
with an opaque return. With get_facet_index gone, no workaround is needed.
The OperationError::MissingMapIndexForFacet variant stays — it is still used
by segment::read_view::facet and lib/collection/src/operations/types.rs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
Andrey VasnetsovandClaude Opus 4.7
|
431e217b03 |
refactor: introduce FieldIndexRead trait (#8975)
Mirror the PayloadFieldIndex / PayloadFieldIndexRead split (#8966) one layer up at the FieldIndex enum level. FieldIndexRead names the uniform read-only surface (filter, estimate_cardinality, count_indexed_points, for_each_payload_block, get_telemetry_data, values_count, values_is_empty, special_check_condition, as_numeric, as_facet_index) — the methods that StructPayloadIndexReadView and its helpers reach through IndexesMap. Trait declaration only. No impl, no callers in this PR — this is purely additive. Follow-up PRs will impl it on FieldIndex, then migrate non-destructuring helpers to a FieldIndexRead bound. as_numeric and as_facet_index use return-position impl Trait (impl NumericFieldIndexRead + '_, impl FacetIndex + '_), matching how PayloadIndexRead exposes the same shapes. The trait is therefore not object-safe; consumers will use a generic bound rather than &dyn FieldIndexRead. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
b91e7bceda |
test: stabilize test_payload_strict_mode_upsert_no_local_shard (#8973)
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> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
bd4aab364e |
test: fix flaky test_cluster_metadata by polling for consensus (#8971)
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> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
22b3cb9a22 |
refactor(index): drop PayloadIndex: PayloadIndexRead super-trait (#8969)
* refactor(index): drop PayloadIndex: PayloadIndexRead super-trait `PayloadIndex` now only declares the mutating surface; reads live on the sibling `PayloadIndexRead` trait. The previous super-trait relationship forced any type that implemented `PayloadIndex` to also implement `PayloadIndexRead`, blocking a future `PayloadIndexRead`-only view that doesn't (and shouldn't) own the writable index machinery. No behavioural change. Audit before committing showed no generic bound site on `PayloadIndex` exists in the workspace, and every caller that uses read methods already imports `PayloadIndexRead` explicitly (the trait was already used as a generic bound on `SegmentReadView`'s `TPayloadIndex` parameter and on `iter_filtered_points`). The full workspace builds clean and all segment / storage / collection tests pass without any consumer update. Doc comment on `PayloadIndex` updated to point readers at `PayloadIndexRead` for the read surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(index): introduce StructPayloadIndexReadView<P, I, V> (#8970) Move the read surface of `StructPayloadIndex` onto a new borrowed view struct generic over `<P: PayloadStorageRead, I: IdTrackerRead, V: VectorStorageRead>`. The view holds exactly the fields that `PayloadIndexRead` requires -- no more, no less: pub struct StructPayloadIndexReadView<'a, P, I, V> { payload: &'a Arc<AtomicRefCell<P>>, id_tracker: &'a I, vector_storages: &'a HashMap<VectorNameBuf, Arc<AtomicRefCell<V>>>, field_indexes: &'a IndexesMap, config: &'a PayloadConfig, visited_pool: &'a VisitedPool, } `StructPayloadIndex` now exposes a `with_view(|v| ...)` accessor that borrows `id_tracker` once at the top and constructs the view for the closure scope. All read-method bodies move onto the view, which is the sole `PayloadIndexRead` implementor for this index. Why three generics ================== - `P: PayloadStorageRead` -- already generic via PR #8968. - `I: IdTrackerRead` -- direct method calls; held as `&I` (not `&Arc<AtomicRefCell<I>>`) because the cell is collapsed at the `with_view` boundary, saving a per-method `borrow()` atomic op. `dyn IdTrackerRead` does not satisfy `I: IdTrackerRead` bounds in Rust without an explicit blanket impl, so generic is the only consistent option here. - `V: VectorStorageRead` -- the only access site is `available_vector_count()` for the `HasVector` cardinality branch (`condition_cardinality` in `read_view/filtering.rs`). Why `payload` keeps the `Arc` ============================= `PayloadProvider<P>::new(...)` (introduced in PR #8968) takes `Arc<AtomicRefCell<P>>` so that the returned `FormulaScorer<'q>` / `Box<dyn FilterContext + 'a>` can outlive the caller frame. The view therefore holds `&'a Arc<AtomicRefCell<P>>` (asymmetric vs the bare `&I` for `id_tracker`). Switching to a borrow-based provider would require reworking `formula_scorer` / `filter_context` to callback style; deferred to a follow-up if needed. What does NOT move ================== - `build_field_indexes` and `clear_index_for_point` stay on `StructPayloadIndex`. `build_field_indexes` is read-shaped but only has write-side callers, and pulls in the `selector` machinery which uses `path` + `storage_type`. Keeping it on the writable struct means `path` and `is_appendable` do not need to leak into the view. - The `selector` / `selector_with_type` helpers stay on the writable struct for the same reason. - The free helpers in `query_optimization/condition_converter.rs` (range / geo / null / is-empty checkers) stay where they are; their visibility is bumped from `fn` to `pub(in crate::index)` so the view can still call them. Module layout ============= lib/segment/src/index/struct_payload_index/ mod.rs # owning struct + with_view build.rs # write-side build coordination payload_index.rs # impl PayloadIndex (mutating only) tests.rs read_view/ mod.rs # view struct + module wiring payload_index_read.rs # impl PayloadIndexRead for view filtering.rs # struct_filtered_context, condition_cardinality, query_field, estimate_field_condition condition_converter.rs # impl block from query_optimization/ optimizer.rs # impl block from query_optimization/ value_retriever.rs # impl block from query_optimization/ tests.rs # smoke test that builds the view directly Consumer migration ================== - `Segment::with_view` nests the new `StructPayloadIndex::with_view` inside it; `SegmentReadViewFor<'s>` uses the view as its `TPayloadIndex` parameter. - HNSW (`hnsw.rs`), sparse (`sparse_vector_index.rs`), plain (`plain_vector_index.rs`) call sites wrap their read-method calls in `payload_index.borrow().with_view(|v| ...)`. - `Segment::get_indexed_fields`, `update_all_field_indices`, and `SegmentBuilder::build` switch to `with_view` for `indexed_fields()` / `get_payload_sequential()`. - Integration tests and benches similarly migrate. - `set_payload` (still on `PayloadIndex` write impl) inlines its former `self.get_payload(...)` call as `self.payload.borrow().get(...)` to avoid going through `with_view` from a `&mut self` write path. Smoke test (`read_view/tests.rs`) constructs the view directly over `InMemoryPayloadStorage` + `InMemoryIdTracker` + an empty vector-storage map, and exercises `indexed_fields()`, `query_points()`, and `available_point_count()` -- proving the view is genuinely decoupled from `StructPayloadIndex`. This is the abstraction PR 4 will use to wire a read-only segment. Verified ======== - `cargo build --workspace --tests --benches` -- green - `cargo test -p segment --lib` -- 666 passed (665 + 1 new smoke test), 0 failed - `cargo test -p segment --tests` -- 120 integration tests pass - `cargo test -p storage --lib` -- 44 passed - `cargo test -p collection --lib` -- 197 passed - `cargo clippy -p segment --tests --benches` -- clean Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
4d17cd56b7 |
refactor(payload_storage): generalize PayloadProvider over PayloadStorageRead (#8968)
Add `payload_ref(point_offset, hw_counter) -> OwnedPayloadRef<'_>` to the `PayloadStorageRead` trait so payload retrieval no longer needs to know whether the underlying storage is in-memory (zero-copy borrow) or mmap/on-disk (materialised clone). In-memory storage returns `OwnedPayloadRef::Ref(...)` to preserve the existing zero-copy hot path; on-disk storages return `OwnedPayloadRef::Owned(...)`. `PayloadProvider` is now generic over `P: PayloadStorageRead` (`Arc<AtomicRefCell<P>>`) instead of being hard-coded to `PayloadStorageEnum`. The previous `match`-on-enum dispatch in `PayloadProvider::with_payload` is gone; dispatch happens through `PayloadStorageRead::payload_ref`. Manual `Clone` impl avoids imposing a `P: Clone` bound (cloning the provider is just an `Arc` bump). The `empty_payload` fallback field is removed -- `payload_ref` always returns a usable ref. `<P: PayloadStorageRead + 'a>` is threaded through the consuming signatures in `query_optimization/`: `optimize_filter`, `convert_conditions`, `optimize_should/min_should/must/must_not`, `condition_converter`, `variable_retriever`, `payload_variable_retriever`. The `+ 'a` bound is required because the returned `OptimizedFilter<'a>` / `ConditionCheckerFn<'a>` / `VariableRetrieverFn<'a>` contain boxed closures that capture the provider. The cascade stops at the box: `StructFilterContext<'a>`, `FormulaScorer<'a>`, and the rest of the search machinery are unaffected. Construction sites (`StructPayloadIndex::formula_scorer`, `struct_filtered_context`, `retrievers_map`) need no source-level change -- type inference picks `P = PayloadStorageEnum` from the `Arc<AtomicRefCell<PayloadStorageEnum>>` they pass in. The panic-on-corruption policy that lived in the previous Mmap branch is preserved verbatim and now applies uniformly across storage backends. This unblocks building a `PayloadStorageRead`-generic read-only view of `StructPayloadIndex` (follow-up work). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
2c13e51217 |
refactor split of struct_payload_index.rs (#8967)
* refactor split of struct_payload_index.rs * review fixes |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
e6e2246c9b |
refactor: split PayloadFieldIndex into PayloadFieldIndex + PayloadFieldIndexRead (#8966)
Mirror the IdTracker / IdTrackerRead split. PayloadFieldIndexRead holds the read-only query surface (count_indexed_points, filter, estimate_cardinality, for_each_payload_block); PayloadFieldIndex: PayloadFieldIndexRead retains storage-lifecycle methods (wipe, flusher, files, immutable_files). All 11 impls updated (numeric_index, map_index x3, null_index x3, bool_index x3, full_text_index, geo_index). Test imports adjusted where they use the read-side methods. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
a8aa74eb5c |
refactor: split field_index_base.rs into a module (#8965)
Break up the 778-line field_index_base.rs into focused submodules under field_index_base/ (payload_field_index, value_indexer, field_index, builder) and relocate NumericFieldIndex / NumericFieldIndexRead into the numeric_index module where they naturally belong. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
26eb16fd6a |
make storage generic in ReadOnlySparseVectorStorage (#8964)
* make storage generic in ReadOnlySparseVectorStorage * fix tests |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
006c449ffd |
test: bump wait_for_peer_online timeout for recovery-with-user-transfers test (#8963)
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> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
b423de05a1 |
Move element type from UniversalRead trait to method generics (#8955)
* Move element type from UniversalRead trait to method generics Lifts the `T` parameter off `trait UniversalRead<T>` (and the matching `UniversalWrite<T>`) and onto the read/write methods themselves. The `ReadPipeline` associated type becomes a GAT over `T`. With per-method generics, callers that need to read several element types from one storage just write `S: UniversalRead` instead of stacking `UniversalRead<u8> + UniversalRead<Counts> + ...`. Removes the workarounds the old shape required: - `TypedStorage<S, T>` newtype (sole purpose was disambiguating multi-bounds) - `UniversalReadFamily` HKT shim - `StoredGeoMapIndexStorage` four-bound trait alias - `CachedSlice<T>` is now non-generic; `T` moves to `get_range`/`len` No runtime behavior change: alignment in `IoUringRuntime` and `CachedSlice` is preserved because `T` is still known at each call site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restore TypedStorage as a typed-access fail-safe Reintroduces `TypedStorage<S, T>` as a transparent wrapper around `UniversalRead`/`UniversalWrite` storage that fixes the element type to `T`. With per-method generics on the underlying traits, callers can otherwise read or write any `T` from the same handle; this wrapper binds it at the type level so accidental cross-type access fails to compile. The wrapper exposes inherent typed methods (`read::<P>`, `read_iter`, `write`, `len`, …) that delegate to the inner storage with `T` fixed. It does not implement `UniversalRead`/`UniversalWrite` itself — those are intentionally avoided to prevent the typed binding from being bypassed via the generic trait methods. Restores the wrapping at the previous call sites: `StoredStruct`'s inner storage, `ImmutableIdTracker`'s version mmap, the geo and numeric index storages, the chunked-vectors chunks, and the immutable dense vector storage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Silence clippy::len_without_is_empty on TypedStorage `TypedStorage::len` returns `Result<u64>` (a fallible byte length from the underlying storage), so an `is_empty` companion would also be fallible and offer nothing over `len()? == 0`. Suppress the lint at the impl block, matching how the underlying `UniversalRead::len` is already exempted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fmt --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
0c6e5bbfce |
test: fix flaky test_consensus_snapshot_create_collection voter race (#8951)
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> |
||
|
Andrey Vasnetsov
|
a670f2d7c1 |
Tolerate consensus apply timeouts in test_rejoin_cluster create loops (#8950)
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. |
||
|
Andrey Vasnetsov
|
86e3c123fe | add skills link into welcome message (#8932) | ||
|
Andrey VasnetsovandClaude Opus 4.7
|
ce6a493a37 |
Add ordering parameter to gRPC create/delete vector name (#8926)
Match the REST API by adding an optional `WriteOrdering` field to `CreateVectorNameRequest` and `DeleteVectorNameRequest`, and propagate it through the tonic handlers and remote-shard forwarding paths. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
675bde342a |
Read-only ID tracker (#8887)
* wip: read-only id tracker for mutable data * read-only id tracker |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
df471ed62a |
refactor(segment): split ChunkedVectors into read-only core + write wrapper (#8915)
Extract the read-side API into ChunkedVectorsRead<T, S: UniversalRead<T>> and rebuild the existing ChunkedVectors<T, S: UniversalWrite<T>> on top of it via composition + Deref. Lets read-only consumers use the storage without pulling in UniversalWrite, and avoids method duplication. Reorganizes the file into a chunked_vectors/ module: config (constants, ChunkedVectorsConfig, Status), chunks (read_chunks/create_chunk helpers), read (ChunkedVectorsRead), write (ChunkedVectors wrapper). read_chunks now takes a writeable flag so both open paths share it. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
ac69def67b |
test: fix flaky test_shard_transfer_includes_deferred_points[snapshot] (#8860)
* 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> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
893a896e06 |
refactor(segment): split immutable_id_tracker into a module (#8888)
Mirrors the layout of mutable_id_tracker: storage helpers for the mappings, versions, and deleted bitslice files live in their own submodules, leaving mod.rs focused on the ImmutableIdTracker type and its trait impls. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
6b08e54b43 |
feat(segment): add ReadOnlyPayloadStorage backed by GridstoreReader (#8886)
Introduce a read-only payload storage that wraps `GridstoreReader<Payload>` and implements `PayloadStorageRead`. Also rename the parameter on `PayloadStorageRead::get`/`get_sequential` from `point_id` to `point_offset` for consistency with the underlying storage API. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 4.7
|
4f31635b34 |
refactor(segment): migrate info/size_info/telemetry to SegmentReadView (step 11) (#8882)
* refactor(index): extend VectorIndexRead and PayloadIndexRead for telemetry/info Two trait extensions (no defaults — every implementor must opt in): * \`VectorIndexRead::is_index\` — distinguishes a real index from a plain full-scan one. Used by reporting code. Moved out of inherent \`VectorIndexEnum::is_index\` into the trait. Explicit impls on Plain (false), HNSW (true), Sparse* (true). * \`PayloadIndexRead::get_telemetry_data\` — per-field-index telemetry. Moved out of inherent \`StructPayloadIndex::get_telemetry_data\` into the trait impl. \`PlainPayloadIndex\` returns an empty Vec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(segment): migrate info/size_info/telemetry to SegmentReadView Step 11 of the SegmentReadView migration — the final logical step. New \`read_view/info.rs\` exposes builder methods: * \`build_size_info(uuid, segment_type, is_appendable)\` * \`build_info(uuid, segment_type, is_appendable)\` — same plus \`index_schema\` * \`build_telemetry(uuid, segment_type, is_appendable, config, detail)\` The trivial segment-level fields (\`uuid\`, \`segment_type\`, \`is_appendable\`, \`config\`) are passed in by the caller — they stay direct on each segment-type rather than going through the view. Everything else (vector data breakdown, payloads size, deferred counts, vector-index telemetry, payload-field telemetry, …) is computed once inside the view through the read traits. \`Segment::size_info\`, \`info\`, and \`get_telemetry_data\` collapse to one-line \`with_view\` delegators that pass in the trivial fields. Cleanup: \`Segment::deferred_deleted_count\` is now unused (the view has its own equivalent helper); deleted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
c9459cc241 |
chore(index): drop unused imports in formula_scorer
Leftovers from moving \`StructPayloadIndex::formula_scorer\` out of this file. Caught by clippy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
e1c851115a |
refactor(index): drop FormulaScorerRead, return concrete FormulaScorer
\`FormulaScorer<'a>\` is already self-contained — it holds the parsed formula, prefetch scores, retrievers and condition checkers, all owned or borrowed independently of any payload index. Wrapping it in a trait adds nothing (a future ReadOnlySegment can construct one too). * Remove the \`FormulaScorerRead\` trait. \`score(point_id)\` goes back to being an inherent method on \`FormulaScorer\`. * \`PayloadIndexRead::formula_scorer\` returns \`OperationResult<FormulaScorer<'q>>\` directly. * \`StructPayloadIndex\` and \`PlainPayloadIndex\` updated to match. * \`PlainPayloadIndex\` no longer needs the turbofish placeholder — \`Err(...)\` is enough. * View's \`formula_rescore.rs\` drops the \`FormulaScorerRead\` import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
f5d4c7d576 |
refactor(segment): migrate formula rescore to SegmentReadView
Step 10 of the SegmentReadView migration. Move \`segment/formula_rescore.rs\` → \`read_view/formula_rescore.rs\`: * \`do_rescore_with_formula\` (private helper). * \`rescore_with_formula\` (\`ReadSegmentEntry\` orchestrator). Both now use the trait-method \`PayloadIndexRead::formula_scorer\` (added in the prior commit) and \`IdTrackerRead::internal_id\` instead of inherent calls. \`Segment::rescore_with_formula\` collapses to a single \`with_view(|v| v.rescore_with_formula(...))\` delegator. The legacy \`segment/formula_rescore.rs\` is deleted; \`mod formula_rescore;\` removed from \`segment/mod.rs\`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
423fc53218 |
refactor(index): abstract formula_scorer behind FormulaScorerRead trait
A read-only payload index implementation will have its own concrete formula scorer type, so PayloadIndexRead can't return the appendable \`FormulaScorer<'a>\` directly. * New \`FormulaScorerRead\` trait next to \`FormulaScorer\` exposing only what the rescore code path consumes (\`score(point_id)\`). Implemented for \`FormulaScorer<'_>\` by moving its inherent \`score\` into the trait impl. * \`PayloadIndexRead::formula_scorer\` returns \`OperationResult<impl FormulaScorerRead + 'q>\` (RPITIT). * The inherent \`StructPayloadIndex::formula_scorer\` (which lived in \`formula_scorer.rs\`) is moved into the trait impl block in \`struct_payload_index.rs\`, with the body delegating to a new \`FormulaScorer::new\` constructor (fields stay private). * \`PlainPayloadIndex\` always returns \`Err::<FormulaScorer<'q>, _>(...)\` — formula scoring is not supported there. The turbofish supplies the placeholder type tag. * Re-export \`FormulaScorer\` and \`FormulaScorerRead\` from \`rescore_formula::mod\`. \`retrievers_map\` bumped from \`pub(super)\` to \`pub(crate)\` so the trait impl can call it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
2f461d6a0d |
refactor(index): drop default impl of fill_idf_statistics, force per-impl
A new index implementation should never silently skip IDF wiring. Make \`VectorIndexRead::fill_idf_statistics\` a required method (no default) and provide explicit no-op impls for the dense indexes: * \`PlainVectorIndex\` — no-op. * \`HNSWIndex\` — no-op. \`SparseVectorIndex\` keeps the real implementation: moved from a sibling \`pub fn\` (inherent) into the \`VectorIndexRead\` trait impl block, so there is now exactly one definition. \`VectorIndexEnum\` already overrides per-variant from the previous commit; nothing changes there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
6937584273 |
refactor(segment): migrate search, retrieve, and fill_query_context to SegmentReadView
Step 9 of the SegmentReadView migration. New \`read_view/search.rs\` hosts: * \`retrieve\` — full body (deferred filtering, vector enrichment, payload attachment). * \`process_search_result\` — converts internal \`ScoredPointOffset\`s into user-facing \`ScoredPoint\`s; calls the new \`view.retrieve\`. * \`search_batch\` — \`ReadSegmentEntry\` orchestrator. * \`fill_query_context\` — uses the trait \`VectorIndexRead::fill_idf_statistics\` (added in the prior commit) and the existing \`indexed_vector_count\`. The \`Segment\`-side trait method bodies for \`search_batch\`, \`retrieve\`, and \`fill_query_context\` collapse to single \`with_view\` delegators. \`rescore_with_formula\` (whose body still uses Segment-side \`do_rescore_with_formula\`, migrated in step 11) routes its \`process_search_result\` call through \`with_view\`. \`segment/search.rs\` is trimmed to just the \`#[cfg(feature = "testing")] pub fn search\` helper, which calls \`search_batch\` (now a \`with_view\` delegator) — no migration needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
6ba1bc9f26 |
refactor(index): move VectorIndexEnum::fill_idf_statistics to trait, drop indexed_vectors
Adds \`fill_idf_statistics\` to the \`VectorIndexRead\` trait with a no-op default (only sparse-vector indexes track IDF). \`VectorIndexEnum\` overrides it with the per-variant dispatch that previously lived as an inherent method. The inherent \`indexed_vectors\` is removed: it returned the same value as the trait method \`indexed_vector_count\` for every variant (sparse's \`indexed_vector_count\` is itself \`inverted_index.vector_count()\`). The remaining caller in \`fill_query_context\` will use \`indexed_vector_count\` after the next commit moves it to the view. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
fd7251b025 |
refactor(index): drop FacetIndexRead, reuse FacetIndex on FacetIndexEnum
The introduced \`FacetIndexRead\` trait was almost identical to the existing \`FacetIndex\` trait — the only difference was the augmented \`for_each_value\` signature on \`FacetIndexEnum\` that took \`hw_counter\` and \`deferred_internal_id\` to skip values whose only points are deferred. * Move that "skip deferred" logic onto \`FacetIndex\` itself as a default method \`for_each_visible_value\` — implemented in terms of the existing \`for_each_value\` and \`for_each_value_map\`. All \`FacetIndex\` impls (MapIndex, BoolIndex, FacetIndexEnum) get it for free. * Make \`FacetIndexEnum\` impl \`FacetIndex\` directly (delegating to the inner index's \`FacetIndex\` impl). The augmented inherent \`for_each_value\` is gone. * \`PayloadIndexRead::facet_index_for\` now returns \`Option<impl FacetIndex + '_>\` (no separate \`FacetIndexRead\` trait). * View facet code uses \`for_each_visible_value\` instead of the old augmented \`for_each_value\`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
f0426a271c |
refactor(segment): migrate facets to SegmentReadView
Step 8 of the SegmentReadView migration. Move \`segment/facet.rs\` → \`read_view/facet.rs\`: * \`approximate_facet\` * \`facet_values\` Both now go through the trait-method \`facet_index_for\` (added in the prior commit) and the existing trait-method \`filter_context\` instead of the previous inherent \`get_facet_index\` and \`struct_filtered_context\` calls. \`Segment::unique_values\` and \`Segment::facet\` (the \`ReadSegmentEntry\` trait orchestrators) collapse to single \`with_view\` delegators. The legacy \`segment/facet.rs\` is deleted; \`mod facet;\` removed from \`segment/mod.rs\`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
ad4167e70d |
refactor(index): abstract facet index access behind FacetIndexRead trait
Same shape as the numeric_index_for change in the previous PR: a
read-only segment will have its own per-key facet-index type, so
PayloadIndexRead can't return the concrete \`FacetIndexEnum<'a>\`.
* New \`FacetIndexRead\` trait in \`field_index/facet_index.rs\` mirrors
the inherent methods of \`FacetIndexEnum\` (\`for_points_values\`,
\`for_each_value\`, \`for_each_value_map\`, \`for_each_count_per_value\`).
* Implemented for \`FacetIndexEnum<'a>\` by delegating to the inherent
methods.
* \`PayloadIndexRead::facet_index_for(key) -> Option<impl FacetIndexRead + '_>\`
added (RPITIT, no boxing). Each impl picks its own concrete return
type:
- \`StructPayloadIndex\` returns \`Option<FacetIndexEnum<'_>>\`.
- \`PlainPayloadIndex\` always returns \`None\`; the explicit
\`None::<FacetIndexEnum<'_>>\` turbofish supplies a placeholder
type tag.
* \`FacetIndexRead\` re-exported from \`field_index/mod.rs\` (the
\`facet_index\` submodule itself stays \`pub(super)\`).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
generallandClaude Opus 4.7
|
9f59371891 |
refactor(segment): migrate random sampling to SegmentReadView
Step 7 of the SegmentReadView migration. Move \`segment/sampling.rs\` → \`read_view/sampling.rs\`: * \`read_by_random_id\` * \`filtered_read_by_index_shuffled\` * \`filtered_read_by_random_stream\` * \`read_random_filtered\` orchestrator \`Segment::read_random_filtered\` collapses to a single \`with_view(|v| v.read_random_filtered(...))\` delegator. The legacy \`segment/sampling.rs\` is deleted; \`mod sampling;\` removed from \`segment/mod.rs\`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
238138e1e9 |
refactor(common): use EitherVariant for NumericIndexInner::stream_range
Drop the bespoke 4-arm \`NumericRangeIter\` enum introduced in the prior commit. The existing \`common::either_variant::EitherVariant\` is the same shape and already has \`Iterator\` plus all the standard adapter specializations. Adds a \`DoubleEndedIterator\` impl to \`EitherVariant\` (\`next_back\`, \`nth_back\`, \`rfold\`, \`rfind\`) so it can be used in this position. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
2ba3b9a054 |
refactor(index): replace Box<dyn> with RPITIT in StreamRange and NumericFieldIndexRead
Both \`StreamRange<T>::stream_range\` and the new \`NumericFieldIndexRead\` trait now return \`impl Iterator\` / \`impl DoubleEndedIterator\` directly. No more boxed trait-object allocations in the order-by hot path. Mechanism: * \`NumericIndexInner::stream_range\` has 4-way branching (empty / mutable / immutable / mmap), all with different concrete iterator types. Unified via a new 4-arm \`NumericRangeIter\` enum that delegates \`Iterator\` and \`DoubleEndedIterator\` to its variants. * \`NumericFieldIndex::stream_range\` and \`get_ordering_values\` have 2-way branching (Int / Float). Unified via \`itertools::Either\`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
af4dc8708d |
refactor(index): abstract numeric_index_for return behind NumericFieldIndexRead trait
A read-only payload index implementation will have its own concrete numeric-index type that doesn't fit the appendable \`NumericFieldIndex<'a>\` enum. So PayloadIndexRead can't return that concrete type directly. Introduce \`NumericFieldIndexRead\` trait exposing only what ordered reads consume (\`get_ordering_values\`, \`stream_range\`). Implement it for the existing \`NumericFieldIndex<'a>\` (delegates to the inherent methods + \`StreamRange\`). \`PayloadIndexRead::numeric_index_for\` now returns \`Option<impl NumericFieldIndexRead + '_>\` (RPITIT). Each implementation picks its own concrete return type: - \`StructPayloadIndex\` returns \`Option<NumericFieldIndex<'_>>\`. - \`PlainPayloadIndex\` always returns \`None\` (it has no field indexes); the explicit \`None::<NumericFieldIndex<'_>>\` turbofish just supplies a placeholder type tag — no value is constructed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
6c4a0b1bc8 |
refactor(segment): finish scroll migration — move filtered_read_by_index to view
Step 5 leftover: with PayloadIndexRead.iter_filtered_points now on the trait, filtered_read_by_index can move to the view alongside the other three scroll helpers. * `read_view/scroll.rs` gains `filtered_read_by_index` and the `read_filtered` orchestrator. * `segment/scroll.rs` is deleted entirely; `mod scroll;` removed from `segment/mod.rs`. * `Segment::read_filtered` collapses to a single `with_view(|v| v.read_filtered(...))` delegator. * `scroll_filtering_test.rs` integration test calls `filtered_read_by_index` via `with_view`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
53a0ac2823 |
refactor(segment): migrate ordered reads to SegmentReadView
Step 6 of the SegmentReadView migration. Move `lib/segment/src/segment/order_by.rs` into `read_view/order_by.rs`: * `filtered_read_by_index_ordered` — uses `payload_index.iter_filtered_points` (now on trait, see prior commit) and `numeric_index_for` (also now on trait). * `filtered_read_by_value_stream` — uses `numeric_index_for` and `filter_context` (already on trait). * `read_ordered_filtered` — `ReadSegmentEntry` orchestrator, also moved to the view. `Segment::read_ordered_filtered` collapses to a single `with_view(|v| v.read_ordered_filtered(...))` delegator. The legacy `segment/order_by.rs` file is deleted entirely; `mod order_by;` removed from `segment/mod.rs`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
89d7c1b1c2 |
refactor(index): extend PayloadIndexRead with iter_filtered_points and numeric_index_for
Prerequisite for migrating order_by, sampling, facet, and scroll's `filtered_read_by_index` to `SegmentReadView`. Both methods existed only as inherent methods / public field access on `StructPayloadIndex`; the view holds a generic `&TPI: PayloadIndexRead` reference and so can only call trait methods. * `iter_filtered_points` is now a trait method, generic over `I: IdTrackerRead` for the id_tracker parameter and using `impl Iterator` in return position (RPITIT). Each impl keeps its own zero-cost concrete iterator chain — no boxed dyn-iterator allocation. The trait becomes non-object-safe, which is fine because nothing currently uses `dyn PayloadIndexRead`. * `numeric_index_for(key) -> Option<NumericFieldIndex<'_>>` exposes the per-key numeric-index lookup that ordered reads rely on, replacing direct access to `StructPayloadIndex::field_indexes`. * Implementations on `StructPayloadIndex` (real) and `PlainPayloadIndex` (the latter has no numeric indexes, so `numeric_index_for` returns `None`). * Existing internal callers (hnsw, scroll, sampling, order_by, facet) now pass `&*id_tracker` to fully deref `AtomicRef<IdTrackerEnum>` to `&IdTrackerEnum` for the generic-param inference. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
65d8a85353 |
refactor(segment): migrate scroll helpers (3 of 4) to SegmentReadView
Step 5 of the SegmentReadView migration. New `read_view/scroll.rs` module hosts: * `should_pre_filter` — payload-index cardinality estimation, used by all three scroll-shaped trait methods (read_filtered, read_ordered_filtered, read_random_filtered). * `read_by_id_stream` — streamed enumeration of visible points. * `filtered_read_by_id_stream` — streamed enumeration with a payload-filter context applied. `Segment`-side `read_filtered` collapses to a `with_view` orchestrator (except for the `filtered_read_by_index` branch, see below). `read_ordered_filtered` and `read_random_filtered` now route their `should_pre_filter` calls through `with_view` (their own bodies migrate in steps 6 and 7). `filtered_read_by_index` stays on `Segment` for now: it depends on `StructPayloadIndex::iter_filtered_points`, which is an inherent method that takes the concrete `&IdTrackerEnum` and returns `impl Iterator`. Migrating it cleanly requires extending `PayloadIndexRead` with a trait-object-friendly version of that method, which is the same prerequisite Steps 6/7/8 will need (sampling, order_by, facet all use `iter_filtered_points`). I will do that as a focused pre-step before Step 6. `deferred_internal_id` / `deferred_deleted_count` view helpers bumped from private to `pub(super)` so the new `scroll` module can call them. `scroll_filtering_test.rs` integration test updated to call `filtered_read_by_id_stream` via `with_view`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
8482623620 |
refactor(segment): migrate payload-access methods to SegmentReadView
Step 4 of the SegmentReadView migration. New `read_view/payload.rs` module hosts: * `payload_by_offset` (was `pub(super)` on Segment in `segment_ops.rs`) * `payload` (`ReadSegmentEntry` trait method, real composition: `lookup_internal_id` + `payload_by_offset`) * `estimate_point_count` (`ReadSegmentEntry` trait method, composes filter handling, payload-index cardinality and deferred-point adjustment) The corresponding Segment-side trait methods become `with_view` delegators, and `Segment::payload_by_offset` is deleted. `Segment::lookup_internal_id` (a transitional `with_view` delegator introduced in step 1) is now unused after `vector` and `payload` moved off of it; deleted as well. Tests that called it now go through `with_view`. `get_indexed_fields` stays direct on Segment — single-line `payload_index.borrow().indexed_fields()`, no shared logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
89b5999932 |
refactor(segment): migrate vector-access methods to SegmentReadView
Step 3 of the SegmentReadView migration. Move the vector-access logic onto the view in a new `read_view/vectors.rs` module: * `vector_by_offset`, `vectors_by_offsets` (was `pub(super)` on Segment) * `read_vectors` (was `pub(crate)` on Segment in the now-deleted `segment/vectors.rs`) * `vector` and `available_vectors_size_in_bytes` (`ReadSegmentEntry` trait methods) `Segment::vector` and `Segment::available_vectors_size_in_bytes` collapse to `with_view` delegators. The `retrieve` trait method now calls `view.read_vectors(...)` inside per-name `with_view` closures. Removed entirely: * `segment/vectors.rs` (its only function moved to the view) * `Segment::vector_by_offset` and `Segment::vectors_by_offsets` in `segment_ops.rs` Kept on Segment: * `vector_names` — trivial `vector_data.keys().cloned().collect()`, no shared logic to factor out. * `all_vectors` — invariant `NamedVectors<'_>` lifetime fights the view borrow, and the body is a 6-line iteration over `self.vector` (which is itself now a view delegator). ReadOnlySegment will write the same trivial loop. Tests in `segment/tests/mod.rs` that exercised `segment.vector_by_offset(...)` now go through `segment.with_view(|v| v.vector_by_offset(...))`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
b3978dc7cb |
refactor(segment): extract deferred-point view methods into read_view/deferred.rs
Move the six deferred-point methods (deferred_internal_id / deferred_deleted_count helpers + deferred_point_count, has_deferred_points, point_is_deferred, deferred_point_ids, available_point_count_without_deferred) out of read_view/segment_ops.rs into their own read_view/deferred.rs module. Keeps each read_view file focused on a single concern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
ff9c20c651 |
style(segment): rustfmt fixup for has_deferred_points
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
3120f2100b |
refactor(segment): migrate deferred-point logic to SegmentReadView
Step 2 of the SegmentReadView migration. Move the five composition-style deferred-point methods onto the view: `deferred_point_count`, `has_deferred_points`, `point_is_deferred`, `deferred_point_ids`, `available_point_count_without_deferred`. Each combines the deferred-point status with id-tracker reads, so they are real shared logic — not trivial getters. The Segment-side trait method bodies in `entry.rs` collapse to `self.with_view(|v| v.foo(...))` delegators. `deferred_internal_id` and `deferred_deleted_count` are one-line field accessors on `DeferredPointStatus`, so they stay direct on `Segment`. The view has its own private helpers with the same one-liner shape — duplicated only at the access-path level. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generallandClaude Opus 4.7
|
29d8661cba |
refactor(segment): finalize SegmentReadView shape; migrate read_range and lookup_internal_id
Step 1 of the SegmentReadView migration. * `SegmentReadView` is expanded to its final field set, parameterised over four read-only traits (`IdTrackerRead`, `PayloadIndexRead`, `PayloadStorageRead`, `VectorDataRead`). A `SegmentReadViewFor<'s>` type alias hides the verbosity at call sites for `Segment`. Steps 2-11 only add `impl` methods; the struct shape is now frozen. * `Segment::with_view` borrows the three `AtomicRefCell`-wrapped storages up front and populates every view field; closure bound relaxed `Fn` -> `FnOnce`. * `lookup_internal_id` (Option -> PointIdError conversion) and `read_range` (iterator + take_while) are migrated to the view. The previous Segment-side helpers become thin `with_view` delegators where external callers still rely on them. * Trivial id-tracker getters (`has_point`, `is_empty`, `available_point_count`, `deleted_point_count`, `total_point_count`) stay implemented directly on `Segment` -- one-liners that `ReadOnlySegment` will reimplement just as cheaply, no shared logic to factor out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
generall
|
4dae9a2a53 | missing file |