mirror of
https://github.com/qdrant/qdrant.git
synced 2026-09-29 01:17:56 -05:00
perf/update-path-spawn-blocking
641
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Tim Visée
|
7b466c60aa | Bump dev version to 1.19.2-dev (#10464) | ||
|
tellet-q
|
fdfe5b8bd5 |
Revert "feat(consensus): warn when applying a single entry stalls the consensus thread (#10215)" (#10241)
This reverts commit
|
||
|
Andrey Vasnetsov
|
de1db19c09 | Use cachestat for memory residency probes (#10455) | ||
|
|
61ffec42ab |
Fall back to per-vector TQ scoring for scattered ids (#10381)
* Fall back to per-vector TQ scoring when runs are short
Run-batched scoring pays off on plain and dense filtered scans but
regresses HNSW, where neighbor ids rarely form consecutive runs and batch
setup dominates. Gate both EncodedVectorsTQ::score_points and Turbo
score_query_batch on offsets_worth_batch_scoring, which takes the run path
only when the ids split into runs averaging BATCH_SCORE_MIN_MEAN_RUN
vectors or more.
The average decides, not the longest run: a sorted id list -- what a
filtered scan hands the scorer -- already contains adjacent pairs at ~1%
density while its runs still average one vector, so a "contains a run of
>= 2" test sends those down the run path to pay setup per vector, measured
at up to +75% against the better path. An average also stays independent
of the batch size the driver slices ids into, which a longest-run test does
not. The threshold comes from the measured crossover -- mean run 2.0-3.6,
stable across dims 128/512/1024, both RAM storages and the 1/2/4-bit
widths -- and a fully contiguous block is recognized in O(1), so a plain
scan pays nothing for the gate.
* io_uring: never gate run-batched scoring
The gate exists because run batching costs setup that short runs do not
repay on RAM and mmap storages. io_uring is the opposite: one run-granular
read beats the batched per-vector path at every density measured -- 27% on
HNSW-shaped id lists, 43% at 25% filter density, 93% on a full scan --
because per-request submission and completion bookkeeping dominates once
the data sits in the page cache. Gating it costs 36% on HNSW-shaped lists.
Add EncodedStorage::prefers_run_reads, defaulting to false so every storage
keeps its current routing, and override it for the single-file quantized
storage when its backend is io_uring. Remote backends (object stores, a
gRPC peer) deliberately keep the per-vector path: their reads pipeline
across a batch, while run-granular reads would serialize the round trips.
QuantizedStorage::is_in_ram_or_mmap() still reports true for every backend,
which is what routes io_uring into the gate in the first place. Correcting
that would also change how the multivector storage picks between its
in-memory and uring scoring paths, so it is left to a separate change.
* Rename prefers_run_reads to prefers_contiguous_reads
"Run reads" is easy to misread as "execute reads"; contiguous makes the
storage I/O preference explicit.
* QuantizedStorage::for_each_run: pipeline run reads on async backends
With `prefers_contiguous_reads()` true for io_uring, every batch goes
through `for_each_run`, which read each run synchronously: a scattered
id list (HNSW neighbours) waited on one disk read per vector, where
`for_each_in_batch` kept the whole batch in flight through `read_batch`.
Submit all runs of a batch together, still one read per run, so
scattered reads stay pipelined while a scan still reads each run in one
request. Backends without async reads keep the sequential loop.
`turbo_vector_search` (dim 1024, 200k vectors, 4096 shuffled ids per
iteration) against dev: cold scattered io_uring 187 ms -> 26.5 ms
(dev 29.5 ms); the warm scan keeps 198 ms -> 21 ms. Warm scattered
lands at 5.06 ms (dev 4.67 ms), giving up the 3.68 ms of synchronous
reads, which only holds with the data already in the page cache.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRiRaiPsm5CEBgpQ7VAQab
* Iterate consecutive runs and share the run-scoring gate
`for_each_consecutive_run` becomes `consecutive_runs()`, a lazy iterator
over `Run { first, start, len }`, so a storage can feed runs straight into
a read pipeline. `QuantizedStorage::for_each_run` loses the intermediate
`Vec` and the duplicated `ReadRange` construction: the pipelined branch
maps the iterator into `read_batch`, the synchronous branch keeps the
per-run `Sequential`/`Random` hint that picks between mmap's two mappings.
The routing condition duplicated at both scoring call sites moves into
`EncodedStorage::prefers_run_scoring`: same expression, one place.
`for_each_run`'s contract no longer promises run order: pipelined
backends report reads as they complete, so callers address results by
`first`. Add a contract test over the mmap and disk-cache backends; the
latter is the async-capable backend that runs on every platform and
covers the `read_batch` branch io_uring takes on Linux, which no test
exercised before.
Measured on Apple M3 against
|
||
|
Luis Cossío
|
81d27d9baf | [live-reload] hotfix: infallible to_owned in DiskCache (#10429) | ||
|
Andrey VasnetsovandClaude Fable 5
|
01ddb5cc65 |
[UIO] Split async into extension traits, implement only where genuinely async (#10424)
* Split async IO into extension traits; only async-capable backends implement them Move `read_bytes_async` / `open_async` off the universal `UniversalRead` / `UniversalReadFs` traits into dedicated extension traits, `UniversalReadAsync` and `UniversalReadFsAsync` (traits/async_io.rs). Only backends with a genuine async story implement them — the blob family, the disk caches layered over it, and a trivial ready-impl for mmap (tests and the mmap lookup path) — each in a dedicated async_io.rs next to its sync impl. `CachedFs` now requires its inner filesystem to be `UniversalReadFsAsync`; the requirement reaches segment code through one supertrait bound on `UniversalReadExt`. io_uring implements no async surface anymore: the tokio_uring bridge thread, its tests, the musl-gated tokio-uring dependency, and the `IoUringFile` read-only-segment wiring (`UniversalReadExt` impl and the *RoIoUring condition-checker variants) are deleted — io_uring is not a read-only-segment backend. The payoff for live reload: `CachedFs::resolve_prefetched` awaits every parked prefetch, and the edge refresh flow now runs preload -> resolve -> reload, so the per-segment write locks never wait on IO. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Decouple UniversalReadExt from the async filesystem requirement UniversalReadExt is condition-checker dispatch; it never consumed the async surface itself. Drop its `Fs: UniversalReadFsAsync` supertrait bound and relax CachedFs's struct-level bound back to `UniversalReadFs` — the async requirement now lives on the one impl that consumes it, `CachedReadFs for CachedFs` (schedule_open parks the inner filesystem's `open_async` futures). The bound then surfaces only on the lifecycle/preload impl blocks that go through CachedReadFs (segment open, live-preload/reload, config reload, edge load/refresh); the search path carries no async bounds at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis CossíoandTim Visée
|
44e54f4539 |
[edge] open and reload IO don't block search pool (#10366)
* existing segments: wait for IO outside of search pool * new segments: wait for IO outside of search pool * extract reload into separate function * Update lib/edge/Cargo.toml --------- Co-authored-by: Tim Visée <tim+github@visee.me> |
||
|
Luis Cossío
|
0798695099 |
[UIO] Segment live_preload waits for all IO before returning (#10357)
* `LiveReload::live_preload` returns futures * await reopens and reloads concurrently |
||
|
Luis CossíoandClaude Fable 5
|
515e8ace69 |
[UIO] make UniversalRead::live_preload async (#10356)
* rename `reopen`->`live_reload` and `schedule_reopen`->`live_preload` * `UniversalRead::live_preload` returns a shared future * assert snapshot-miss eagerly on `live_preload` `live_reload` cannot see the failed preload: its blocking fallback re-resolves the length from the remote and succeeds. The error surfaces at preload time, as callers (`ok_not_found`) expect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Cossío
|
83311bc243 |
[UIO] CachedFs waits for scheduled files to resolve + misc (#10353)
* [CachedFs] new `schedule` and `wait_all` primitives * [AppendableIdTracker] don't reopen if just opened * eager NotFound in `schedule_open` * add traces for async reads * finish `preopen`/`preload` with `wait_all` * lock all segments in parallel for `live_reload` * LIST before everything to do: we don't have whole-fetch in async mode. to prevent sequential `len`, we won't overlap static files with LIST. * `wait_all` returns nothing |
||
|
|
e91206e71a |
Skip prefetch for small vector storages (#10420)
* Skip prefetch for small vector storages (they fit in L2) * Update lib/common/common/src/prefetch.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update lib/common/common/src/prefetch.rs Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com> * clippy --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com> |
||
|
qdrant-cloud-bot
|
9199eff03f |
Fix debug-tools musl build: gate tokio-uring on non-musl targets (#10419)
The debug-tools workflow cross-compiles for x86_64-unknown-linux-musl, but tokio-uring 0.5.0 requires libc::statx which is unavailable on musl. Skip the tokio-uring dependency on musl and fall back to sync io_uring reads. |
||
|
Luis Cossío
|
e9cc3b8673 |
schedule_open returns nothing (#10355)
|
||
|
Luis CossíoandClaude Fable 5
|
2279c4a79b |
[UIO] UniversalReadFs::open_async (#10352)
* `UniversalReadFs::open_async` * `schedule_open` polls once Scheduled opens must start eagerly: sync backends complete their `open_async` on the first poll, preserving the prefetch contract (handles outlive later file deletions/replacements). Moved down from the integration branch so this PR stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Cossío
|
4e4aca8893 |
[UIO] renames + enforce LiveReload::live_preload (#10351)
* make LiveReload::live_preload required * rename `schedule_prefetch`->`schedule_open` * rename `reschedule_prefetch`->`reschedule_open` |
||
|
Luis Cossío
|
dc3b888075 |
[UIO] impl IoUringFile::read_bytes_async (#10288)
* bridge async with a dedicated `tokio_uring` thread * impl `IoBufMut` for `AVec` * [AI] Add tests * [AI] handle O_DIRECT odirect test |
||
|
Luis Cossío
|
9e9edcdcd7 |
[UIO] impl DiskCache::read_bytes_async (#10259)
* [AI] TDD: new tests * impl `read_bytes_async` for simplediskcache |
||
|
Luis Cossío
|
491fe1a1f4 |
[UIO] UniversalRead::read_bytes_async stubs (#10258)
* impl `read_bytes_async` stubs * clippy * map err to `TaskPanicked` when appropriate |
||
|
Jojii
|
c4963e7a0a |
Hide DRAM latency in quantized scoring with software prefetch (#10342)
* Add prefetch to quantization storage * Gated prefetch * Add support for ARM * Update doc strings * T1 Prefetching * Update stale doc strings + Tests |
||
|
Arnaud GourlayandClaude Opus 5
|
384cb79b15 |
build(deps): disable unused default features (#10331)
Drops 6 crates from the release build and 7 from the workspace test build, with no source changes. - geo: no triangulation, only Contains/Intersects/Haversine (spade, earcut) - jsonwebtoken: HS256 from_secret only, no PEM keys (pem, simple_asn1) - tar: nothing sets unpack_xattrs, which defaults to false (xattr) - duplicate: every duplicate_item names its module (proc-macro2-diagnostics) - pprof: no C++ frames to demangle (cpp_demangle) Also promotes duplicate to a workspace dependency. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Jojii
|
aea5def9da |
Optimization: Skipping items before pushing into PriorityQueue. (#10099)
* Optimization: Skipping items before pushing into PriorityQueue. * Apply suggestion from top-k-update branch * Stabilize order of equal scored items in tests |
||
|
dependabot[bot]andqdrant-cloud-bot
|
88196be010 |
build(deps): bump tango-bench from 0.7.2 to 0.8.0 (#10317)
* build(deps): bump tango-bench from 0.7.2 to 0.8.0 Bumps [tango-bench](https://github.com/bazhenov/tango) from 0.7.2 to 0.8.0. - [Release notes](https://github.com/bazhenov/tango/releases) - [Commits](https://github.com/bazhenov/tango/compare/v0.7.2...v0.8.0) --- updated-dependencies: - dependency-name: tango-bench dependency-version: 0.8.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * fix: drop deprecated tango_main for tango-bench 0.8.0 tango_benchmarks! now generates main(); tango_main! is a deprecated no-op. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: qdrant-cloud-bot <111755117+qdrant-cloud-bot@users.noreply.github.com> |
||
|
|
bf4e7b7740 |
[LiveReload] Preload ReadOnlyFlags (#10269)
* impl `live_preload` for `ReadOnlyFlags` * fix: apply CodeRabbit auto-fixes Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> |
||
|
Arnaud GourlayandClaude Opus 5
|
896deaeeef |
Fix clippy warnings from Rust 1.98 beta (#10265)
* Fix clippy warnings from Rust 1.98 beta
* drop a redundant trait import in an io_bridge test module, it already
arrives through `use super::*`
* rewrite two `chunks_exact(CONST)` sites as `as_chunks::<{ CONST }>()`
for the new `chunks_exact_to_as_chunks` lint
* return `bool` from `wait_for_consensus_commit` instead of
`Result<(), ()>`, which `result_unit_err` now flags on `async fn`. Its
only caller did `.is_ok()` on it
* allow `result_large_err` on `QueueProxyShard::new_from_version`, which
hands the `LocalShard` back to the caller on failure. Mirrors the allow
already on `ForwardProxyShard::new`
* migrate three `Atomic::fetch_update` calls to `try_update`, the name it
is renamed to in 1.99. The new name already exists at our 1.97 MSRV
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* keep guarantee on caller
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Tim ViséeandRoman Titov
|
ec3589f8b2 |
Reject .. in collection names (#10242)
* Reject dot segments in collection names Collection names are used as directory components on disk (storage/collections/<name>, snapshots/<name>). Apply the same plain-file-name predicate already used for snapshot names, so names like ".." and "." no longer resolve outside the parent directory. Applied to the legacy validator too: dot segments were never usable as directory names, so no existing collection can be locked out by this. * Fix collection name validation test on Windows, backslash disallowed * Apply suggestions from code review Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> --------- Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> |
||
|
Tim Visée
|
9f01222ccf |
Integrate new bitflags structure (#10123)
* Add `FlagsMode::from_feature_flags`, the mode for newly created flags Compact in serverless-compatible deployments, dynamic otherwise. Only creation consults it; opening existing flags detects their mode from disk. * Support the compact mode in the read-only flags types Add `ReadOnlyFlags`, the mode-dispatching union of the two read-only counterparts, serving the shared `RoaringFlagsRead` surface. Teach `InMemoryBitvecFlags` to detect the mode it opens; its compact `reload_appended` decodes the whole (small) file, as the format has no random access. * Create flags through mode selection in storages and indexes Vector storage deleted flags and the bool/null indexes now open through `open_or_create` with the mode from the feature flags: serverless deployments create compact flags, dedicated ones keep creating dynamic flags, and existing flags are opened in their detected mode either way. * Read flags in either mode in the read-only bool and null indexes `ReadOnlyFlags` shares the `RoaringFlagsRead` surface and the lifecycle signatures of the roaring type it replaces, so the swap is a type rename. * Add TODO to not lock bitmask structure during flush * `MutableStoredBitmask::save` returns the number of bytes written Zero when the skip-clean save wrote nothing. Lets wrappers charge the actual write to a hardware counter. * Refuse to open compact flags in a dynamic-mode directory Creating the compact file next to dynamic files would leave a directory of both modes behind, which every later open rejects — refuse up front instead. Both production callers already rule the case out through `FlagsMode::detect`, so this only removes a foot-gun for future callers. The open-or-eagerly-create logic moves into `open_or_create_compact_mask`, shared with the update-only writer next. * Rewrite `UpdateOnlyStoredFlags` onto the compact bitmask The update-only flags writer now writes the compact mode — a single roaring-encoded `compact_flags.dat` through `MutableStoredBitmask` — instead of rewriting the whole padded dynamic file pair every batch. A flush with no effective changes now writes nothing at all, where the old writer rewrote the full mask on any `set`. This also fixes opening serverless-created segments: the old open eagerly wrote a `status.dat` into directories the writable side had created in the compact mode, leaving files of both modes behind and poisoning the directory for every later open. A directory already holding dynamic-mode flags is refused loudly rather than kept current or migrated; rebuild the segment to migrate its flags. Migration may come later. Drops the now-dead `InMemoryBitvecFlags::into_bitvec` and `DynamicFlagsStatus::new`, and demotes `file_size_for` to private. * Run edge tests with serverless feature flags The edge fixtures ran with default feature flags, building leader shards with dynamic-mode flags — a configuration edge never serves in production, and one the update-only flags writer now refuses. It also hid that the writer poisoned compact directories: no test exercised update-only writes over a serverless-created shard. Feature flags are process-global and first-init-wins, so every fixture in the binary initializes the same serverless set; the manifest test folds into it, since serverless implies `write_segment_manifest`. * Don't use sequencial mode for one shot reads |
||
|
Arnaud GourlayandClaude Opus 5
|
f3ffc65531 |
fix(shard): flush CoW destinations before the payload-index pre-build flush (#10201)
* fix(shard): flush CoW destinations before the payload-index pre-build flush create_field_index force-flushes each segment before building an index on it (flush-before-build, #9767), one segment at a time, outside flush_all's all-segment lock capture and copy-on-write dependency ordering. That flush durably advances a CoW source past the delete halves of its pending moves. The appendable-first iteration order usually flushes the destination before the source, but not always: a destination proxy-wrapped by a running optimization is classified non-appendable and can skip its flush entirely through the already_indexed short-circuit (the proxy reports the field as present), and a move landing mid-pass is ordered behind nothing. Once the source flushes, the move's WAL entry stops being replayable: the pre-image is durably deleted while the only current copy sits in the unflushed destination, and a graceful close then loses the point. This is the root cause of the nightly model-testing reload divergence (#10095), traced end-to-end in CI runs 31583878492 and 31583871346: cow move op 5197 into a freshly proxied destination, index op ~5252 flushing every source past it while skipping the proxy, destination reloading at 5181, replay declining with 'No point with id'. The fix mirrors flush_all's invariant at the only per-segment flush site: before flushing a segment, flush the destinations of its pending flush_dependency edges (one hop suffices, destinations are appendable and never CoW sources). Destination guards are taken before the flush lock to keep the documented [segment locks -> flush lock] ordering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(shard): regression test for the CoW-destination flush in create_field_index Reproduces the #10095 loss shape deterministically: a pending copy-on-write move out of a non-appendable source, a destination whose own pre-build flush is skipped by the already_indexed short-circuit, then a holder-wide create_field_index. Verified failing with the dependency-aware flush neutralized (destination stays behind the move while the source flushes past it) and passing with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(shard): move the CoW-aware single-segment flush into SegmentHolder Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
xzfc
|
74dd4b71e3 |
Integrate batched HNSW (#10194)
* [14] HnswGraph: wrapper enum over in-RAM and batched graphs * [15] HnswGraph: route async backends to the batched graph * [15.a] De-tautologize `test_open_matrix` Anti-pattern: `expect_batched` mirrors `format_is_batched` logic. * [16] HNSW healing: reopen as direct * [17] Add async_hnsw_graph feature flag * Batch size |
||
|
Luis Cossío
|
57e7389f91 |
[LiveReload] Prepare segment preload (#10221)
* genericize live_reload fs parameters * impl live_preload for ReadOnlySegment * split edge refresh into preload and apply passes * only rotate file infos after successful reload |
||
|
|
3836a0bfc4 |
Add writer for stored bitmask type (#10107)
* Extract stored bitmask encoding into `bitmask_file_bytes` Also single-source the u32 position-space bound as `MAX_LOGICAL_LEN`. * Add `MutableStoredBitmask`, collecting bitmask changes in RAM Materializes via the existing reader without keeping the file handle open, tracks diverged positions, and atomically rewrites the whole file on save - skipping the write when nothing changed. * Rename payload to bits * Use changed boolean * Remove now obsolete test * Borrow the bitmap in bitmask encoding via Cow, avoiding a clone on save Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: generall <andrey@vasnetsov.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
tellet-q
|
dfca67f5ed |
feat(consensus): warn when applying a single entry stalls the consensus thread (#10215)
* feat(consensus): warn when applying a single entry stalls the consensus thread longer than a threshold |
||
|
Andrey VasnetsovandClaude Fable 5
|
5f8cef9ebd |
[UpdateOnly] Writer over object storage (#10214)
* Drop the vestigial UniversalWrite bound from the update-only writer Neither writer kind performs in-place writes: AppendableSegment is built on UniversalAppend, and DeleteOnlySegment tombstones via whole-mask atomic_save (UniversalWriteFileOps), which UniversalAppend's supertrait already carries. The bound is a leftover from the DiskIdTracker-based iterations that mutated the deleted mask in place. With it gone, UpdateOnlyEdgeShard::apply_batch is instantiable with the object-store-appendable CachedBlobFile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * edge-shard-update: --apply writes the batch, over object storage too Open the object-storage backends through CachedBlobFs/CachedBlobFile instead of the read-only DiskCacheFs handle, so the shard is appendable in both modes, and add --apply: generate the same schema-derived batch and run apply_batch instead of preview_batch. Dry run stays the default and the generation is shared, so the preview cannot drift from what an apply would do. AwsConfig::native_append is exposed as --native-append for AiStor/RustFS-style endpoints; the Cached* types join io_bridge_object_store's re-export of the io_bridge stack. Applying to a leader-produced shard currently fails with a clean refusal — its appendable segment's payload storage was created in mutable mode, which the append-only writer rejects — the known segment-bootstrap gap, next in line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * CachedBlobFile: latency tracing for append_bytes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * UpdateOnlyEdgeShard: sequential batches through one writer Writers open once at shard open, next to the lookup segments they resume from. apply_batch hands the writer back on success, live-reloading the lookup half of every segment the batch wrote to (new LookupSegment::live_reload, mirroring the read-only segment's); on error the writer is consumed, since its lookups may no longer describe the durable state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * edge-shard-update: --interactive mode, sequential batches on one writer After each applied batch, prompt on stdin for the next round's ids and apply them through the writer apply_batch handed back — no shard re-open — with op-num (and seed) incremented per round. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * CachedBlobFile: create the missing object on an offset-0 rewrite append The caller-side rewrite path (part-copy S3 stores below the direct-append threshold) validated the offset against the mirror length, whose initialization HEAD-requests the remote and surfaced NotFound for an object that does not exist yet. Direct-append backends (GCS compose, native append) already create the object on an offset-0 append; the rewrite now reads a missing remote as length zero so its whole-object PUT does the same, and a non-zero offset against a missing object reports an offset conflict. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * UpdateBatchOutcome: per-point records of retired slots Each applied point now carries a PointApplyRecord: what happened to it (stored/deleted/skipped/missing) and which slots it vacated where — tombstoned per segment, or superseded in place for the old write-target copy of a stored point. Built in the same loop that decides tombstone-vs-supersede, so the report cannot drift from the writes. edge-shard-update logs one line per point after the applied summary, telling a fresh insert from an overwrite and naming the segments the old copies were deleted from. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Fable 5
|
06ffcb881f |
Add CachedBlobFile: cached reads + write-through appends for object stores (#10206)
* Add CachedBlobFile: cached reads + write-through appends for object stores Combine a DiskCache mirror (reads) with a BlobFile remote handle (appends) into CachedBlobFile/CachedBlobFs, the appendable universal-IO citizen for object stores. Appends perform the remote mutation inline and are durable at Ok: a native write-offset append in AppendMode::Native (with a soft limit on appends per object), or a whole-object rewrite in AppendMode::Rewrite for stores without native append. After a successful append the mirror length is advanced without extra IO; appended blocks fault in from the remote on first read. The multipart UploadPartCopy rewrite path (prefix >= 5 MiB) and the rewrite-required error classification are left as todo!() pending the AsyncRewrite backend capability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Backend-advertised AppendMethod; reactive appended-block cap recovery Replace CachedBlobFile's stored AppendMode with AsyncAppend::supported_append: the backend advertises Native or PartialUpload, and append takes a matching AppendRequest variant, rejecting the ones it does not support. The multipart UploadPartCopy todo moves into the S3 backend's PartialUpload arm. Drop the native_appends soft-limit counter: it is per-handle in-memory state that resets on every restart, so it can never be the correctness mechanism and persisting it would not make it authoritative either. The store is the authority: hitting its appended-block cap now surfaces as the new UniversalIoError::AppendRewriteRequired (S3 400 TooManyParts), and CachedBlobFile recovers with a whole-object rewrite. Unrecognized errors stay hard errors instead of silently triggering rewrites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Per-store append strategies; server-side rewrites for plain S3 and GCS Replace the single AppendContext struct with an enum of strategy objects, one per store capability, each owning its append logic: - NativeAppend: the signed write-offset PutObject (S3 Express, MinIO AiStor; AwsConfig::native_append declares it for AiStor-like endpoints, s3_express implies it). - PartCopyAppend: plain S3 — appends land as one atomic multipart rewrite whose prefix parts are server-side UploadPartCopy requests; nothing but the appended data crosses the network. object_store keeps such provider-specific calls out of its portable surface, so the requests are hand-signed like the native append. - ComposeAppend: GCS — the appended data is uploaded as a temporary neighbor object and composed onto the destination server-side, conditional on the observed generation (a real compare-and-swap). AppendMethod is replaced by AppendSupport, which tells the caller the only thing it needs: when the store takes a direct append. Always (native, and compose: no part minimums, no block cap), AboveThreshold (part-copy: the copied prefix lands as non-last multipart parts, >= 5 MiB each), or Never. CachedBlobFile drops its hardcoded MIN_COPY_PREFIX and rewrites locally only below the backend-advertised threshold; AppendRequest::Rewrite now means only "append and rebuild as a single blob" — the appended-block cap recovery. The append module is split one file per strategy, with a shared SignedRequestContext transport and a test-only HTTP stub. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * DiskCache tracks the remote object's etag Seeded from the new known_etag open extra (OpenExtra::with_known_etag), refreshed from FileInfo on schedule_reopen, and settable directly for callers that mutate the remote out of band. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Remove AppendRequest enum; appended-block cap recovery moves into the backend AsyncAppend::append takes plain (path, offset, data). A native S3 store that rejects an append with TooManyParts now falls back to the part-copy rewrite inside the dispatcher, instead of surfacing AppendRewriteRequired to CachedBlobFile for a second Rewrite request. The Rewrite variant was handled identically to Append everywhere except that one native path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Escalate to download+rewrite when the store rejects a part-copy rewrite The cap-recovery rewrite is chosen by the store's returned error, not a client-side threshold: a part-copy attempt rejected with EntityTooSmall (typed as UniversalIoError::AppendEntityTooSmall, parsed from the S3 error <Code>) falls back to downloading the sub-part-minimum prefix and PUTting the whole object back, guarded by a prefix-length offset check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix S3 Express appends: zonal endpoint + s3express SigV4 service Hand-issued appends targeted the standard endpoint and signed as "s3", so every append to a directory bucket got 404 NoSuchBucket, masked as AppendOffsetConflict by the 404 mapping. Derive the zonal {bucket}.s3express-{az}.{region} base from the mandatory --{az}--x-s3 bucket suffix (mirroring object_store's private derivation), carry the SigV4 service name in SignedRequestContext, and treat a 404 as a conflict only for NoSuchKey or bodiless responses — NoSuchBucket stays a loud error guarding the endpoint derivation. extract_xml_tag moves up to the context module and now tolerates tag attributes and pretty-printed bodies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Server-side etag precondition on appends; BlobFile loses UniversalAppend AsyncAppend::append carries an expected_etag that S3 part-copy rewrites attach as x-amz-copy-source-if-match (412 -> AppendEtagMismatch, a new typed error) and download_rewrite checks against the GET's own etag; native write-offset PUTs and GCS compose ignore it. BlobFile appends only through the inherent etag-aware append_bytes now — CachedBlobFile calls it directly with its DiskCache-tracked etag — and BlobFs's mutating ops become inherent, delegated from CachedBlobFs, per the standing TODOs. The append conformance battery runs over the CachedBlobFs stack, via new direct constructors that share one backend. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Drop unfulfilled too_many_arguments expectation rewrite_parts has exactly seven parameters — at the clippy threshold, not over it — so the lint never fires and the expect fails CI under -D unfulfilled-lint-expectations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ivan PleshkovandClaude Opus 5
|
86b9330628 |
transfer: send raw payloads, behind feature flags (#10066)
A raw point can carry its payload as the byte blob it is stored as, mirroring `PointStructRaw.raw_payload` on the internal gRPC API. The blob travels from the sending node into the receiving node's WAL untouched, so the sender never parses the payload it read and neither node builds a protobuf value tree for it. It is parsed exactly once, where the operation is unpacked for apply (`process_point_operation`), because that is the first place the parsed form is actually needed: `set_full_payload` goes through the payload index, which cannot be updated from bytes. The gRPC boundary therefore only checks the encoding tag and rejects a point that sets both payload fields, the way the enclosing request already rejects both `points` and `raw_points`. Moving the parse onto the apply path makes its error classification load-bearing, so a malformed blob is reported as `OperationError::MalformedPayloadBlob` — the payload sibling of `MalformedVectorBlob`, mapped to `CollectionError::BadInput` for the same reason: a bad blob that reached the WAL has to be skipped on replay instead of crash-looping recovery. Three consequences of the blob living that long are handled explicitly rather than by convention: - `decode_payload_raw` takes the blob only once it has parsed, so a failure leaves the point holding it instead of holding neither representation. - `upsert_points_raw` and `sync_points_raw` refuse a point that still carries a blob. They read the parsed payload, so such a point would otherwise be stored with no payload at all, and a `debug_assert!` would not catch it in release. - `is_equal_to` compares blob to stored blob as bytes. A differing encoding costs a redundant upsert on sync, never a skipped one. The `raw_payload_transfer` bench measures the trade, per 100-point batch (one transfer batch) at payloads of ~200 B / ~700 B / ~7 KB: - Sender, storage bytes to wire: 16x / 37x / 113x faster. This is where the whole win is — no parse of the blob that was read, no value tree built. - WAL encode: 5x / 11x / 25x faster, writing a byte string instead of a map. - Receiver, wire to applicable point: 1.09x / 1.10x / 1.06x. Near neutral, as it swaps walking a prost value tree for a JSON parse. - Wire bytes: ~6% smaller. WAL bytes: 10-32% *larger*, because the blob is JSON while a parsed payload is written as a compact CBOR map. The WAL growth is accepted rather than fixed: decoding earlier to win those bytes back costs a second full deserialization, and would leave the receiving side with a `payload_raw` that is never populated. Making the blob itself compact belongs in the payload storage encoding (`RawPayloadEncoding` is the extension point for it), not here. Two flags, both off by default and both sender-only (nodes accept raw points and raw payloads regardless), read where the transfer batch is prepared: - `transfer_raw_points` transfers every collection as raw points, not only those whose vector storage would drift in a decode-encode round-trip. - `transfer_raw_payloads` ships the blob a raw read hands out; without it the prepared batch decodes it back into the parsed payload, and the wire message is exactly what it is today. Neither is enabled by `all`: a node only accepts them once it runs a version that understands them, so they can only be switched on a release later. Nothing enforces that yet — the transfer has no peer-version gate. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Yash Singh
|
73504cc85f |
fix(common): use checked u32 conversion for simple_disk_cache block range (#10184)
to_block_range cast block indices with a raw as u32, silently truncating past 64 TiB and reading the wrong region. Mirror the sibling guard in disk_cache/cached_slice.rs and fail loudly with u32::try_from(...).expect. |
||
|
Luis Cossío
|
500eed65b1 | add etag to FileInfo (#10190) | ||
|
Andrey VasnetsovandClaude Fable 5
|
b43f70a6b6 |
[UpdateOnly] tombstone points in immutable segments via whole-mask rewrite (#10196)
* [UpdateOnly] tombstone points in immutable segments via whole-mask rewrite DeleteOnlySegment::tombstone_points marks the retired slots in the segment's deleted-points bitmask (id_tracker.deleted, shared by the immutable and disk-resident tracker formats) and replaces the file whole via atomic_save — the one mutation that works on backends without random-offset writes. Both read-only trackers already live-reload this file by opening a fresh handle and diffing, so the rewrite needs no read-side changes. The mutation cycle lives in StoredBitSlice::atomic_update: read the stored bits (or start from a caller-provided seed), apply the update, save atomically; a closure error writes nothing. The seed comes from the read phase by analogy to AppendableIdTrackerState: LookupSegment::writer_state now returns WriterIdTrackerState, whose DeleteOnly variant carries the deleted mask when the tracker already holds it in memory — always for the immutable tracker, only if materialized for the disk-resident one, which deliberately avoids loading the full deleted set. Tombstoning needs no more of the backend than reads plus atomic_save, so DeleteOnlySegment's bound drops to UniversalRead<Fs: UniversalWriteFileOps>. Unlike the writable trackers' drop(), the slot's version is not zeroed (the versions file is in-place-mutated, which object stores cannot do): deletion authority in these formats is the bit — every lookup filters through it — and a stale version on a tombstoned slot is the same state a crash between drop-bit and drop-version leaves, which fix_inconsistencies already absorbs as storage cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Close the temp-file handle in tests that atomically replace it NamedTempFile holds the file open for its lifetime, and Windows refuses the rename in atomic_save while any handle is open. into_temp_path() closes the handle and keeps the deletion guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Cossío
|
fb1d00ecc1 |
[LiveReload] impl LiveReload::live_preload for Blobstore (#10039)
* impl for Gridstore * impl for Logstore * fix path in `Tracker::preopen` * take non-mut `&self` * support UnchangedOpen in `live_reload` |
||
|
Luis Cossío
|
3a7d5781ec |
schedule_reopen takes &self (#10192)
|
||
|
Luis Cossío
|
8f5e83b13d |
[CachedFs] Unchanged file open is no-op (#10050)
* add `UniversalIoError::UnchangedOpen` * add & impl `CachedReadFs::reschedule_prefetch` * clippy * add `OkUnchanged` helper * propagate scheduling errors immediately * drop lock before reacquiring it * clear prefetched_files on new snapshot * only avoid prefetch on full FileInfo equality * `UnchangedOpen` maps to `Cancelled` * match-all match |
||
|
Andrey VasnetsovandClaude Fable 5
|
6633205704 |
[UpdateOnly] create Blobstore-backed storages append-only under a feature flag (#10154)
* [UpdateOnly] create Blobstore-backed storages append-only under a feature flag A new `append_only_storages` feature flag, enabled by `serverless_compatible`, switches every Blobstore creation site — the payload storage, the appendable field indexes (numeric, map, geo, full-text) and the sparse vector storage — to the append-only Logstore mode. One shared helper maps each site's Gridstore layout to its Logstore counterpart, carrying the page size and compression over; blocks and regions have no append-only equivalent. Only creation consults the flag: an existing storage keeps its persisted mode, both modes are always readable, so flipping the flag never strands data. Two changes make the flag usable rather than booby-trapped: `Logstore::delete_value` now succeeds trivially where nothing is stored, as mutable mode does, and errors only for a stored value. The ordinary write paths delete defensively — an index clears a slot before filling it, an empty value is stored as a deletion — and only ever hit occupied slots when something is genuinely mutated in place. A segment derives `append_only_storages` from the persisted payload storage mode when it opens — not from the flags, which may have changed since it was created — and it forces append-only mutation semantics on itself: every mutation clones to a fresh slot, and the same-operation slot-reuse shortcut is disabled, since the second step of a multi-step write would rewrite a payload row those storages cannot rewrite. The end-to-end test runs as its own binary (feature flags are process-global) with `serverless_compatible` on: the segment comes out holding Logstore storages, and upserts, updates of existing points, multi-step same-operation writes, deletes, an index build over existing points, a flush and a reload all run against them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] assume the flag pairing instead of deriving it, trim the docs Per review: `append_only_storages` without `append_only_mutations` is not a state to defend against — `init_feature_flags` forces the pairing, and the same-operation slot-reuse check reads the flag directly. That deletes the segment-side derivation: the `append_only_storages` segment field, the persisted-mode read at open, and the `is_append_only` accessor chain through `Blobstore`, `PayloadStorageImpl` and `PayloadStorageEnum`. Docstrings and comments trimmed to the guarantees; how the write paths use them is their own business. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] restructure the creation config around mode-neutral options `CreateOptions` in the blobstore crate holds what a caller actually decides — page size, block size, compression — and `into_config(append_only)` turns them into the config of either mode, each taking the fields it can express. The segment-side `storage_config` supplies only the mode, from the feature flag. That removes the misnomer chain the previous cut left behind: nothing named gridstore returns a config that might not be one, and no call site builds a `GridstoreConfig` just to have its fields repacked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [UpdateOnly] keep Logstore strict; fix the callers that deleted nothing Per review, `Logstore::delete_value` goes back to an unconditional error: a delete reaching an append-only storage is a caller bug to fix, not a case to absorb. The callers that issued vacuous deletes are fixed instead: - The numeric and geo indexes only delete from the storage when their in-memory index actually held values at the slot — the two are written in lockstep, so an empty slot has nothing stored either. The map and text indexes already worked this way. - The sparse storage skips the delete for keys at or past its end, where nothing was ever stored. Each removed call was wasted work in mutable mode too. The e2e test now also drives a numeric index and the sparse storage against append-only mode, and asserts that deleting a stored sparse vector fails. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [UpdateOnly] drop the same-op slot reuse; upserts write the whole point at once The append-only path never needs a multi-step point write: the one real multi-stepper was the shard's upsert — `upsert_point` followed by a payload step under one operation number — and it now goes through `upsert_moved_point`, which writes vectors and payload as one operation and one slot. With that, the same-operation slot-reuse carve-out in `handle_point_mutate` has nothing to carry: on an append-only segment every mutating step clones to a fresh slot, unconditionally, and the `append_only_storages` special case disappears with it. The version gate skips only on strictly newer versions, so a caller that still multi-steps stays correct — it pays a slot per step. `PointToUpsert` now exposes the point's parts — raw vectors, decoded vectors, payload — and both write paths are provided from them: `upsert_into` hands the parts to `upsert_moved_point`, `write_moved` adapts them to the copy-on-write move callback. The two hand-written `upsert_into` bodies and the follow-up payload helper are gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Regenerate OpenAPI for the `append_only_storages` feature flag Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * one extra debug assertion * fmt --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dependabot[bot]
|
f206bbca68 |
build(deps): bump serial_test from 3.5.0 to 4.0.1 (#10169)
Bumps [serial_test](https://github.com/palfrey/serial_test) from 3.5.0 to 4.0.1. - [Release notes](https://github.com/palfrey/serial_test/releases) - [Commits](https://github.com/palfrey/serial_test/compare/v3.5.0...v4.0.1) --- updated-dependencies: - dependency-name: serial_test dependency-version: 4.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Shruti Sharma
|
911d6a2afa | perf(common): add zero copy fast path for multi-block disk cache hits (#10108) | ||
|
|
b60f298dea |
Add UpdateOnlyAppendableIdTracker, the append-only ID tracker writer (#10093)
* Add UpdateOnlyAppendableIdTracker, the append-only writer The write counterpart of `ReadOnlyAppendableIdTracker`, producing the two files that tracker already consumes — `mutable_id_tracker.mappings`, an append-only log of mapping changes, and `mutable_id_tracker.versions`, a dense array of one version per slot — through `UniversalAppend`, so the same code drives a local file and an object store. `insert_operations` records a batch of `MappingOperation`s in order: an insert claims the next slot above the highest one in use and reports it, a delete retires an external id and claims nothing. Nothing is rewritten in place, so re-inserting a live id moves it to a fresh slot and supersedes the old one — the update-only shape of an update. `set_internal_versions` extends the versions array. Ids may come in any order but must be exactly the slots the array does not cover yet: a slot below the end would need an in-place overwrite, and a hole would have to be zero-filled — and since "covered by the versions file" *is* the commit signal for readers, that would publish a slot as a live point of version 0 before its data exists. Both are rejected rather than written. Both methods append at an offset they probed for, never at an implicit end: the offset is a compare-and-swap token, so a file that has moved on since the probe is rejected instead of being written twice or in the wrong place. Both have persisted what they wrote when they return `Ok` — append, then run the handle's flusher — and nothing is buffered across calls. The order of the two calls, claim the slot then commit the version, is what makes a crash in between safe: readers ignore slots the versions array does not cover. Cleaning up the slots such a crash abandons is left to the opener, along with repairing a torn tail; the writer fails loudly rather than guessing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Share the versions file format between both write paths `set_internal_versions` was reimplementing what `versions_storage` already knew: entries are `SeqNumberType`-sized, slot `n` lives at `n * VERSION_ELEMENT_SIZE`, a file length that is not a whole number of entries has a torn tail. Every one of those facts existed in two to four places, spelled as inline `/`, `%` and `write_u64::<FileEndianess>`, so the append-only writer could drift away from the in-place one silently. Move them into `versions_storage`, which now owns the format for both writers and both readers: - `write_version` / `read_version`, the entry codec, with a static assertion tying its `u64` to `VERSION_ELEMENT_SIZE` so a change to `SeqNumberType` cannot silently shrink every offset; - `version_offset` and `versions_byte_len`, the slot arithmetic; - `VersionsLayout`, which splits a file length into committed entries and a partial tail. The two writers still react differently — the in-place one truncates the tail, the append-only one refuses it, because an append cannot — but they no longer each work out what the tail is. `store_version_changes`, `load_versions`, `set_internal_versions` and the read-only tracker's live reload all go through it. The write loops themselves stay separate: one seeks to sparse offsets, the other emits a validated consecutive run, and merging them would obscure both. What they share is where the bytes go, which is the part that must not diverge — and a new test pins it down by writing the same versions through both writers and comparing the files byte for byte. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Append mapping and version entries as batches Both writers built one concatenated buffer and handed it to `append`. `append_batch` takes the entries as separate buffers and places them in a single operation — a vectored write locally, one request on an object store — so the entry boundaries reach the backend instead of being flattened away first. Versions are fixed-size, so the entries are the payload's `chunks_exact`. Mapping changes are variable-length, so their bounds are recorded as they are serialized. Both keep the compare-and-swap offset, which `append_batch` validates the same way `append` does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test the writer against MutableIdTracker end to end The suite leaned on round-trips through the storage loaders, which only restated what the writer had just written. Four such tests are replaced by one that checks the property actually worth having: drive the append-only writer and `MutableIdTracker` with the same points, versions and deletes, open each segment through `ReadOnlyAppendableIdTracker`, and require the two views to be indistinguishable — counts, deleted state, external ids, live points' versions, and id resolution. Versions are compared for live points only. `MutableIdTracker::drop` overwrites the slot with `DELETED_POINT_VERSION`, which an append cannot do, so the append-only writer leaves the point's original version there; neither is observable for a point that is gone. The remaining tests keep what a round-trip cannot show: slot allocation across calls, instances and deletes (three tests folded into one), the rejection of holes and rewrites, and the byte-for-byte agreement of the two writers on the versions file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * better buffer names * fmt * Heal a torn versions tail instead of refusing to write A writer that dies mid-entry leaves the versions file ending inside a slot. The in-place writer already truncated that tail before writing; the append-only writer refused, which left the file unwritable forever since an append cannot truncate. Share the decision — what counts as torn, the healthy length, the warning — in `heal_versions_tail`, and let each writer supply the shrink its backend can do: `set_len` in place, or reading the committed prefix back and putting it in place as a whole file where there is no truncate. Dropping the tail loses nothing: the array covers a slot only once its whole entry is there, so a partial entry belongs to a slot no reader ever saw and no writer counted as committed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Append mappings at the end of the log, not the end of the file Mapping entries vary in length, so no length tells you whether the log ends on an entry boundary. Appending at the file's end therefore could not fail: a torn entry was silently appended after, and every entry from there on was framed off the stray bytes. Carry the boundary instead. `new` takes the offset just past the last complete entry — `ReadOnlyAppendableIdTracker::mappings_read_to`, from the same view that supplies `max_internal_id` — and appends there, which turns a file ending elsewhere into an append offset conflict. On that conflict `heal_mappings` cuts the file back to the log's end, the same read-prefix-and-rewrite the versions file heals with, and writes the batch again. A torn entry and a batch that landed unacknowledged are indistinguishable without parsing, and need not be told apart: neither `max_internal_id` nor `mappings_end` moves before an append is durable, so the retry writes the same bytes at the same offset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move the healing functions into their own file `update_only/mod.rs` had grown to hold the writer, its two public write paths and the two repair routines they fall back on. Split the latter out: `heal_versions` and `heal_mappings` move verbatim into `update_only/heal.rs`, as a second impl block, following the layout the read-only half already uses (`lifecycle.rs`, `live_reload.rs`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Retire inherited pending inserts when opening the writer A slot is spoken for from the moment the mappings log claims it — components may already have written data at it — so the writer must resume above every slot the log ever handed out. Deriving that bound from a reader's point set undercounts it twice over: a claimed slot whose version was never committed is not in the mapping, and one whose external id was deleted afterwards is not among the pending inserts either. Track it in the log instead, as `ReadOnlyAppendableIdTracker::max_claimed_internal_id`, bumped on every insert entry regardless of what becomes of the point, and take it as `UpdateOnlyAppendableIdTracker::new`'s bound. The points on those claimed-but-unversioned slots are the other half. They cannot be adopted: a writer stopped partway through them, so some components hold their data and others do not, and which is unknowable here. They cannot be left alone either, the versions array being dense — covering any slot above one of them publishes it, half-written. So `new` now takes the pending inserts explicitly and retires them, recording a `Delete` per id before the writer can be used at all, which is what makes it fallible. Doing it at construction rather than lazily on the first write means no write path can be added later that forgets to. `set_internal_versions` accordingly stops rejecting holes: it writes the whole run from the end of the array through the highest id given, covering skipped slots with `DELETED_POINT_VERSION` as the in-place writer's seek already does. It gains an upper bound in exchange — publishing a slot means covering every slot below it, so an id the log never claimed is refused. Live-reload semantics are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Skip UpdateOnlyAppendableIdTracker heal tests on Windows Healing replaces the file via atomic_save while an mmap handle is still open, which Windows denies with os error 5. * Drop mmap handles before healing ID tracker files atomic_save cannot replace a path while an mmap is still open on Windows. Copy the committed prefix, drop the handle, then rewrite and reopen. * Trim ID tracker docs and drop redundant helpers Condense the doc comments on the update-only tracker to the style of the sibling modules, merge a duplicate impl block, and remove `read_version` and a debug assert that restates its own operands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop the versions layout helpers and inline the arithmetic VersionsLayout, versions_byte_len and version_offset wrapped one divmod and one multiplication between them, and adopting the struct made the live-reload hunk longer than the line it replaced. Compute the committed length where it is needed instead, and let heal_versions_tail and heal_versions return unit, since no caller used the layout they handed back. Also drop the writer's unused max_claimed_internal_id accessor, and fold retires_inherited_pending_inserts_at_construction into retires_inherited_pending_inserts: the merged test asserts the retirement happened before the writer did anything, and commits a real version to the retired slot rather than letting it take the filler, so it still shows the Delete is what hides the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: qdrant-cloud-bot <111755117+qdrant-cloud-bot@users.noreply.github.com> |
||
|
Jojii
|
7c0948628d |
Batched deletion checks in full-scan/exact search (10-35% faster Turbo4) (#10042)
* Batched bitmap checks in full-scan/exact search Instead of walking the deleted bitmap bit by bit and re-checking every point, the new peek_top_visible reads all bitmaps one u64 at a time and handles 64 points in one step. About 1.5x QPS on unfiltered full scans. * Remove debug statement * Extract parallel bitmap scaning into separate helper * Optimizing `BatchedBitmapScan` * Also apply to HNSW full-scan fallback |
||
|
Andrey VasnetsovandClaude Opus 5
|
908cd2f10a |
[UIO] Pin the append filesystem to its handle (#10092)
`UniversalRead::Fs` was reachable from an append handle and known to be
a `UniversalWriteFileOps`, but not pinned: `fs.open_append(..)` on it
returned some `Fs::AppendFile`, not the handle type in hand. Pin it the
way the read side is pinned, so `S::Fs` both opens `S` for reading and
hands `S` out as its append handle:
UniversalRead<Fs: UniversalWriteFileOps<AppendFile = Self>>
Every append handle's canonical filesystem already produced itself
(`MmapFs → MmapFile`, `IoUringFs → IoUringFile`, `BlobFs<A> →
BlobFile<A>`), so this only writes down what held — the workspace
compiles unchanged.
Generic-over-`<S: UniversalAppend>` code now reaches its file-creating,
append-opening filesystem as `S::Fs` with no second associated type, and
`&impl UniversalWriteFileOps<AppendFile = S>` accepts any other producer
— the mirror of `&impl UniversalReadFs<File = S>`.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Luis Cossío
|
51bf1a6823 |
[DiskCache] No-growth reopen is no-op (#10049)
* add `ScheduleReopen::Unchanged` to avoid retrying to reopen blockingly * wrap `ScheduledReopen` in `Option` |
||
|
Tim Visée
|
20391cac08 | Bump dev version to 1.19.1-dev (#10097) | ||
|
Andrey VasnetsovandClaude Opus 5
|
090d2d6d10 |
[UIO] Open append handles through UniversalWriteFileOps (#10091)
A file handle that appends was only reachable through
`UniversalReadFs::open` with `writeable: true`, which ties the append
capability to the backend's read handle type. Give the write-side
filesystem trait its own opening path instead:
type AppendFile: UniversalAppend;
fn open_append(&self, path, options) -> UioResult<Self::AppendFile>;
`AppendFile` is deliberately not tied to `UniversalReadFs::File`: the two
capabilities live on independent traits, so a backend may serve reads
through one handle type and appends through another, and a filesystem
that opens no read handles at all still names an append handle. For the
same reason there is no `OpenExtra` parameter — the append handle may
come from a different backend, whose per-open knobs would not apply.
`OpenOptions::for_append` forces `writeable` on, since a read-only
append handle is a contradiction rather than an error worth propagating.
Drop the `UniversalWriteFileOps` impls on the two disk caches first.
Both were vestigial: `DiskCacheFs` got its forwarding-to-remote impl
mechanically in the read/write trait split (#9682) and no caller ever
used it, and `BlockCacheFs` lives in a module that is dead code. Neither
cache can open a writeable file, so neither can produce an append
handle; mutations go straight to the backing storage. An
`assert_not_impl_any!` locks this in next to the existing one on
`DiskCache`.
`BlobFs` consequently requires `AsyncAppend` rather than `AsyncWrite`:
only a backend with a native single-request append can hand out an
append handle. Object stores that can just put whole objects (GCS,
Azure) stay read-only through universal I/O; nothing used their write
side.
The conformance suite gains `run_open_append_conformance`, run by
`run_append_conformance` and public for filesystems whose own `File`
does not append. It covers `MmapFs`, `IoUringFs` and `BlobFs` over the
in-memory object store.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Andrey VasnetsovandClaude Opus 5
|
aa6c5d8403 |
Global quota API (#10035)
* feat: global quota API Memory and disk are node-wide resources, so configuring their thresholds per collection through strict mode makes little sense. Move them behind a single cluster-wide `QuotaManager`. The quota config is seeded from `storage.quotas` in the settings (and so from env vars), overridden by `quota.json` in the storage directory, and updated cluster-wide through a new `SetQuotaConfig` consensus operation which rewrites that file on every peer. Raft snapshots carry it too, so a peer that joins by snapshot picks it up. Quotas are enforced wherever the strict mode memory and disk checks used to run, but no longer gated behind `strict_mode.enabled`: a value set in an enabled strict mode config still wins per resource, the quota is the default. Rejections name both the condition that tripped and the config that governs it. `GET /quotas` reports the config plus current utilization to global read users; `PUT /quotas` replaces it for global manage users. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: cover the quota endpoints in the API consistency checks `test_all_rest_endpoints_are_covered` and the OpenAPI endpoint count both break on any new REST endpoint. Add `GET`/`PUT /quotas` to `ACTION_ACCESS` with their JWT access tests, and bump the expected API count. The quota endpoints stay out of `REST_ENDPOINT_WHITELIST`: that list is for data-plane endpoints reported per-endpoint in metrics. Also add a Raft snapshot CBOR compatibility test — snapshots are exchanged between peers of different versions during a rolling upgrade, so `quota_config` must be absent-tolerant in both directions. Review feedback: persist through `SaveOnDisk`, which already implements the write-before-swap protocol this was doing by hand; validate the config at both persistence boundaries, since a hand-edited quota file or a config arriving through consensus does not pass the REST handler's validation, and a `0%` limit would reject every update forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: assert seeding a quota from invalid settings persists nothing Follow-up to review feedback claiming `SaveOnDisk::load_or_init` writes the init value before it is validated. It does not — only `SaveOnDisk::new` persists — but the property matters: were seeding to persist first, invalid settings would leave a `quota.json` that fails validation on every subsequent start, and the node could only be recovered by deleting it by hand. Pin it down with a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: make QuotaManager the single reader of memory and disk The quota checks measured memory and disk themselves, while the optimizer and the WAL disk watcher each called `fs4::available_space` behind their own ad-hoc caches. Fold all of it into QuotaManager: it owns the readings, the freshness policy, and the limits they are compared against. Moves the module to `lib/shard`, since the optimizer sits below `storage` and has to reach it; `storage::quota` re-exports it, so consensus, the `/quotas` API and StorageConfig are unchanged. The manager is installed as a process singleton by TableOfContent, ahead of loading any collection. - Callers hand in QuotaLimits overrides instead of a StrictModeConfig, and an override can now only tighten. A collection-level admin could raise `max_disk_usage_percent` past a cluster-wide limit that needed global manage rights to set; ties resolve to the quota so the rejection names the knob that actually has to change. - Measurements are cached for 5s, but a reading at or above its limit is never reused: a rejected client retries, and freeing the resource has to take effect on the next request rather than a TTL later. - `fits_on_disk` sizes an optimization against physical free space only, never the configured limits. Optimizations are what free a full disk, so the quota must not be what stops one. - `percent_of` widens to u128 instead of saturating the multiply, which under-reported utilization (failing open) above ~184 PB. - StorageConfig::quotas is optional; absent means no quota is enforced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: don't recover dead replicas onto a node at a resource limit Recovering a dead replica pulls a whole copy of its shard onto this node. If it is already at its memory or disk quota that transfer cannot finish, and starting it only pushes the node further past the limit. Skip it and reconsider on a later sync, once the resource frees up. Adds QuotaManager::check_capacity for work that lands bytes here without being an update. Unlike fits_on_disk the configured limits do apply: taking on a replica is not what frees a full node, so there is no deadlock to avoid by letting it through. The check is hoisted out of the per-shard loop because a node over its limit re-measures on every call, so checking per dead shard would cost a statvfs each. It is free when no quota is configured. Also trims the comments across the quota module, which had grown well past what the code needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: drop trivial and duplicated quota tests Six tests removed, ~140 lines, with no loss of coverage: - a_rejection_names_the_knob_that_has_to_change asserted that a format! contains its own literals; the message is covered end-to-end by the override test and by test_global_quota.py. - a_node_over_its_quota_has_no_capacity_to_take_on_a_replica was 30 lines for check_capacity, a one-line delegation to check_update the test above it already calls. - a_rejecting_measurement_is_never_served_from_the_cache duplicated the meter test, which proves the same rule with an injected reader instead of inferring it from the real filesystem. - free_space_is_reported_without_enforcing_anything covered a one-line accessor, and its point is what the fits_on_disk test is for. - The two resolve tests and the three meter tests each collapse into one. DiskFit::Unknown keeps its coverage as two lines inside the fits_on_disk test rather than its own fixture. The snapshot compat pair becomes one test: the second only asserted cluster_metadata.is_empty(), which says nothing about quotas — the real check was the deserialize, now an expect that states it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: re-measure free space as the disk fills, and drop a Windows-only assert Two CI failures, both from this branch. e2e test_low_disk: the DiskUsageWatcher I replaced escalated to checking on every call once free space fell below 512 MB. Folding it into the quota manager lost that — available_bytes passed no limit, so a reading was reused for the full 5s however little space was left. On a disk filling as fast as that test fills it, 5s blind is enough to actually run out and the WAL write dies instead of returning "No space left on device". available_bytes now takes a watch_below level and never reuses a reading under it, which is what the old ladder was expressing. The watcher passes max(min_free, 512 MB), so the escalation point is back; above it the 5s cache still costs fewer syscalls than the old 128-call ladder. fits_on_disk gets the same rule by passing required_bytes, so a merge that does not fit re-checks rather than sitting on a stale sample. Windows: fits_on_disk on a missing path was asserted to be Unknown, but GetDiskFreeSpaceEx resolves up to the containing drive and succeeds — as common::disk_usage's own test documents. Dropped; the branch is a two-line else and is not portably reachable. Also renames an_optimization_is_sized_against_the_disk_not_the_quota, which needed explaining to be understood. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: report the quota config in telemetry Reads it from the quota manager rather than the settings, so it is the config the node is actually enforcing: a peer that missed a consensus update reports what it is applying, not what the cluster agreed on. Gated on global access, the same access `GET /quotas` requires, and left out of `PeerTelemetry` — a quota is per-node state, so each peer reports its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: regenerate OpenAPI, and cover the quota in the telemetry key sets Two CI failures from the previous commit. Referencing QuotaConfig from TelemetryData moves its definition earlier in `components/schemas`, because TelemetryData is generated ahead of QuotaStatus. Regenerated rather than hand-patched, so the schema is a pure move. test_telemetry_detail asserts the exact set of top-level telemetry keys. The quota is reported at every level, including 0 — it is three scalars, it is the default the endpoint serves, and it is what explains an update being rejected — so both key sets gain it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: remove `max_disk_usage_percent` from strict mode Disk is a node-wide resource, so a per-collection percentage of it never meant anything a caller could act on: the limit describes how full the *node* is, and which collection the write happens to target has nothing to do with it. The global quota is where it belongs. It shipped in 1.18.2 without documentation, so this drops it outright rather than deprecating. Removal is soft in every direction: StrictModeConfig has no `deny_unknown_fields`, so a client still sending it gets it ignored rather than a 400, and the same struct deserializes the persisted collection config, so collections created on 1.18.2+ keep loading. Proto field 22 is reserved so the number is never reused. The e2e test becomes a quota test — the fixture and the timing are the interesting parts and they carry over unchanged; only how the threshold is configured differs. `max_resident_memory_percent` was documented and stays for now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: enforce the strict mode memory limit outside the quota `max_resident_memory_percent` was folded into the quota as an override, which meant the quota check had to know about strict mode, and retiring the setting would mean unpicking `EffectiveLimit` and `LimitSource` from the resolution logic. It is now a check of its own in `verification/mod.rs`, next to the strict mode checks it belongs with, borrowing only the measurement from the quota manager — which stays the node's single reader of process memory, so both checks still share one reading. Deleting the setting later is deleting one function and its one caller. `QuotaManager::check_update` takes no arguments and consults the quota alone. A collection can still only tighten the limit for itself, because its own check runs in addition rather than in place of the quota's, and each rejection now names the config that has to change without having to carry a `LimitSource` to say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: enforce the quota on the update path, not in strict mode The quota check sat inside `check_strict_mode_toc_batch` only because that was the one place holding the collection's strict mode config. It doesn't need one any more, and the placement had a real cost: coverage depended on each handler remembering to ask for a strict mode check, and four of the internal update RPCs do — `sync_internal`, which moves the most bytes onto a node, does not. It now runs in `Collection::update_from_client` and `update_from_peer`, which every update passes through. `update_from_client` checks ahead of the shard split, so an operation is accepted or refused whole rather than landing on some shards and being refused by others. Classification moves with it, from ~10 `consumes_memory` impls on request DTOs to one exhaustive `CollectionUpdateOperations::consumes_quota`. The internal enum has variants — raw upserts, conditional upserts, the syncs — that have no client-facing request type, so per-DTO impls structurally could not classify them. Shard-transfer syncs stay excluded, as they are today: a transfer is sized up once before it starts, and refusing its batches partway abandons work that is nearly done only for it to restart from the beginning. Index and named-vector creation reach shards through consensus, past this check — a peer must not refuse what the cluster agreed to — so they keep their pre-consensus check, now against the quota manager directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: deprecate `max_resident_memory_percent` in strict mode Same reason the disk threshold went: memory is node-wide, so a per-collection percentage of it caps how full the *node* is, which has nothing to do with which collection is being written to. The node-wide quota caps it once for everything. Unlike the disk threshold this one shipped documented, in 1.18.0, so it keeps working — as a limit a collection can tighten for itself, never lift — and gets the usual markers: `#[deprecated]` on both Rust structs, `[deprecated = true]` on proto field 21, and `deprecated: true` in the OpenAPI schema, which schemars derives from the attribute. The note names 1.21 as the removal. Recording a version matters here: the audit in docs/plans/overdue-deprecations.md found that this repo has never written a removal deadline down, and members of the 1.15.0 deprecation batch are still in tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: reconcile the quota readers with #9891 #9891 landed effective (cgroup) figures in telemetry while this branch was making QuotaManager the single reader of memory and disk. Two collisions, neither of which git sees. `segment::utils::mem::total_memory_bytes` is now a shared accessor with a 5s TTL, so a cgroup resize is picked up. The quota module had its own `OnceLock` copy that froze the value at startup — exactly what #9891 set out to fix — so it delegates to the shared one instead. Telemetry's new `disk_size` called `common::disk_usage::disk_usage` directly. That reader lost its TTL cache on this branch when the caching moved into the quota manager's meter, so it would have taken an uncached `statvfs` on every telemetry request, and it put a second disk reader back in the tree. It goes through `QuotaManager::disk_capacity_bytes` now, sharing the reading the quota check already takes. Verified it still reports the storage filesystem, matching `df`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: split the quota manager by what each half does `manager.rs` had grown to 450 lines holding three separate jobs: owning the config and its file, taking the readings, and comparing one against the other. - `manager/store.rs` — the `Store` enum, `QUOTA_CONFIG_FILE`, and config validation, which is now the store's own business rather than something every caller has to remember to do first. - `manager/measure.rs` — every reading, and `DiskFit`. The "nothing else calls `statvfs` or reads process RSS" claim is now checkable by looking at one file. - `manager/enforce.rs` — `check_update` / `check_capacity` and the threshold comparison. - `manager/mod.rs` — the struct, its construction, and the config accessors: what a reader needs to see first. Tests move with their subject. No behaviour change: `set_config` used to validate before delegating to the store, and now the store validates on write, which is the same order of operations from the outside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: count copy-on-write deletes toward the quota Dropping a vector or a payload key does not free anything on its own: copy-on-write rewrites the point to produce the version without that field, so storage grows first and is only reclaimed once the optimizer gets to it. Gating those as if they were reclaiming space let a full node keep taking writes that make it fuller. Deleting whole points stays exempt. That is the one operation that has to work on a node at its limit, or there is no way back under it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: say that the quota's reported usage is per node `GET /quotas` returns one cluster-wide config and one set of utilization figures, which reads as though both describe the cluster. They do not: memory and disk are node-local, so `usage` is whatever the peer that served the request is seeing, and a peer under its limit says nothing about the others. Also corrects `resident_memory_percent`, which claimed to be a share of total system memory. It is a share of the memory available to the process, which under a cgroup is the limit rather than the host's RAM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: treat a node over its quota as a failed replica, not a bad request A quota rejection described the request as invalid (400) and was classified non-transient, which is how the replica set recognises errors that every replica would produce alike. A quota is the opposite: the input is fine and the answer depends on which machine you ask. On the default `wait=false` path that combination silently dropped the write — `update.rs` only deactivates transient failures when nothing completed — leaving the replica Active and permanently missing data its co-replicas had. It is now `InsufficientStorage`, transient, HTTP 507 / gRPC `ResourceExhausted`. So a node that is out of room is handled like one that is offline: - last active replica, or every replica over quota: nothing could take the write, and the client is told the cluster is out of room. - more than one replica: the full node is deactivated through the same path a dead peer takes, and the update stands if enough replicas accepted it. `check_capacity` already keeps recovery off that node until it has room. The check also moves off `update_from_client`, which applied the coordinator's own limit to the whole operation even when it held no replica of the shards being written. Each replica set now gates its own local write and records the refusal as a failure of this peer, so a node only ever answers for itself. `ResourceExhausted` is shared with rate limiting, and the reverse conversion mapped it straight to `RateLimitExceeded` — a forwarded rejection came back as 429. Statuses now carry a marker so the two stay distinguishable across the wire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: report quota pressure per node, and across the cluster A quota is node-local, so finding out which node has hit one meant asking each of them in turn — and nothing at all showed up in monitoring. `/metrics` gains a `quota_exceeded` gauge for the local node. It is emitted only while the quota is enabled: with it off the value would be a constant 0 that says nothing about the node, and an alert built on it would go quiet rather than fire if someone disabled the quota. Telemetry's `quota` field carries the same verdict alongside the config, since that is where the metric is derived from. `GET /quotas` now answers for the whole cluster. A new `GetQuotaUsage` RPC on the internal `QdrantInternal` service returns what one peer is using, and the handler fans it out to every known peer in parallel. Peers that do not answer are left out rather than failing the request — the nodes that are out of room are exactly the ones most likely to time out, and a partial answer still names them. Outside distributed mode the field is absent rather than a map of one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: report the quota metric per resource `quota_exceeded` was one flag for the whole node, which does not say what to go and fix — disk is freed by deleting or optimizing, memory by unloading. It now carries a `resource` label: quota_exceeded{resource="memory"} 0 quota_exceeded{resource="disk"} 1 A resource with no limit gets no series at all, for the same reason the metric is absent while the quota is disabled: a series that can never reach 1 reads as healthy and would quietly carry an alert that cannot fire. `QuotaManager::exceeded` returns the per-resource verdict, with `None` for a resource this node does not cap. Telemetry reports the same breakdown, since the metric is derived from it. The peer usage RPC keeps a single flag — it sits next to both percentages, so it only has to answer "is this peer refusing writes". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: drop a no-op error conversion the linter caught `check_global_access` already returns a `StorageError`, so mapping it through `StorageError::from` converted the type to itself and tripped `clippy::useless_conversion`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: hold a tripped quota until usage clears a release margin A resource resting on its limit crosses it in both directions on the noise between two readings, and each crossing is expensive: the node refuses a write, its replica is deactivated, usage dips, recovery starts sending a whole shard copy back, and the arriving data pushes it over again. The loop sustains itself, and every lap costs a shard transfer. A limit now trips at its configured value but only clears once usage has fallen 5 percentage points below it, so the crossing has to be real. The margin is floored at 1%, since a limit smaller than the margin would otherwise be impossible to fall back under and would strand the node. The verdict is carried on the manager rather than recomputed, which makes it the thing reporting shows: expect `exceeded` to be set while the utilization next to it is already back under the limit. Rejections say so too, rather than claiming a limit that is no longer exceeded: Disk usage is at 87% of total capacity. It reached the configured limit of 90% and has to fall below 85% before this node takes writes again. Changing the config clears the verdicts. New limits are a deliberate act, and should not be held back by the margin of a limit that no longer exists. Both resources are now evaluated on every check instead of stopping at the first failure, so a verdict is never left behind reporting a reading that has since been superseded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: make the quota release margin configurable 5 points is a guess about how noisy a deployment's usage is, which is not something one number can be right about: a node whose disk moves in gigabyte steps needs a wider margin than one that creeps, and an operator who wants the old flip-on-every-reading behaviour should be able to ask for it. `release_margin_percent` joins the rest of the quota config, so it seeds from `QDRANT__STORAGE__QUOTAS__RELEASE_MARGIN_PERCENT`, replicates through consensus, and changes with `PUT /quotas`. Defaults to 5 and is filled in when a request omits it, so it always answers with the margin actually in force rather than leaving the caller to assume one. `0` releases as soon as usage is back under the limit. `QuotaConfig` grows a hand-written `Default` for it, since deriving one would have quietly defaulted the margin to 0 and disabled the hysteresis for anyone constructing a config in code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: leave the release margin unset by default, and hold verdicts in atomics `release_margin_percent` is `null` unless someone sets it, rather than materialising 5 into every config. A quota written today then does not pin a number a later release may want to revise, and `{"enabled": false}` still round-trips as itself. `QuotaConfig::limits` resolves it, next to `enabled`, so enforcement never sees the unset case. The verdicts move from a `Mutex<QuotaExceeded>` to one `AtomicBool` per resource. They are judged independently and nothing reads them as a pair, so the lock only added contention to the path every update takes; a verdict that races a concurrent check is re-decided by the next one from a fresh reading. That also drops the tri-state. Only "was this over its limit" has to survive between checks — whether a resource is enforced at all follows from the config and the reading, so it is derived when reporting rather than stored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: drop a comment arguing with a design that was never here Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |