mirror of
https://github.com/qdrant/qdrant.git
synced 2026-07-26 12:41:04 -05:00
dev
354 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ivan Pleshkov
|
7469834d9b |
[TQDT] TQ dense/multi vector storage consistency (#9953)
* [TQDT] Align TQ vector storage layout with the reference dense storage
Make the TurboQuant vector storage structurally mirror the reference dense
(and multi_dense) storages, down to file names and their contents:
- Rename files + structs to the dense convention:
- immutable.rs -> turbo_vector_storage.rs (ImmutableTurboVectorStorage ->
TurboVectorStorageImpl)
- appendable.rs -> appendable_turbo_vector_storage.rs
(AppendableTurboVectorStorage -> AppendableMmapTurboVectorStorage)
- multi.rs -> multi_turbo/appendable_mmap_multi_turbo_vector_storage.rs
(TurboMultiVectorStorage -> AppendableMmapMultiTurboVectorStorage)
- ReadOnlyTurboMultiVectorStorage -> ReadOnlyChunkedMultiTurboVectorStorage
- Thin out turbo/mod.rs to module declarations + re-exports: open_* fns move
into their storage files, consts + turbo_storage_roundtrip into shared.rs,
and TurboScoring / TurboMultiScoring join the other TQ traits in
vector_storage_base.rs.
- Split read_only/ into the chunked storage (read_only/) and the single-file
storage (read_only/immutable/), each with the mod/lifecycle/live_reload/
read_ops 4-file layout, mirroring dense/read_only/.
- Introduce multi_turbo/ mirroring multi_dense/, with its own read_only/
submodule holding ReadOnlyChunkedMultiTurboVectorStorage.
- Relocate the storage test suites to
tests/test_appendable_turbo_vector_storage.rs and
tests/test_appendable_multi_turbo_vector_storage.rs, paralleling the
dense/multi_dense integration test files (tests moved verbatim, no new
tests added).
- Fix a gpu-gated VectorStorageEnum match that referenced stale DenseTurbo /
DenseTurboAppendable variant names.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* are you happy fmt
* fix after rebase
* [TQDT] Address review feedback on TQ vector storage split
- gpu tests: use the real `VectorStorageEnum::DenseTurboAppendableMemmap`
variant (the old `DenseTurboAppendable` name never existed post-rename, so
the gpu-feature test failed to compile — missed because `cargo build
--features gpu` does not compile the `#[cfg(test)]` code).
- memory_reporter: report `DenseTurboUring` files as `FileStorageIntent::OnDisk`
like the other io_uring variants; io_uring never mmap-caches, so delegating
to `is_on_disk()` could wrongly report `Cached` for a populated backend.
- turbo_vector_storage: fix the misleading `insert_tq_bytes` doc comment — the
single-file backend rejects the upsert via `?`, so `set_deleted` is never
reached.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [TQDT] Fix clippy::wildcard_enum_match_arm in read-only routing test
Spell out the non-routing `VectorStorageType` variants instead of `_`, so a
future added variant fails the match rather than silently mapping to `false`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [TQDT] Fix stale Turbo4 storage-variant assertion in quantization test
The segment is built with the default (appendable/chunked) storage type, so a
Turbo4 datatype now lands in `DenseTurboAppendableMemmap`, not the single-file
`DenseTurboMemmap`. The assertion was left on the pre-split variant; align it
with the non-turbo branch, which already expects the appendable variants.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* are you happy fmt
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
qdrant-cloud-bot
|
94cc5eeec9 |
Ignore slow Windows CI tests for turbo multi model and quantization (#9850)
Skip the three slowest Windows rust-tests (>60s each): turbo multi random-ops model tests and the turbo Manhattan HNSW quantization case. Use rstest test_attr for the parametrized case since a top-level ignore does not apply to all generated tests. Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Andrey Vasnetsov
|
1d4d6f02da |
Per-query IDF corpus for sparse vector search (#9661)
* Add per-query IDF corpus for sparse vector search
Let the caller choose, per query, which population sparse IDF statistics
are computed over. `params.idf` is either `"global"` (default, unchanged
behavior) or `{"corpus": <filter>}`, where the corpus filter is
independent of - and usually broader than - the retrieval filter.
Decoupling the two keeps the score scale stable when the retrieval
filter tightens: term importance is measured against a population the
user names, not against whatever subset the filter happens to select.
Design decisions:
- Corpus grammar is restricted to a conjunction (`must`) of `match`
conditions on payload fields; loosening later is backward compatible.
- Strict mode validates the corpus filter like a read filter
(unindexed fields rejected).
- `idf` on a vector without the IDF modifier is a validation error,
never silently ignored.
- An empty corpus yields degenerate but corpus-scoped scores (smoothed
IDF over N=0), never a fallback to global statistics - in multi-tenant
collections a fallback would leak term statistics across tenants.
Implementation:
- QueryContext IDF stats are keyed by corpus, so one batch can mix
requests with different corpora.
- Statistics come from the sparse index: df(term) is counted over the
query terms' posting lists only, never by scanning stored vectors.
Small corpora (under ~1/32 of the segment, by cardinality estimate)
are kept as a sorted id list galloping through posting lists via
skip_to; large ones as a dense membership mask filled streaming from
the filtered-points iterator. A misestimated small corpus degrades
into the mask.
- Exposed uniformly: REST (`params.idf`), gRPC (`IdfParams` message),
edge python bindings; OpenAPI schema regenerated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Apply rustfmt
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix clippy manual_is_multiple_of in sparse IDF corpus test.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Allow any filter as IDF corpus
Drop the must+match grammar restriction on the corpus filter. A
restriction enforced only as a validation step over the full Filter
type buys nothing; if a narrower corpus syntax is ever wanted, it
should be a dedicated API-level type instead.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix build: add memory field to SparseIndexConfig in idf corpus test
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>
|
||
|
Arnaud Gourlay
|
dd84044a3e |
refactor: replace StructPayloadIndex::open bool flags with StorageType and IndexLoadMode (#9754)
StructPayloadIndex::open took two adjacent bools (is_appendable, create)
and call sites passed every literal combination: (true, true),
(true, false) and (false, true) all exist. A transposed pair compiles
and silently yields e.g. non-appendable + create instead of
appendable + load-only.
The target enum already existed: open immediately converted the bool
into the private StorageType { Appendable, NonAppendable }, so the bool
survived only at the API boundary, exactly where the swap hazard lives.
Make StorageType public, take it directly, and introduce
IndexLoadMode { CreateIfMissing, LoadExisting } for the create flag.
create_segment had the same trailing create: bool with bare literals at
both callers, so its parameter is lifted to IndexLoadMode as well:
load_segment passes LoadExisting, build_segment passes CreateIfMissing.
No behavior change.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Andrey Vasnetsov
|
fb681b7d9d |
Lazy roaring flags bitmap and bool index counts (#9749)
* [AI] make ReadOnlyRoaringFlags bitmap and bool index counts lazy
Opening a read-only segment scanned every flags file end to end:
`ReadOnlyRoaringFlags::open` materialized the whole RoaringBitmap via
`iter_ones()`. Every payload field carries a null index, so this was paid
per field per segment, for bitmaps most queries never touch.
Make the bitmap a `OnceLock`, filled by a scan on first access. Open now
reads only the tiny status file. `ReadOnlyBoolIndex`'s three eager count
fields collapse into one lazily-derived, cached `BoolCounts`; its
`live_reload` refreshes them in place when present and leaves them unset
otherwise, so reloading an index nothing queries stays scan-free.
Propagate the resulting `OperationResult` through `RoaringFlagsRead`,
`PayloadFieldIndexRead::count_indexed_points`, `FieldIndexRead`,
`PayloadIndexRead::{indexed_points, get_telemetry_data}`, `build_info` /
`build_telemetry` and `SegmentEntry::{info, get_telemetry_data}`, out
into shard, edge and collection.
`ram_usage_bytes` stays infallible: an unmaterialized bitmap holds no
RAM, so it reports 0 via the new `bitmap_if_materialized`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [AI] correct `preopen` comment: `open` no longer scans the flags file
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [AI] fix edge examples for fallible `info()`
`EdgeShardRead::info` now returns `OperationResult<ShardInfo>`. The
examples live in their own workspace (lib/edge/publish), so the main
`cargo check --workspace` never saw them.
Every call site sits in `fn main() -> Result<(), Box<dyn Error>>`, so
propagate with `?`. `bm25-search` compiled either way but would have
printed the `Result` rather than the `ShardInfo`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
Jojii
|
68e7efa6ee | Rename ambiguous build_multivec_segment test fixture (#9713) | ||
|
Arnaud Gourlay
|
cad112bb1c |
Fix Clippy 1.97 (#9716)
* Remove from_iter_instead_of_collect from workspace lints The lint was removed from clippy (beta) and now triggers renamed_and_removed_lints warnings in every crate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::chunks_exact_to_as_chunks Replace chunks_exact with a constant chunk size by as_chunks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::needless_late_init Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::useless_borrows_in_formatting Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::uninlined_format_args Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::for_kv_map Iterate map values directly instead of discarding keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Allow clippy::result_large_err on QueueProxyShard::new_from_version The Err variant intentionally hands the LocalShard back to the caller. Same pattern as the existing allow on ForwardProxyShard::new. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Allow clippy::result_unit_err on wait_for_consensus_commit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
ab0d3ecc62 |
Add unified memory: cold|cached|pinned placement parameter for collection components (#9684)
* Add unified `memory: cold|cached|pinned` placement parameter for collection components
Introduce a single `memory` parameter that controls how each collection
component's data is held in RAM, replacing the inconsistent zoo of
`on_disk` / `always_ram` / `on_disk_payload` flags:
- `cold`: not pre-loaded from disk, cached with usage
- `cached`: pre-populated into page cache on load, evictable under pressure
- `pinned`: materialized on heap, never evicted by cache pressure
The parameter is available on dense vectors, HNSW config, all quantization
configs, the sparse index, all payload field index types, and payload
storage (as a new `payload: { memory }` sub-object on collection params).
When set, it overrides the deprecated legacy flag; when unset, behavior is
unchanged. Legacy flags are marked deprecated (Rust + proto) but keep
working; conflicts are resolved in favor of `memory` with a warning.
New capabilities enabled by the tri-state model:
- HNSW graph links can be pinned (first production caller of the existing
`GraphLinksResidency::Pinned`)
- sparse mmap index, quantized vectors and on-disk payload field indexes
gain a `cached` tier (mmap + populate on open)
`pinned` is rejected by API validation for components without a heap
variant (dense vector storage, payload storage). Low-memory mode degrades
placements at load time via `Memory::clamp_to_low_memory`, matching the
existing `prefer_disk`/`skip_populate` behavior. Effective-placement
comparison in the config-mismatch optimizer avoids spurious rebuilds when
the same placement is expressed through the new parameter.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix gpu-gated tests for the new `memory` field
CI clippy runs with --all-features, which compiles the gpu-gated tests
that were missed locally: add the `memory` field to config literals and
allow deprecated placement params, same as in the rest of the tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add OpenAPI tests for memory placement, keep sparse config downgrade-clean
- OpenAPI tests: create/update collections with `memory` on every component,
assert the parameters are echoed in collection info, assert legacy-only
collections expose no new fields, and assert `pinned` is rejected (422)
for dense vector storage and payload storage on both create and update.
- Persist only the explicitly requested `memory` parameter in
`sparse_index_config.json` instead of the legacy-resolved placement, so
configurations using only the deprecated `on_disk` flag keep byte-identical
files that older Qdrant versions load without unknown fields.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Validate collection meta ops at construction, not only in the API layer
The `memory: pinned` rejection for dense vectors and payload storage
lived in `Validate` impls on the internal request types, which only ran
through the REST actix extractor. gRPC validates just the proto message,
so a gRPC client could persist `pinned` where it is not supported and
have it silently treated as `cached`.
Run the derived validation in `CreateCollectionOperation::new` and
`UpdateCollectionOperation::new` instead: the constructors are the
common chokepoint for all API paths, before the operation is proposed
to consensus. This covers every validator on these types, not just the
`memory` checks, and keeps consensus-apply unaffected so mixed-version
clusters never reject already-committed operations.
`UpdateCollectionOperation::new` becomes fallible; `remove_replica` now
uses `new_empty` since it carries no user config. Regression tests drive
the gRPC conversion path and assert `InvalidArgument` for `pinned` on
create and update, with `cold`/`cached` accepted as a control.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
Andrey Vasnetsov
|
efab63d024 |
Add prefix matching option to keyword index (#9683)
* Add prefix matching option to keyword index
Introduce an opt-in `prefix` option for the keyword payload index and a
new `match: { "prefix": ... }` filter condition, enabling efficient
byte-wise prefix filtering over keyword values (e.g. URL prefixes,
web-ui value autocompletion via facet + prefix filter).
Index side: a new `prefix_index.bin` file stores a sorted, front-coded
key dictionary with a resident block index (cumulative counts per
block); it is an ordered view over the keys of `values_to_points.bin`
and stores no postings. Presence of the file signals prefix support at
load time, so legacy segments load unchanged and enabling the option
goes through the standard incompatible-schema rebuild. The mutable
variant keeps an in-RAM ordered key set (not persisted), the immutable
variant builds a sorted key vector at load, and the on-disk variant
reads the dictionary lazily (block index resident, 1-2 block reads per
prefix lookup; reader is generic over UniversalRead).
Query side: prefix conditions are served from the dictionary when
available (filter + cardinality estimation from per-block aggregates),
from the forward index as per-point checks, and degrade to the payload
full-scan fallback otherwise - same execution model as other match
conditions. Strict mode (`unindexed_filtering_*`) rejects prefix
queries on fields without a prefix-enabled keyword index via a new
KeywordPrefix capability.
HNSW payload blocks: prefix-enabled indexes additionally emit prefix
blocks for heavy branching trie nodes (single-child chains collapsed to
their longest common prefix, one block per distinct point set, emitted
largest-first) so filtered search with prefix conditions gets navigable
subgraphs without rebuilding the same subset repeatedly.
API: `prefix` flag on KeywordIndexParams (REST bool, gRPC empty
message for extensibility), `prefix` variant in the Match oneof, edge
python bindings, regenerated OpenAPI spec.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Split prefix index into a dedicated module, fix clippy in tests
Reorganize the flat prefix_index.rs / prefix_read.rs into a
map_index/prefix_index/ module: format.rs (on-disk layout primitives),
writer.rs, reader.rs (PrefixIndex), map_read.rs (StrMapIndexPrefixRead
with per-variant impls) and tests.rs, with a file-format diagram and a
read-path walkthrough in the module docs. No logic changes.
Also fix clippy --all-targets complaints in test code: replace a
wildcard Match arm with an exhaustive list and a field-reassign-with-
default with a struct literal.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add OpenAPI test for prefix match and snapshot file-tracking test
- tests/openapi/test_prefix_match.py: index-less fallback, prefix index
creation with schema echo, scroll/count parity against ground truth,
facet + prefix filter (the autocompletion flow), strict-mode rejection
without the prefix capability.
- test_prefix_index_file_tracking: `prefix_index.bin` is listed in
`files()` / `immutable_files()` exactly when built with the option.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Replace hand-rolled varint parsing with bytemuck Pod records
Per review: the prefix index format now uses fixed-size little-endian
Pod records (BlockEntry 24 B, KeyEntry 12 B, Header 40 B) written with
bytemuck::bytes_of and read back by copy via pod_read_unaligned — no
manual varint encode/decode, no alignment requirement, one shared
read_record helper. Costs ~9 bytes per key on disk versus LEB128; the
raw key bytes dominate dictionary size, so the simplification wins.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fetch the whole candidate block range with a single storage read
Candidate key blocks of a prefix lookup are contiguous in the file, so
enumerate them from one ranged read instead of one read per block; the
over-read versus the exact key range is bounded by the two boundary
blocks. Block decoding is split into a storage-free helper reused by
the per-block path of stats estimation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Align prefix payload blocks with the geo index granularity principle
Geo's large_hashes emits only the smallest geohash regions above the
threshold — a disjoint antichain, never a parent nested with its
children. Prefix payload blocks now follow the same rule: a heavy
collapsed trie node is emitted only if nothing heavy is nested inside
it, counting both deeper qualifying prefixes and single heavy values
(which already get their own exact-match blocks). Emitted blocks are
therefore mutually disjoint and disjoint from exact-value blocks; no
near-collection-sized ancestor subgraphs, no reliance on the HNSW
connectivity check to skip nested duplicates.
Implemented as a `covered` flag propagated through the existing
LCP-interval scan, still one O(total key bytes) pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Document block wire format and unaligned-read rationale in decode_block
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Arnaud Gourlay
|
0346ea66cd |
fix: unblock optimizer after deleting a named vector (#9641)
* fix: unblock optimizer after deleting a named vector Deleting a named vector could permanently block the config-mismatch optimizer. The source-superset check in SegmentBuilder::update cancelled every rebuild that found the deleted vector still in old segment files, and each retry cancelled again, so optimizations got stuck forever. Removing the check (as in #9609) would fix delete but reintroduce data loss for the CreateVectorName race. Instead, tell the two cases apart with the live collection schema: prune a source vector that is gone from the schema (a real deletion), but cancel when it is still present (a freshly created vector this optimizer has not yet seen). This is safe because the schema is persisted before the op reaches segments, and the live schema is read after the source segments are frozen. The live set covers dense and sparse vectors, since a segment stores both together. When no live source is wired in, the conservative always-cancel behavior is kept. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: wire live vector names into edge optimizers Deleting a named vector left the edge path with the pre-fix behavior: segment_optimizer_config hardcoded live_vector_names to None, so a merge touching a segment that still carried the deleted vector cancelled, and EdgeShard::optimize() propagated the cancellation as a hard error forever. Share the shard config behind an Arc and hand the blocking optimizers a provider that reads the current vector names on every call. Same safety argument as the server wiring: update() holds the segments read guard across both the segment application and the config update, so any name a frozen source segment carries is visible to the live read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: share vector-name enumeration via CollectionParams::vector_names The optimizer's live-schema set and the WAL-recovery valid-name set are the same dense+sparse enumeration and must stay in lockstep; a drift between them would reintroduce a wrong prune/cancel decision. Replace the private helper in optimizers_builder and the inline block in WAL recovery with a single CollectionParams method. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: drop SegmentOptimizer::live_vector_names forwarding hop The default trait method only forwarded to the config getter and had a single caller; ShardOptimizationStrategy now reads the config directly, removing one layer of indirection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ivan Pleshkov
|
9c1b0ed9ee |
[TQDT] Quantization over tq strorage (#9507)
* quantization over tq strorage are you happy fmt are you happy clippy rotation refactor rotation refactor fix tests better test name review remarks dont rotate query in raw scorer tq sources revert quantized_scoring_datatype revert simplify are you happy fmt * remove old comments * review remarks |
||
|
xzfc
|
785815e209 |
ConditionChecker fixups (#9559)
* 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`. |
||
|
Andrey Vasnetsov
|
852ca200b5 |
feat: feature-flagged segment manifest for LocalShard (#9530)
* 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>
|
||
|
Andrey Vasnetsov
|
7b52f04dae |
refactor: move version() from ReadSegmentEntry to StorageSegmentEntry (#9527)
`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> |
||
|
Tim Visée
|
b9154713d7 |
Fix empty min_should with non-zero min_count matching everything (#9401)
* Empty match any with non-zero min count matches nothing * Update description * Validate that min_count is greater than 0 |
||
|
xzfc
|
bf1aa818c6 |
Sparse InvertedIndex: bring back generic methods (#9485)
* sparse InvertedIndex: bring back generics * nits --------- Co-authored-by: Luis Cossío <luis.cossio@outlook.com> |
||
|
Daniel Boros
|
4d6fb4e0ab |
feat: add open for read-only sparse vector index (#9435)
* 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> |
||
|
xzfc
|
3d03dd4752 |
[UIO] condition checker: fallible checks (#9405)
* condition checker: fallible checks * Update |
||
|
xzfc
|
dccfc9a19a |
[UIO] condition checker: fallible constructor (#9404)
* Drop StructFilterContext, use OptimizedFilter directly Reason: it's a no-op wrapper around OptimizedFilter. * GeoMapIndexRead::check_values_any: return OperationResult * condition checker: fallible constructor |
||
|
Andrey Vasnetsov
|
ba3472ea49 |
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> |
||
|
xzfc
|
d6fec5ee3c |
[UIO] Batched sparse index (#9304)
* Add UniversalRead::read_bytes_iter * Batched sparse vector index * Clarify |
||
|
Tim Visée
|
7041b81f76 |
Use assert_matches! (#9231)
* 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> |
||
|
xzfc
|
c52aa9a23f |
Sparse: use blink-alloc instead of typed-arena (#9303)
* Use blink-alloc * Pool arena |
||
|
Arnaud Gourlay
|
abc53d717f | Remove unecesssary Clippy allows (#9267) | ||
|
xzfc
|
fff9c1f1c7 |
Migrate InvertedIndexCompressedMmap to UIO (#9144)
* 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 |
||
|
Jojii
|
7a8703f166 |
[TQDT] API (#9172)
* [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 |
||
|
Jojii
|
76834eccdd | fix: Make HNSW discover test deterministic (#9203) | ||
|
qdrant-cloud-bot
|
6d8f3c6ce9 |
ci(windows): skip IO-heavy tests that aren't OS-specific (#9188)
* 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> |
||
|
Arnaud Gourlay
|
386e222b42 |
fix: reject source-superset schema mismatch at SegmentBuilder (#9110)
* 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> |
||
|
xzfc
|
faf2714f4b | Warn on clippy::wildcard_enum_match_arm (#9096) | ||
|
Andrey Vasnetsov
|
75a0a5dd0b |
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> |
||
|
xzfc
|
a5e1cedf68 |
Error propagation for sparse InvertedIndex (#9076)
* Remove unused InvertedIndexMmap / InvertedIndexImmutableRam * Error propagation for InvertedIndex trait |
||
|
Jojii
|
9f76cb20ab | Determinism for TQ Bits2 HNSW tests (#9036) | ||
|
Jojii
|
2861c6118c | Fix flaky oversampling assertion in HNSW quantization tests (#9029) | ||
|
Andrey Vasnetsov
|
295e65e960 |
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 Vasnetsov
|
b91c85776a |
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 Vasnetsov
|
8de9c11a5d |
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> |
||
|
Ivan Pleshkov
|
491712424d |
tq remove data fit option (#8943)
* tq disable data fit option * remove any mention in grpc |
||
|
Jojii
|
d3ad1ac988 |
API Adjustments for TQ (#8914)
* API Adjustments for TQ * Clippy |
||
|
generall
|
35c1a7c61c |
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> |
||
|
generall
|
bc353f4113 |
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> |
||
|
Daniel Boros
|
76031bd261 |
feat: split read only payload index (#8858)
* feat: split read only payload index * fix: imports * fix: linter |
||
|
Andrey Vasnetsov
|
483809294d |
split read only vector index (#8855)
* [AI] split trait for vector index into read only * fmt * gpu fix * fmt |
||
|
Andrey Vasnetsov
|
144528da31 |
split read only vector store (#8852)
* [AI] split trait for vector store into read only * fmt * fix: trait import --------- Co-authored-by: Daniel Boros <dancixx@gmail.com> |
||
|
Andrey Vasnetsov
|
c1597ac57e |
Split IdTracker trait into IdTrackerRead and IdTracker (#8826)
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> |
||
|
Jojii
|
9bd24411c3 |
TurboQuant E2E segment-level HNSW tests (#8799)
* [ai + manual] HNSW Quantization tests for TQ + better error calculation * Compare TQ 1/1.5 bits to binary quantization |
||
|
xzfc
|
bc958cec96 | Callback-based for_each_payload_block (#8766) | ||
|
Ivan Boldyrev
|
8046c1c453 |
Immutable storage null index (#8653)
* Immutable null storage index Add `ImmutableNullIndex` that uses segment-level deleted points to update in-memory indexes without modifying the storage. * Integrate `ImmutableNullIndex` * Some refinements * Fix merge conflicts * Apply review suggestions * [ai] Get rid of delegate * Fix tests * Fix reopen null index * code review * nitpicks * cut duplication --------- Co-authored-by: timvisee <tim@visee.me> Co-authored-by: Arnaud Gourlay <arnaud.gourlay@gmail.com> |
||
|
Arnaud Gourlay
|
d43635129f | Fix bool index reload as immutable on appendable segments (#8785) | ||
|
Tim Visée
|
74e51f7339 |
Claude: simplify codebase (#8627)
* [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 |