* Add NumericIndexValue
* MapConditionChecker: get rid of `F: Fn() -> bool` bound
To put this type into the upcoming `ConditionCheckerEnum`, it should be
nameable.
* filter_context: `Box<dyn ConditionChecker>` -> `OptimizedFilter`
Removes one level of dyn indirection, so faster checks.
* NullConditionChecker: merge IsEmpty/IsNull checkers into one
So, less variants in the upcoming `ConditionCheckerEnum`.
* feat: feature-flagged segment manifest for LocalShard
Add an on-disk segment manifest (`segments/manifest.json`) that lists a
shard's segments and their state, so out-of-process readers (e.g. a
read-only follower, possibly over object storage) can discover segments
without scanning the filesystem. Gated by the new `write_segment_manifest`
feature flag (off by default).
shard: define the structure + helpers (`SegmentsManifest`,
`SegmentManifestState`, `from_segment_holder`) in a new `segment_manifest`
module, plus the `SEGMENT_MANIFEST_FILE` constant and path helper. The
manifest is a flat `{ "<uuid>": "<state>" }` map; only `active` is written
today, with `under_construction`/`retiring` defined so the format can be
extended without breaking compatibility.
collection: LocalShard owns the writing logic. The manifest is persisted
via `SaveOnDisk<SegmentsManifest>`, initialized from the live segment set
on load/build and refreshed by the optimization worker whenever the
segment set changes (the helper re-derives from the holder and no-ops when
unchanged). No changes to `lib/shard/src/optimize.rs` internals.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Emj8TFxdtrf3K32eWhGgor
* feat: make append_only_mutations a proper feature flag
Replace the debug-only `QDRANT_APPEND_ONLY_MUTATIONS=1` env-var escape
hatch in segment construction with a `FeatureFlags::append_only_mutations`
flag, so it works in release builds and is configurable like the other
flags (config / `QDRANT__FEATURE_FLAGS__APPEND_ONLY_MUTATIONS`).
Deliberately left out of `FeatureFlags::all()`: it changes mutation
semantics and `all` is enabled in dev and e2e configs, so it stays
explicit opt-in.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Emj8TFxdtrf3K32eWhGgor
* upd openapi schema
* feat: register segments in manifest via must-use token, holder builder
Wire segment-manifest maintenance through the segment lifecycle so a
newly created segment is registered as soon as it exists on disk, and
construction can't silently skip it:
- build_segment now returns a #[must_use] NewSegmentToken carrying the
new segment UUID; the lint forces callers to register or drop it.
- SegmentHolder owns the manifest and reconciles it on sync; new
segments are registered ASAP (even before being added to the holder)
via the token, before they can receive writes.
- SegmentHolderBuilder is the only way to obtain a shard's holder; its
build() wires up the manifest, so it can't be forgotten. init/set
manifest helpers are now private / test-only.
- Optimization registers the optimized segment before dropping the
superseded segments' data; deletion is intentionally lenient.
- Document the consistency assumptions on SegmentsManifest: it is a
superset-biased view that may list not-yet-finalized or already-deleted
segments, which readers must tolerate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`version()` is the segment's update version — only meaningful for the
mutable/storage path. Every real caller already reaches it through
`StorageSegmentEntry` (flush), `SegmentEntry` (update), or a concrete
`Segment`/`ProxySegment`; none use it through the read-only base trait.
Move the declaration down to `StorageSegmentEntry` and relocate the
`Segment`/`ProxySegment` impls accordingly. `ReadOnlySegment` no longer
needs it, so drop the method and the write-only `version`/`initial_version`
fields it carried (`live_reload` never refreshed `version`, and neither
field was ever read).
This leaves `ReadSegmentEntry` a clean read-only surface and stops a
read-only segment from exposing a meaningless (stale) version.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: add open for read-only sparse vector index + enum sparse dispatcher
* fix: universal-IO loads for read-only sparse index open
* refactor: rename load_via/open_via to load_universal/open_universal
* refactor: drop StorageVersion::load in favor of load_universal
The regular-IO `load` duplicated `load_universal` over plain `File` IO.
Remove it and route all callers through `load_universal(&MmapFs, ..)`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(sparse): decouple inverted index from concrete storage; split segment_constructor_base (#9461)
Make the read-only index enum generic over storage `S` (no concrete MmapFile),
and remove construction callbacks from the sparse index open paths.
- InvertedIndex is now a pure read/search trait: `open`/`from_ram_index`/
`type Fs` moved off the trait to inherent methods on each concrete index
type, so construction no longer requires `S::Fs: Default`.
- SparseVectorIndex open split into a generic `plan` (load-vs-build decision +
RAM-index build) and generic `finish` (assembly); callers do the concrete
per-type construction, so no construction callbacks are needed.
- ReadOnlySparseVectorIndex::open takes the already-constructed inverted index
and caller-loaded config instead of a `load_inverted_index` callback.
- VectorIndexReadEnum is generic over `S: UniversalRead`; sparse mmap variants
hold `InvertedIndexCompressedMmap<_, S>` rather than a concrete `MmapFile`.
- Split the 1182-line segment_constructor_base.rs into a module (paths,
vector_storage, payload_storage, id_tracker, vector_index,
sparse_vector_index, create_segment, segment, legacy_state); the sparse
dispatcher's match arms collapse into three per-family helpers.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat: LiveReload (no-op) dispatch for read-only vector index enum (#9436)
---------
Co-authored-by: generall <andrey@vasnetsov.com>
Co-authored-by: Claude Opus 4.8 (1M context) <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>
* 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>
* Use assert_matches!
* Add trailing commas
* Use more assert_matches!
Also, drop now redundant `expected blah but got blah` messages because
`assert_matches!` will print these.
* Use debug_assert_matches!
---------
Co-authored-by: xzfc <xzfcpw@gmail.com>
* Migrate InvertedIndexCompressedMmap to UniversalRead
* Rehaul search_scratch.rs (was scores_memory_pool.rs)
- Use `typed_arena::Arena` instead of `bumpalo::Bump`.
Reason: bump don't drop.
Drawback: `Arena::new()` is not free, and `Arena::clear()` doesn't
exist; but this commit has workarounds.
- Use names that make more sense.
* Misc fixes
* InvertedIndexCompressedMmap: explicit S type parameter
* [ai] TQDT in the API
* [ai] unify new sparse error for TQDT
* Rename to `turbo4`
* Add `Turbo4` to comments and doc strings.
* Also validate named sparse vector creation
* ci(windows): skip IO-heavy tests that aren't OS-specific
On the Windows CI runner, several tests are 3-25x slower than on Ubuntu
purely due to slow filesystem IO. These tests exercise platform-agnostic
logic (optimizer, snapshot, WAL recovery, dedup, deferred points) and
are fully covered by the Linux and macOS jobs.
Mark them with `#[cfg_attr(target_os = "windows", ignore = "...")]` so:
- Windows CI skips them and finishes faster.
- They're still listed and runnable via `cargo test -- --ignored`
on Windows for local debugging.
Based on JUnit timings from CI run 26462785436 (PR #8827), this should
save ~5 minutes wall-clock on the Windows job, taking it closer to the
~13min Ubuntu and ~9min macOS jobs (currently 20m24s).
Tests affected:
- lib/wal: check_wal, check_last_index, check_clear, check_reopen,
check_truncate, check_prefix_truncate, test_prefix_truncate_parametric
- lib/edge/optimize: full tests module
- lib/segment deferred-point tests: read_operations,
dense_segment_combinations, sparse, facets
- lib/collection: snapshot_test, points_dedup, wal_recovery,
collection_test::test_ordered_read_api, snapshot_recovery_test
Co-authored-by: Cursor <cursoragent@cursor.com>
* revert(ci/windows): keep WAL and WAL-recovery tests on Windows
Reviewer correctly pointed out that WAL is mmap-backed and has
substantial Windows-specific code paths:
- Different segment allocation (fs4 vs rustix::ftruncate)
- Windows-specific delete_windows() with mmap-drop + retry loop
- Windows-specific sync_all() because directory fsync is unavailable
- Windows-specific lock proxy file (directories aren't lockable)
So those tests genuinely need Windows coverage. Reverted skips for:
- lib/wal/src/lib.rs: all check_* tests and test_prefix_truncate_parametric
- lib/collection/src/tests/wal_recovery_test.rs: all three tests
Still skipped on Windows (no OS-specific code in their production paths):
- lib/edge/optimize.rs (no cfg(windows) in source)
- lib/segment deferred-point tests (segment/ has no cfg(windows))
- lib/collection snapshot/dedup tests (collection/ has no cfg(windows))
- lib/collection integration snapshot_recovery + ordered_read_api
Co-authored-by: Cursor <cursoragent@cursor.com>
* revert(ci/windows): keep collection integration and snapshot_test
Per reviewer request, keep running these on Windows:
- lib/collection/tests/integration/* (snapshot_recovery_test,
collection_test::test_ordered_read_api)
- lib/collection/src/tests/snapshot_test.rs
These exercise higher-level collection/snapshot behavior that benefits
from cross-platform validation.
Remaining Windows skips (production code has no cfg(windows) branches):
- lib/edge/src/optimize.rs: 14 optimizer tests
- lib/segment/src/segment/tests/mod.rs: 4 deferred-point tests
- lib/collection/src/tests/points_dedup.rs: 2 dedup tests
Co-authored-by: Cursor <cursoragent@cursor.com>
* ci(windows): also skip HNSW/quantization integration tests
Per reviewer, also skip these segment integration test modules on Windows:
- hnsw_quantized_search_test::* (25 tests)
- multivector_filtrable_hnsw_test::* (rstest cases)
- multivector_quantization_test::* (rstest cases)
- byte_storage_quantization_test::* (rstest cases)
- payload_index_test::test_struct_payload_index_nested_fields
These exercise pure HNSW/quantization correctness on top of standard
segment IO that is already covered by tests we keep running on Windows.
Adds ~930s of sequential time to the Windows skip list, bringing the
expected wall-clock saving from ~3 min to ~8-10 min.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: serialize optimization proxy install against shard updates
execute_optimization captures `target_config` from the optimizer's frozen
config and then wraps source segments in proxies. Between those two points,
`CollectionUpdater::update` can apply a `CreateVectorName(V)` to the source
segments via `apply_segments`, leaving the optimizer with sources that have
V but a target_config that does not. The optimization then produces a
merged segment without V, and a follow-up optimization (running with the
refreshed config that includes V) fails to use that segment as a source:
"Cannot update from other segment because it is missing vector name X".
Close the race by extending the scope of the existing
`LockedSegmentHolder::acquire_updates_lock` to cover the proxy install
window. `CollectionUpdater::update` already takes this lock before
processing any shard update, so concurrent writers wait until proxies are
in place — at which point further mutations hit the proxies (recorded as
intent and propagated to the merged segment in `finish_optimization`)
instead of the originals. The guard is dropped right after proxy install so
the slow build phase does not extend it.
Tests:
- Three `SegmentBuilder::update` tests document the precondition the lock
now guarantees: with a target schema that adds a named vector the source
lacks, update errors with "missing vector name X". Quantized and
mixed-source variants exercise the same error path.
- `test_optimize_blocks_proxy_install_on_updates_lock` asserts the
invariant directly: while the updates lock is held, proxies are not yet
installed. Verified to fail when the new guard is removed (otherwise it
passes because `finish_optimization` also takes the same lock, so a
naive "did optimize finish?" check would not catch a missing proxy-install
guard).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(optimize): finish_optimization lock order; drop redundant tests
Address review of #9110:
1. `finish_optimization` was acquiring `upgradable_read` before
`acquire_updates_lock`, while the new guard at the start of
`execute_optimization` acquires them in the reverse order. With two
optimizer threads in flight, thread A in `finish_optimization` could
hold `upgradable_read` and wait on `updates_lock` while thread B at the
top of `execute_optimization` held `updates_lock` and waited on
`upgradable_read` (parking_lot allows only one upgradable reader),
deadlocking. Swap `finish_optimization` to take `updates_lock` first so
both halves agree.
2. Drop the quantized and mixed-source variants of the inverted
`SegmentBuilder` unit test — all three asserted the same error path
(the mismatch check fires before quantization training or per-source
branching), so only one is useful as documentation of the precondition.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: reject source-superset schema mismatch at SegmentBuilder
Drop the lock approach (deadlocked test_continuous_snapshot) and fix the bug
at the merge layer instead.
Snapshot's proxy_all_segments_and_apply acquires the segment_holder
upgradable_read first and then takes acquire_updates_lock tactically inside
the snapshot operation. The previous commits' lock-extension acquired
updates_lock before upgradable_read, so a snapshot in flight and an
optimization just entering execute_optimization could deadlock holding each
other's required next lock. Snapshot cannot easily reverse its order — that
would hold updates_lock for the entire snapshot duration, blocking all
writes.
Move the fix to where the actual harm happens: SegmentBuilder::update
iterates the target's vector_data and silently drops source vectors that
aren't in target. That silent drop is what produces the broken merged
segment in the CreateVectorName-vs-optimizer race. Add a check that every
source vector name is in the target schema; the optimization aborts cleanly
on mismatch and the next round (with refreshed config) merges correctly.
This is strictly stronger than the lock: the lock only closed the window
where V arrived *during* the proxy-install region. The schema check catches
both that window and the window where V's apply_segments completed before
the optimizer's lock acquisition.
Diff is contained to lib/segment; no locking changes, no cross-crate
plumbing. test_continuous_snapshot passes again.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(segment_builder): use Cancelled instead of ServiceError for schema mismatch
ServiceError flips the shard to RED status (via `report_optimizer_error` →
`segments.optimizer_errors`) and stays sticky until the next
`recreate_optimizers_blocking` clears it. That's the right shape for
hardware/IO failures but wrong for the schema-mismatch case here, which is
an expected, recoverable race outcome — the next optimizer round with a
refreshed target_config merges the same originals cleanly.
`Cancelled` is the variant the optimization worker treats as a recoverable
cancellation: logged at debug, tracker marked Cancelled, no
`report_optimizer_error` call, no RED status.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(segment_builder): also use Cancelled for the existing target-superset error
The existing "missing vector name" check at the start of the merge loop
also fires during a race — specifically the optimizer-vs-DeleteVectorName
shape, where V is removed from originals before J wraps proxies but J's
frozen target_config still has V. Like the new source-superset check, this
is an expected, recoverable race outcome, so use Cancelled instead of
ServiceError to avoid flipping the shard to RED.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* 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>
* 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>
* 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>
* 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>
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>
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>
Read-only methods now live on a separate IdTrackerRead trait, with the
mutating IdTracker trait extending it. This lets read-only call sites
depend only on the read API.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [ai] Replace manual into mappings with Into::into
* Reformat
* [ai] Use implicit .iter
* Don't iterate over keys too
* [ai] Replace unwrap_or
* Reformat
* [ai] Use as_deref and then_some
* [ai] Use more to_string
* [ai] Use explicitly typed into conversions
* Reformat
* [ai] More explicit into conversions
* Reformat
* measure filterable hnsw indexed vs unindexs + cpu vs gpu
* measure filterable hnsw indexed vs unindexs + cpu vs gpu
* limit GPU parallelism
* address review
* Propagate HardwareCounterCell through MmapMapIndex::get_values
Previously, `MmapMapIndex::get_values` used `ConditionedCounter::never()`,
which silently skipped all hardware IO counter tracking for mmap map index
value reads. This propagates a real `HardwareCounterCell` through the full
call chain so that disk IO from `values_iter` is properly measured.
Changes:
- `MmapMapIndex::get_values` now accepts `&HardwareCounterCell`, creates a
`ConditionedCounter` via `make_conditioned_counter`, and measures the
`deleted` bitvec access (matching `check_values_any` behavior).
- `MapIndex::get_values` forwards the counter to the `Mmap` variant.
- `FacetIndex::get_point_values` trait method now accepts `&HardwareCounterCell`,
propagated through `FacetIndexEnum` and both impls (MapIndex, BoolIndex).
- `indexed_variable_retriever` accepts and forwards the counter to
`IntMapIndex`, `KeywordIndex`, and `UuidMapIndex` calls.
- `SegmentBuilder::update` and `_get_ordering_value` accept and propagate
the counter instead of creating disposable instances.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat: add test
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel Boros <dancixx@gmail.com>
* Skip broken tests
* Add rocksdb dropper
This is a temporary tool. It will be removed later in this PR.
* [automated] Drop rocksdb
This commit is made by running tools/rocksdb/drop.sh
* Touch-up after ast-grep
The previous automated commit removed items, but not their comments.
Also, some blocks are left with only one item.
Also, ast-grep can't handle macros like `vec![]`.
This commit completes the job.
* Remove RocksDB dropper
* Remove mentions of rocksdb feature in CI
* Fix clippy warnings
These `FIXME` comments added previously in this PR by ast-grep by
"peeling" cfg_attr like this:
-#[cfg_attr(not(feature = "rocksdb"), expect(...))]
+#[expect(...)] // FIXME(rocksdb): ...
This commit removes these allow/expect attributes and fixes clippy
lints.
* Remove leftover rocksdb-related code
Removed:
- Cargo.toml: rocksdb cargo feature flag and dependencies.
- code: rocksdb-related items that was not under feature flag.
- flags.rs: rocksdb-related qdrant feature flags.
Disabled:
- Some benchmarks because these do not compile now.
* Print warning if rocksdb leftovers found in snapshots
* Remove Clone from tokenizer
* Regenerate openapi.json
* Don't insert deferred points into sparse index
# Conflicts:
# lib/segment/src/segment/tests.rs
# lib/segment/src/segment_constructor/segment_constructor_base.rs
# Conflicts:
# lib/segment/src/segment_constructor/segment_constructor_base.rs
* Clippy
* Assert consistency of deferred_internal_id var
* Return before SparseVector conversion in case of deferred point
* Use debug_assert instead
* Refactor: `SearchSegmentEntry` for search ops
* Move `has_deferrend_points_method`
to `SearchSegmentEntry`
* Reorg imports
* Renamings per review
* fixup! Renamings per review
* `StorageSegmentEntry` trait
It contains methods dealing with storage and syncronization
* Move index methods from `ReadSegmentEntry`
to `SegmentEntry`
* Add `_concurrent` suffix to some methods
* move field index methods into NonAppendableSegmentEntry
---------
Co-authored-by: generall <andrey@vasnetsov.com>
* UIO traits: require Sized
I don't think we ever going to add unsized implementation. So, lets,
require it on the whole trait rather than on individual methods.
* Move `AccessPattern` to the `common` package
Was: segment::vector_storage::vector_storage_base::AccessPattern
Now: common::generic_consts::AccessPattern
* UIO traits: replace `bool` with `AccessPattern`
Instead of `IdTracker` to have `iter_*` methods, it now has a `point_mappings()` method, returning a guard to `PointMapping` value. It prepares for moving the point_mappings under a lock to allow parallel searches and deletions.
Also, an incorrect unsafe in `Segment::iter_points` is replaced by a safe version with `self_cell`.
Reduce test parameters across 5 areas to cut test runtime without
sacrificing coverage:
1. HNSW PQ tests: lower vector dimensionality from 131 to 64 for product
quantization variants, cutting PQ codebook training time (~31s -> ~8s)
2. HNSW index build: use 2 threads instead of 1 for index construction;
the tests only check accuracy above a threshold so determinism is not
required
3. Search attempts: reduce from 10 to 5 query vectors per test
4. Rescoring: skip the expensive re-upsert-all-as-zeros rescoring check
for PQ tests (already covered by scalar quantization variants)
5. Near-miss speedups:
- WAL: reduce QuickCheck iterations from 100 to 50
- Gridstore: halve operation counts, drop 64-byte block size case,
reduce proptest cases for gap search
- Continuous snapshot: reduce timeout from 20s to 10s
Made-with: Cursor
Co-authored-by: Cursor Agent <agent@cursor.com>
