* perf: skip retrieval in scroll when no payload or vectors are requested
Every scroll variant went through SegmentsSearcher::retrieve to build its
records, even when neither payload nor vectors were asked for. That is a
has_point lookup per id per segment plus a version and id resolution per
hit, only to yield records holding nothing but the id. The universal query
API always scrolls this way and fetches payload separately afterwards.
Build the bare records from the ids directly in that case. The retrieve
could only have dropped ids deleted in between, which the update lock held
across the scroll rules out.
* perf: fetch payload and vectors in the leaf of plain query requests
A query without prefetches and without rescoring is served by a single
leaf search or scroll whose result is returned as is. The planner still
built that leaf without payload or vectors and filled them in afterwards
through SegmentsSearcher::retrieve, which resolves every result id in
every segment again: the same cost #10312 removed from the search API,
paid once more at the end of each query.
Let the leaf carry the requested payload and vectors instead, so the
segment attaches them to the results it already holds by offset, and
clear the root plan so the fill step is skipped. Prefetch leaves and
rescored roots (MMR) are unchanged. As with the search API, this fetches
payload for each segment's candidates rather than for the merged top
`limit` alone.
* Use new_empty function
* fix: fetch payload and vectors in scroll leaves only (#10384)
A search leaf hydrates every segment's local top-k before merging, so
`with_payload` there multiplies payload I/O by the segment count — the
regression #6279 fixed and `test_payload_io_read_is_within_limit[query]`
guards. Scroll leaves retrieve once for the merged page, so they keep
fetching directly; search leaves stay bare and the root plan retrieves
for the final result.
Claude-Session: https://claude.ai/code/session_01SUWh5PqUeSefrUqUwXxU3E
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Add repro test for Gridstore stale-gaps allocation panic
The region gaps (gaps.dat) are an acceleration structure derived from
the bitmask (bitmask.dat), persisted to a separate file without
ordering guarantees. After an unclean shutdown (power loss, kernel
crash) the gaps can claim free space where the bitmask has the blocks
marked used. An allocation in that state panics with "New page has
just been created", seen in production during WAL replay on startup.
This test simulates the torn state and expects it to be recovered; it
fails with that panic until the next commit.
* Rebuild Gridstore region gaps once on detected inconsistency
Offsets returned by the block search always come from scanning the
bitmask itself; the region gaps only steer where to look. Stale gaps
can therefore only cause missed allocations, never a wrong allocation:
every torn state funnels into the allocation failure that used to
panic with "New page has just been created".
Instead of paying for gaps validation on every open, detect the
inconsistency at that failure point, log a warning, rebuild the gaps
from the bitmask (repairing content and length), and retry. This is
allowed at most once per instance: after a rebuild the gaps are kept
consistent in memory, so a second failure would be a logic bug and
still panics. Also clamp proposed search windows to the bitmask length
so a length-diverged gaps file reaches the recoverable path instead of
an out-of-bounds panic.
* Fix typo
* Add repro test for gaps length divergence breaking page creation
BitmaskGaps::extend grows the file with zeroes before writing the new
all-free entries through the mmap. After an unclean shutdown the growth
can be persisted while the entry contents are lost, leaving phantom
all-zero entries beyond the bitmask, each claiming a full region.
Phantom full entries are invisible to the gap search, but they force
trailing_free_blocks to report zero, so the next allocation always
tries to create a new page and cover_new_page panics on its "Bitmask
length mismatch" assertion — before the lazy gaps rebuild from the
previous commit can detect anything.
The test expects opening the storage to repair the divergence; it
fails with that panic until the next commit.
* Repair gaps-to-bitmask length divergence when opening Gridstore
The number of regions the gaps file covers must match the bitmask, but
an unclean shutdown can break that: a lost extend writeback leaves
phantom all-zero entries beyond the bitmask, and a lost file growth
leaves the gaps file short. Phantom full entries force page creation
(they zero out trailing_free_blocks) and cover_new_page then panics on
its length assertion — before the lazy content rebuild can detect
anything, so that path cannot recover from this state.
Comparing the lengths is cheap, so do it on every open: on divergence,
log a warning, rebuild the gaps from the bitmask right away, and
consume the once-per-instance rebuild allowance. Allocation behavior
is unchanged on consistent storages.
* Reference to pull request
* Make gaps rebuild safe on Windows
Windows refuses to resize a file with a live user mapping, so the gaps
reset that recreated the file under its own mapping failed there with
OS error 1224 (ERROR_USER_MAPPED_FILE).
Split the rebuild along that constraint. The lazy content rebuild
keeps the mapping and overwrites the entries in place: it never needs
to resize, because a length divergence is repaired when the storage is
opened, and refuses with an error if it encounters one anyway. The
open-time length repair consumes the Bitmask by value so it can drop
the gaps mapping, atomically replace the file with the rebuilt
entries, and map it again — no resize of a mapped file on any
platform.
* Simplify gaps rebuild code
Cleanups from a review pass, no behavior change:
- compute_gaps: one read_all pass over region chunks instead of a
read_bit_range call per region, which also removes the loop body
duplicated from update_region_gaps
- BitmaskGaps::overwrite: take a slice instead of collecting an
iterator the only caller already holds as a Vec
- find_available_blocks: gate the divergence clamp on the O(1)
bit_len instead of hoisting read_all above it
- Gridstore::open: flatten the match-to-tuple into an if let, and
shorten the rebuild warning to match the runtime one
- tests: shared bitmask setup and value read-back helpers; drop the
length-divergence scenario from test_rebuild_gaps that
test_gaps_length_mismatch already covers (its search assertion
moved there)
- fix garbled log and comment wording
* 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>
* 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
* Add `CompactStoredFlags`, segment wrapper over the mutable bitmask
RAM-resident flags with a Flusher (skips the write when clean, cancels after drop) and files lister, backed by one compact stored-bitmask file rewritten whole on flush. Not integrated yet.
* Add `FlagsMode`, detecting the storage mode of a flags directory
`Dynamic` is the existing mmap stack for dedicated deployments, `Compact`
the compact stored-bitmask file for serverless ones; detection probes
which files are present. Also add the clippy allow the compact flags
tests were missing.
* Support the compact storage mode in `BitvecFlags` and `RoaringFlags`
The wrappers keep their in-memory read state in both modes; the new
`FlagsStorage` dispatches the write side between `BufferedDynamicFlags`
and `CompactStoredFlags`. `open_or_create` opens existing flags in their
detected mode and only applies `mode_if_create` to fresh ones — existing
call sites keep constructing the dynamic stack through `new`.
* Add `ReadOnlyCompactFlags`, read-only counterpart of compact flags
Bound to `UniversalRead`: opens on the bitmask header alone,
materializes the bitmap lazily on first query, and never creates a
missing file. Implements `RoaringFlagsRead` for the shared query
surface; `live_reload` reopens a fresh handle, as flushes replace the
file whole but cached handles keep serving the bytes they were opened
on. Not integrated yet.
* Skip compact live-reload tests on Windows, which forbids the rename
Both tests replace the compact flags file behind a reader whose disk
cache keeps the "remote" file mapped. On Unix the rename-over succeeds
and the mapping serves the old inode — the staleness under test — but
Windows forbids renaming over a mapped file, failing the writer's flush
with access denied. A limitation of the local-mmap remote stand-in, not
of the reload logic, which stays covered on the other targets.
* Don't check legacy flag file
* test: parameterize collection fixture and share resharding consensus stub
Let integration tests build a collection with a custom optimizers
config, and move NoopReshardingConsensus from the consensus idempotency
test into the shared test module.
* test: reproduce scale-down resharding abort hanging on deferred points
abort_resharding holds the shard holder write lock while
scale_down_cleanup_points deletes migrated points with
WaitUntil::Visible and no timeout. With prevent_unoptimized enabled that
wait only resolves once the optimizer has cleared every deferred point
of the shard, so a stalled optimizer wedges the write lock, and with it
every shard holder reader and - in a cluster - the consensus apply
thread driving the abort (SetShardReplicaState(Dead) on a
ReshardingScaleDown replica, e.g. after that peer is killed).
The test drives such an abort with the optimizer disabled and asserts it
completes. It currently fails by hanging into its 30s timeout, and must
pass once the cleanup delete no longer waits for visibility.
* When cleaning up old points, don't wait until visible
* Gridstore: introduce storage operating mode in config
Add a mode field to the gridstore config, selecting between the dynamic
mode (current behavior, the default) and the upcoming serverless mode.
The mode is specified through StorageOptions on creation, persisted in
config.json, and read back first when opening so the correct variant can
be selected automatically. Configs written before this field existed
deserialize as dynamic.
For now, selecting the serverless mode returns an error; the variant
itself is added in follow-up commits.
* Gridstore: move dynamic implementation into dedicated module
Mechanical move of the current Gridstore implementation into
gridstore/dynamic.rs as DynamicGridstore. The public Gridstore struct
becomes a thin wrapper holding a mode variant enum, propagating every
call into the selected variant. For now the enum only has the dynamic
variant; the serverless variant is added in follow-up commits.
No logic changes to the dynamic implementation itself: only visibility,
the config parameter now passed into open (the wrapper reads it first to
select the mode), and open_or_create staying on the wrapper.
* Gridstore: add serverless tracker
Add the append-only mapping tracker for the serverless storage mode.
The tracker file is a plain array of 16-byte mapping entries without any
header: the number of mappings is defined by the exact file length, and
the entry index is the point offset. The file starts empty and only ever
grows by appending, existing bytes are never rewritten. Mappings must be
set in monotonically increasing point offset order; skipped offsets are
backfilled as zeroed entries which decode as None.
New mappings are buffered in memory and appended with a single write per
flush. A flush with a stale target is a no-op so bytes are never written
twice. A torn trailing entry (file length not a multiple of the entry
size) is ignored when reading and truncated away when opening writable.
Unlike the dynamic tracker, the file is read and written directly with
positional file IO instead of memory mapping, as serverless environments
do not handle memory mapped files well.
* Gridstore: add serverless storage variant
Add the append-only gridstore variant for serverless deployments, which
restrict IO to appending to files: existing bytes can never be
rewritten, and IO is expensive so as few files as possible are used.
The variant stores all value data in a single page file next to the
serverless tracker and the storage config, three files in total. Both
data files start empty and only ever grow by appending; there is no
preallocation, no used-block bitmask and no gap/region bookkeeping.
Values are appended at put time at the next block aligned offset, with
the zero padding included in the write so it lands exactly at the end
of the file. Mappings are buffered and appended to the tracker with a
single write per flush, after the page file is synced, so a mapping on
disk never points at data that is not durable.
Values cannot be updated or deleted, and must be put at monotonically
increasing point offsets; violations are rejected before any data is
written. Files are read and written directly, never memory mapped.
The mode is selected through StorageOptions on creation and picked up
automatically from the persisted config when opening.
* Gridstore: serverless support in reader and view
Extend the read-only GridstoreReader and the GridstoreView with the
serverless mode, keeping both public types unchanged: like the writable
Gridstore they now hold a mode variant internally, selected
automatically from the persisted config when opening.
The serverless reader holds the tracker and page directly and reads the
files positionally, without memory mapping. A live reload re-reads the
mapping count from the exact tracker file length (there is no size
header), ignoring a torn trailing entry, and never truncates as it is
read-only. Value reads always go directly to the file, so newly
appended data is readable without remapping anything.
* Gridstore: document storage operating modes
* Gridstore: review fixes for the serverless mode
Hardening and cleanup from a review pass over the new serverless
storage variant:
- Batch the reader side iteration like the writer already did, instead
of materializing tracker mappings for the full range in one go, which
could transiently allocate gigabytes on large storages.
- Recover the append cursors when a positional write fails partway:
truncate the file back to the tracked length so a retried append or
flush never rewrites bytes that already landed in the file.
- Validate page addressability before appending value data, a rejected
put must not grow the page file.
- Cross-check tracker and page consistency when opening: mappings that
reference value data past the end of the page file (e.g. after a
partial copy or restore) now fail fast instead of surfacing as
opaque read errors per point.
- Reject value pointers into any page other than page 0 on the
serverless read path with PageNotFound, matching the dynamic mode
contract, instead of silently reading from a wrong location.
- Refresh the reported storage size on reader live reload even when no
new mappings were flushed, unflushed value data may have grown the
page file already.
- Validate configs read from disk: a corrupt config with zero sized
blocks, pages or regions is now rejected when opening instead of
panicking on a division by zero later.
- Classify rejected serverless puts as UnsupportedOperation, consistent
with rejected deletes, so they don't surface as user-facing
validation errors at the segment level.
- Deduplicate the compression dispatch into Compression::compress and
Compression::decompress, and the serverless file create/open patterns
into shared direct IO helpers, so the two modes and files can't
silently drift apart.
* Gridstore: cover both operating modes in mode-agnostic tests
Parameterize the gridstore tests that exercise mode-agnostic behavior
over both the dynamic and serverless mode with rstest, using a
single and bulk put/get roundtrips, storage files, basic persistence,
corrupt config rejection, batched read congruence, reader live reload,
and the different block sizes.
Mode specific expectations branch inside the tests: expected file
names, storage size semantics (whole blocks vs exactly packed bytes),
value pointer layout (page spill over vs a single packed page), and
gaps (created by deletes in dynamic mode, by skipped puts in serverless
mode). Dynamic-only internals assertions are kept behind a mode check.
Tests around updates, deletes, page spanning, block reuse and other
dynamic-only behavior intentionally stay dynamic; the serverless
specific format invariants remain covered by the dedicated serverless
tests.
* Gridstore: port serverless specific tests from sibling branch
Source the serverless specific test cases that the
serverless-gridstore-updates branch added, adapted to the dedicated
variant implemented here (distinct file names, headerless tracker
with 16 byte entries, a single packed page without trailing padding,
and rejected re-puts):
- writes only ever append: tracker and page files only grow and
previously written bytes stay byte-for-byte untouched
- new mappings land exactly at the end of the tracker file, which
always covers the exact number of mappings
- mapping gaps are zero-padded on disk and survive reopening
- values are packed back to back at block aligned offsets, the page
file ends exactly at the last value
- serverless mode never creates nor reports block flag files
- a flusher persists exactly the mappings that existed at its
creation, later puts stay pending
- a config claiming the wrong mode fails loudly in both directions
instead of loading the incompatible file format of the other mode
Tests around their mode switching, page spanning and tolerated deletes
don't apply to this design and are intentionally not ported.
* Gridstore: test serverless production risk scenarios
Add tests for the operational aspects that matter before serverless
mode goes to production, each covering a scenario that wasn't
evaluated yet:
- Replayed puts of already persisted offsets (a WAL redo after a
crash where the flush completed but was never acknowledged) are
rejected without appending anything, and max_point_offset is the
exact offset a replay must resume at.
- The accepted crash case of a tracker file extended with zeroed
bytes: the entries count as permanent None mappings, can never be
put again, and the storage stays consistent and writable past them.
- The read-only reader never modifies the files: opening over a torn
tracker tail, reading, iterating and live reloading leave both
files byte-for-byte untouched.
- A multi-round put/flush/reopen cycle always exposes exactly the
flushed prefix, with the mapping count matching the exact tracker
file length and unflushed offsets reusable.
- An append beyond the maximum addressable block offset is rejected
before writing anything, keeping retried puts from growing the page
file unboundedly.
* Gridstore: rename serverless mode to append-only, split into module
Rename the mode after its defining characteristic instead of its
deployment target: files only ever grow, existing bytes are never
rewritten. Renames Mode::Serverless to Mode::AppendOnly (persisted as
"mode": "append_only") and the on-disk file names to
append_only_tracker.dat and append_only_page_0.dat. The serverless
deployment motivation stays in the documentation.
Also split the single 2300 line serverless.rs into an append_only
module with dedicated files for the storage, page, view, reader and
tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Use universal IO in Gridstore
* Include upstream preopen logic in new Gridstore variant
* Gridstore: buffer append-only value writes until flush
In append-only mode, put previously wrote the value data to the page
file right away, one write operation per put, while mappings were
already buffered and batch persisted on flush. Buffer value writes the
same way: both the value and its mapping now only land on disk once a
flush cycle executes.
This batches all new value data into a single write operation per
flush, which is significantly more efficient on S3 based storage where
every write is a costly operation. A flush now performs exactly two
writes: one appending all buffered value data to the page file, one
appending all pending mappings to the tracker file, in that order, so
a mapping on disk never points at value data that is not durable.
The page mirrors the tracker's pending mechanism: an in-memory buffer
that is byte for byte the next append (zero padding between block
aligned values included), a watermark captured at flusher creation so
puts made during a flush stay buffered, a stale-flush no-op guard so
appended bytes are never written twice, and truncate-back recovery on
failed writes. Reads transparently serve buffered values from memory.
As a side effect, a crash between flushes now leaves nothing on disk
at all, where the write-through approach left orphaned value bytes in
the page file. The buffered data is held in memory until the next
flush, bounded by the flush cadence.
Universal IO filesystem handles are now required to be Send + Sync, so
the flusher closure can carry one to grow the page file at flush time;
all existing backends already satisfied this.
* Gridstore: rename inner DynamicGridstore to Gridstore
The dynamic variant keeps the Gridstore name; the outer dispatching type
will be renamed to Blobstore in a follow-up. Until then the inner type is
referred to as dynamic::Gridstore to distinguish it from the outer type.
* Gridstore: rename append-only variant to Arenastore
The append-only variant stores all value data in a single ever-growing
page, allocating space by appending, hence: arena store.
* Gridstore: rename outer storage type to Blobstore
The outer type dispatching between the two storage variants is now called
Blobstore, being more generic than Gridstore. This frees up the Gridstore
name, which now exclusively refers to the dynamic mode variant, next to
Arenastore for the append-only variant. Storage components keep using the
outer type, so they now use Blobstore.
The gridstore crate name, GridstoreError, and the persisted names
(config.json mode, payload config storage_type) are unchanged.
* Gridstore: split Gridstore and Arenastore into dedicated modules
The outer module is now blobstore, matching the Blobstore type it
defines. The two storage variants each get their own submodule: the
dynamic Gridstore moves from dynamic.rs into gridstore/ with its reader
and view extracted from the shared files, mirroring the arenastore/
module (previously append_only/) which already had this layout.
* Rename gridstore crate to blobstore
The crate is named after the outer Blobstore storage type it provides.
The gridstore name lives on in the dynamic mode variant. GridstoreError
and the persisted names (config.json mode, payload config storage_type)
are unchanged.
* Arenastore: pack values back to back across multiple pages
Drop the block alignment from the append-only mode: values are packed
byte to byte, without blocks, and the tracker offset is now a plain byte
offset within the page. Blocks and regions are dynamic mode concepts;
their page size constraints no longer apply to append-only configs.
Bring back support for multiple pages. Once appending a value would
grow the current page beyond the configured page size, a new page is
started, bounding the size of and the number of appends to each file:
object stores like S3 Express limit the number of appends per object.
A value larger than the page size gets a page of its own; values never
span pages.
A rollover creates the new, empty page file at put time; the value data
itself stays buffered until the next flush, which appends to each
touched page with a single write, using per-page watermarks captured at
flusher creation. The reader scans for consecutively numbered page
files when opening, validates the most recent mappings against them,
and adopts pages created since on a live reload.
* Blobstore: rename dynamic mode to mutable
Rename Mode::Dynamic to Mode::Mutable, and the persisted config value
with it: config.json now writes "mode": "mutable". There is no
compatibility alias for "dynamic", released versions never wrote the
mode field (a missing field still defaults to mutable), only unreleased
storages did.
The Gridstore type and module names for the mutable variant are
unchanged.
* Fix Edge compilation due to package rename
* Review remarks
* Extract Gridstore preopen into module
* Rename Arenastore files
* Use universal IO for append operations
* Rename GridstoreError to BlobstoreError
The error type belongs to the Blobstore crate and is shared by both the
Gridstore and Arenastore variants, so it follows the crate naming. Also
update the user-facing error messages that referred to the old name.
* Split config into per-variant types
* Rename Arenastore to Logstore
Rename the Arenastore type to Logstore, including the reader, view,
config, module and variant names. The storage file names follow:
log_page_{n}.dat and log_tracker.dat. The persisted mode tag stays
"append_only".
* Move bitmask module into the Gridstore variant
The bitmask tracks free blocks, which only exists in the mutable mode.
Move the module from the crate root into the Gridstore variant that
owns it. It stays re-exported at the crate root because the bitmask
benchmark needs a public path.
* Move pages module into the Gridstore variant
Like the bitmask, the block based pages module is only used by the
mutable mode. Move it from the crate root into the Gridstore variant
that owns it. The Logstore variant has its own page implementation.
* Use universal IO for every Logstore operation
Replace the direct_io module with universal IO in the append-only
tracker, making the whole Logstore go through a universal IO backend
bounded by UniversalRead and UniversalAppend:
- The tracker is generic over the backend now. Reads go through
UniversalRead with the caller's access pattern, flushes land as one
atomic append with the same offset compare-and-swap recovery as the
pages: a retried append after a lost acknowledgement is adopted
instead of appended twice. A torn trailing entry is still truncated
away on writable open, through a fresh handle since shrinking is not
supported through an open one.
- The reader now schedules a prefetch for the tracker file too, it no
longer bypasses the backend.
- The config write, clear and wipe use the backend file operations
instead of local filesystem calls, matching the Gridstore variant.
* Batch reads in Logstore read_values
Apply the same batching logic as the Gridstore variant: resolve all
mappings first, then fetch the value data, both through the backend's
read pipeline so async backends can serve the reads in parallel.
The tracker gains a batched lookup mirroring the mutable tracker's
iter, serving pending mappings and out of range point offsets directly
from memory. The pages gain a batched value read; unflushed values are
served from the in-memory buffers, and since values never span pages
each value is a single read without reassembly.
Like in the Gridstore variant, the callback may now be invoked in a
different order than the requested point offsets.
* Better describe logstore live reload ordering
* use enum for options, swap `*Options`<->`*Config` naming
* don't wrap enum in struct
* ditch unused `StorageConfig`, make deserialization more ergonomic
* rename `*Options`->`*Config`
* make `preopen` non-blocking
* fixup! ditch unused `StorageConfig`, make deserialization more ergonomic
* fixup! use enum for options, swap `*Options`<->`*Config` naming
* fixup! don't wrap enum in struct
* fix rebase
* use `populate` param in Logstore
* test: failing repro of stale page after live reload across rollover
A reader that live-reloads between a page rollover and the following
flush adopts the new, still empty page. The previous page is then no
longer the last one and is never reloaded again, so the tail that the
next flush appends to it stays invisible to the reader forever:
value pointer at byte 100 with length 100 is out of range
AppendOnlyPages::live_reload only reloads the last held page, assuming
earlier pages never change once a newer page exists. But the rollover
creates the new page file eagerly at put time, while the previous
page's buffered tail only lands at the next flush (see
test_rollover_writes_no_value_data_before_flush), so a page can keep
growing on disk after its successor exists.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: reload all pages that grew
* use Fs in `open_or_create`
* fix: publish tracker mappings only after the pages reload
`AppendOnlyTracker::live_reload` observed the mapping count and made it
visible in one step, before `LogstoreReader::live_reload` reloaded the
pages. Every failure path in the page reload -- `list_files`, reopening a
grown page, opening an adopted one, the truncation check -- therefore left
the reader with mappings referencing value data it never loaded, so reads
in the new offset range fail until a later reload happens to succeed. The
edge refresh loop keeps a segment whose reload failed, expecting it to keep
serving its pre-refresh state, which it then does not.
Split observing from publishing: `reload_count` refreshes the handle and
returns the count as a `PendingReload` token, `commit_reload` publishes it.
The reader still observes the tracker first, as the writer persists pages
before the mappings referencing them, but only commits once the pages are
loaded. Reopening without committing is harmless: reads stay bounded by the
unchanged count, and the bytes below it never change.
A partial failure inside the page reload needs no unwinding, pages running
ahead of the tracker is the safe direction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* perf: batch the value reads in Logstore iteration
`LogstoreView::iter_range`, the path behind `Logstore::iter` and
`LogstoreReader::iter`, fetched the mappings for the whole range with a
single read but then read the values themselves one at a time, serially.
Gridstore routes its `iter` through `read_values` and pipelines both stages,
so a full scan of an append-only storage was the one read path without
batching -- one blocking round trip per value on the object store backends
this variant exists for. It is reached by payload storage iteration and by
the payload index build, which scans every payload.
Feed the pointers into `read_batch_values` instead, keeping the single
contiguous tracker read, which is better than the per-offset pipeline
scheduling Gridstore does on that side.
Values are now delivered through the read pipeline, so the callback may be
invoked out of order, as it already could be for Gridstore's `iter` and for
`read_values` in both variants. Both segment callers are order independent.
Tests that happened to rely on the mmap backend completing reads in
scheduling order now sort before comparing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test: don't run the failed-page-reload test on Windows
The test shrinks a page file out of band to make the page reload fail, but
Windows refuses to resize a file while the reader holds it mapped, which it
does by construction here: "the requested operation cannot be performed on a
file with a user-mapped section open". The panic is on the injection itself,
the code under test never runs.
There is no portable injection. Truncating a page the reader holds is what
the check under test detects, so the mapping cannot be avoided; failing the
adopted page open instead needs a listed but unopenable file, and
`local_list_files` descends into matching directories rather than listing
them; failing the directory listing needs the storage directory removed,
which Windows also refuses while pages are mapped.
The storage itself is fine on Windows, its append path grows mapped pages
there and every other Logstore test passes. The logic under test is platform
independent and stays covered elsewhere, with the tracker half of the
guarantee pinned by `test_live_reload`, which runs on every target.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: generall <andrey@vasnetsov.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Luis Cossío <luis.cossio@outlook.com>
* universal_io: add UniversalAppend for atomic single-operation appends
Growing a file previously took a separate set_len + reopen + write dance
(bypassing universal_io and leaving a zero-filled window on crash), and
there was no way to express appends for backends without random-offset
writes.
UniversalAppend::append grows the file by writing at the current end of
file in one atomic grow+write operation and returns the offset at which
the data landed; append_batch lands multiple buffers contiguously in as
few operations as the backend allows. flusher() moves from
UniversalWrite into a new UniversalFlush supertrait so append-only
handles can require it without duplicating the method.
Local backends, both single-syscall:
- MmapFile appends through a dedicated O_APPEND fd (every write(2) /
writev(2) is an atomic grow+write at EOF), then remaps via reopen().
Its flusher also fdatasyncs after appends, since msync alone does not
persist file-size metadata.
- IoUringFile appends via pwritev2(RWF_APPEND). O_APPEND is not an
option there: on Linux, pwrite on an O_APPEND fd appends regardless
of the given offset, which would break positioned writes on clones
sharing the fd.
Concurrent appenders are out of contract (single logical writer);
object-store backends surface the new AppendOffsetConflict error and
recover via reopen() + retry.
* io_bridge: add AsyncWrite/AsyncAppend and appendable BlobFile
Add the write-side backend traits reserved next to AsyncRead: AsyncWrite
(create/remove/save) powers a UniversalWriteFileOps impl on BlobFs
(create-or-truncate put, delete, atomic whole-object save; directory ops
are no-ops), and AsyncAppend — a single-request append where the offset
must equal the current object size, acting as a compare-and-swap token —
powers UniversalAppend on BlobFile.
BlobFile caches the object size across appends (one HEAD for N appends;
a missing object counts as empty so the first append creates it),
concatenates batches into a single request, and drops the cache on
reopen() — the documented recovery path after AppendOffsetConflict. Its
flusher is a no-op: appends are durable once the backend acknowledges
them.
* io_bridge_object_store: native single-request S3 append
object_store has no append support, so issue the PutObject +
x-amz-write-offset-bytes request ourselves, reusing the store's
credential chain (AmazonS3::credentials) and object_store's SigV4
AwsAuthorizer, which signs every header present on the request — no
hand-rolled signing and no direct reqwest dependency. The offset doubles
as a compare-and-swap token: a mismatch (400 InvalidWriteOffset, or 412
on some S3-compatibles) maps to AppendOffsetConflict.
The write-offset append API exists on AWS S3 Express One Zone directory
buckets and compatible stores (e.g. MinIO AiStor) — plain S3 Standard
buckets reject it, and real Express zonal endpoints / session auth are
not verified yet; MinIO-AiStor-compatible stores are the primary target
for now. GCS and Azure sources simply do not implement AsyncAppend.
ObjectStoreSource carries an AppendContext (HTTP client + object URL
base + signing region) built per backend from its config, and gains a
generic AsyncWrite impl (single-put create/save, delete). A test-only
multi-request CAS emulation over InMemory exercises the BlobFile append
stack hermetically; an end-to-end flow against a real append-capable
store is gated behind S3_APPEND_INTEGRATION_TEST=1.
* simple_disk_cache: write-through UniversalAppend for DiskCache
Append to the remote (the single grow+write operation), then write the
same bytes through into the local mirror so tail reads do not re-fetch
what was just uploaded. LocalState::append_local keeps the fetched
bitmap accurate: blocks fully covered by the appended range are marked
fetched, and the pre-append partial tail block — which resize() drops
because set_len zero-fills its gap — is re-marked only when its prefix
was already fetched. If the mirror turns out stale (the remote grew
behind our back), append falls back to resize-only and lazy fetches
heal the gap on the next read.
Writeable opens are now allowed on DiskCacheFs solely to enable append;
DiskCache still never implements UniversalWrite. The writeable flag
propagates to the remote handle, which is opened buffered instead of
O_DIRECT: appends write through the page cache, which O_DIRECT reads on
the same fd would fight (and IoUringFile rejects appends on
prevent_caching handles). The remote-immutability docs are relaxed to
append-only with an immutable prefix, matching what reopen() already
assumed.
Includes a full-stack composition test: DiskCache write-through over
BlobFile offset tracking over an in-memory object store.
* io_bridge_object_store: build the append HTTP client lazily
Opening a source from an AwsConfig eagerly built the reqwest client
(TLS setup, connection pool) even when append was never used. Keep the
AppendContext construction to pure config (allow_http flag, object URL
base, signing region) and build the client on first append instead,
cached in an Arc<OnceLock> shared across clones of the source — and
thus across the file handles opened from it. Sources that never append
now pay nothing; client-construction errors surface on the first append
instead of at open.
* universal_io: test that append grows the regular file on disk
The conformance suite reads appended bytes back through universal-io
handles; also assert the underlying regular file itself — created
outside universal_io, verified with plain fs reads — for both local
backends.
* Mention why we use custom HTTP client, object_store crate has no support
* io_bridge_object_store: reject appends unconfirmed by the size header
A store without write-offset support may accept the signed PutObject as
a plain put — replacing the object with just the appended bytes — and
return 2xx (community MinIO did exactly this before 2025-05, commit
minio/minio@6d18dba9). The old success path fabricated the new length
when x-amz-object-size was missing, so the destruction stayed invisible
while every subsequent append repeated it.
Require the x-amz-object-size response header (returned by AWS and
MinIO AiStor appends) for any append at offset > 0 and fail loudly
without it. Offset-0 appends are equivalent to a whole-object write, so
they remain valid either way — a misconfigured store now fails on the
second append instead of never.
* io_bridge_object_store: honor endpoint/region env vars for appends
With AwsCredentials::Default the store is built via
AmazonS3Builder::from_env, which honors AWS_ENDPOINT_URL_S3,
AWS_ENDPOINT_URL, AWS_ENDPOINT, AWS_REGION and AWS_DEFAULT_REGION — but
append_context derived the append URL and SigV4 region only from the
typed config fields. An env-configured deployment would read from one
host while signing and sending appends to
https://{bucket}.s3.us-east-1.amazonaws.com.
Resolve the append endpoint and region the same way build_store does:
explicit config first, then (default credential chain only) the same
environment variables, with AWS_ENDPOINT_URL_S3 taking precedence as in
from_env. The resolution is a pure function over an injected env lookup
so the test does not touch process-global environment state.
* simple_disk_cache: delegate the append flusher to the remote
DiskCache's UniversalFlush impl was an unconditional no-op, justified by
object-store appends being durable on acknowledgement — but the impl is
generic over any appendable remote, and for local remotes (MmapFile,
IoUringFile, exactly the compositions the tests instantiate) that
silently dropped the fdatasync the UniversalAppend contract requires:
append, flush Ok, power loss, appended bytes gone.
Delegate to the remote's flusher once the cache is materialized: local
remotes get their sync, object-store flushers remain no-ops, and a
never-materialized cache has made no appends so a no-op stays correct.
* io_bridge_object_store: retry transient append failures
The append RPC was a single unretried HTTP attempt, while every other
request in this stack goes through object_store's retry layer — a
routine transient 503 SlowDown or connection reset failed the append
hard where a concurrent read would have silently recovered.
Retry connection errors, 5xx and 429 up to three attempts with a short
linear backoff, re-signing per attempt (the SigV4 signature embeds the
request date). Retrying is safe because the write offset is a
compare-and-swap; the one ambiguity — an attempt that landed but whose
acknowledgement was lost — surfaces as a write-offset conflict on the
retry, which is reconciled with a HEAD: under the single-writer
contract, an object size of exactly offset + data_len proves the tail
is ours, so the append reports success instead of a spurious conflict
(whose reopen-and-retry recovery would duplicate the record).
* universal_io: forward TypedStorage::flusher for any UniversalFlush
The flusher forwarding lived in TypedStorage's S: UniversalWrite impl
block, so append-only storages (DiskCache, BlobFile — UniversalAppend +
UniversalFlush but not UniversalWrite) offered append through the
wrapper while the durability flusher the append contract mandates was
unreachable without going through .inner.
Move it to an S: UniversalFlush block: UniversalWrite implies
UniversalFlush, so existing callers resolve unchanged, and duplicating
the method instead would have hit E0592 on backends implementing both —
the very ambiguity UniversalFlush was extracted to avoid.
* io_bridge_object_store: surface unbuildable append requests as errors
Request building could panic on two reachable paths: url accepts URIs
the http crate rejects (IPv6 zone identifiers, URIs beyond u16::MAX
bytes), and AppendContext::new is public so the object URL base is not
guaranteed to be a base URL. Both expects become S3Config errors, so a
configuration edge case fails the append instead of panicking the
thread driving the bridge runtime.
* simple_disk_cache: don't fail appends the remote already committed
append_impl committed to the remote first and returned Err when the
subsequent local-mirror update failed (e.g. ENOSPC on the cache volume)
— indistinguishable from "nothing was appended", so a retrying caller
would duplicate the record on the remote.
The mirror is cache maintenance, not part of the append: on a failed
write-through, log and degrade to bare growth so lazy fetches heal the
unmarked blocks (safe — blocks are only marked fetched after their
bytes landed). Only an unresizable mirror still surfaces an error, and
the UniversalAppend contract now documents that an append Err does not
guarantee nothing was appended: reopen() and re-check the length before
retrying.
* universal_io: bounds-check positioned io_uring writes against EOF
The UniversalAppend contract states that UniversalWrite::write beyond
the end-of-file fails and append is the only growth path — mmap
enforces it, but IoUringFile's write/write_batch/write_multi were
unchecked pwrites that silently extended the file with a zero-filled
hole, inflating subsequent append offsets.
Check every positioned write against the file length (fstat once per
call), matching mmap's OutOfBounds semantics, and generalize the
regression test to run on both local backends including the batched
path.
* io_bridge: reject appends on handles opened without writeable
BlobFs::open dropped OpenOptions entirely, so a BlobFile opened with
writeable: false still accepted appends — mmap and DiskCache enforce
the writeable requirement, the blob backend silently didn't, and a
stray append through a nominally read-only handle would mutate a shared
object.
Thread OpenOptions::writeable into BlobFile and reject appends with
PermissionDenied when it is unset, mirroring the other backends.
Directly-constructed handles (BlobFile::new/open, which take no
OpenOptions) remain writeable. With every backend now enforcing the
flag, the UniversalAppend contract drops its 'where the backend
enforces open modes' hedge.
* simple_disk_cache: answer empty appends from the mirror
An empty append still went through remote.append_batch, so it returned
the remote's live end-of-file — which can diverge from what this
handle's len() and reads observe when the remote grew behind our back —
while leaving the stale mirror unhealed (unlike a non-empty append in
the same state, which resizes).
Accept empty appends early: return the mirror length without touching
the remote at all, keeping the answer consistent with the handle's own
view. The trait contract now spells out that empty appends return the
handle's view of the end of file without growth I/O.
* universal_io: grow the mmap in place after appends
Every mmap append ended in a full reopen(): an open+fstat+close by path
just to learn the new length, and — on the non-Linux fallback, which
rebuilds the mapping with the open-time populate flag — a re-population
of the ENTIRE file per append, making appends O(file size) for handles
opened with Populate::Blocking. Populating after an append is pointless
anyway: we just touched the data we wrote.
Extract the remap machinery into remap_to() (reopen() keeps its exact
semantics, populate included) and add grow_mapping(): a stat-free grow
that never re-populates. Appends learn the new length from a single
fstat on the already-open O_APPEND fd — kept rather than trusting the
mapping length, which is stale exactly in the externally-grown-remote
scenario the disk cache heals through lazy fetches (the foreign-growth
tests catch the difference). The mirror's resize() passes the length it
just set_len'd, dropping its stat round-trip entirely.
Per small append this is write+fstat+mremap, down from
write+open+fstat+close+mremap, with no populate anywhere.
* universal_io: share the mmap append fd across clones
The flusher captured the per-clone append_file at flusher-creation
time, so a flusher obtained from a sibling clone — or created before
the handle's first append — msynced the shared mapping's data pages but
skipped the fdatasync that persists the appended file size: a
half-persist where a crash loses the acknowledged tail even though a
flusher ran after the appends (writer thread + long-lived flush-worker
clone is exactly the natural WAL shape).
Store the fd in an Arc<OnceLock> shared by all clones and read it at
flush time instead of capture time: any clone's append makes every
handle's flusher sync the size metadata, whatever the clone/flusher
creation order. Initialization races between clones keep exactly one
fd. No hot-path cost: reads and positioned writes never touch the cell,
and the append path pays one atomic load next to its syscalls. The
interior mutability also lets append_fd take &self.
* universal_io: document the clone remap hazard truthfully
The remap SAFETY comment claimed moving is safe "since we are holding
&mut self" — which says nothing about clones: they share the mapping
but keep their own raw ptr/len copies, so after a moving (or, on
non-Linux, replacing) remap a sibling clone's next read dereferences an
unmapped address. The trait contract understated the same hazard as a
concurrent-read constraint, while the UB persists after append returns.
State the contract once on MmapFile (clones must reopen before reading
after any growth; a stale clone read is undefined behavior, not a stale
view), correct the SAFETY argument to rely on it explicitly, annotate
the as_bytes unsafe blocks that depend on it, and sharpen the
UniversalAppend contract bullet accordingly. Making clones structurally
safe (resolving ptr/len through the shared Arc) is deliberately left as
a separate change.
* universal_io: share the vectored append machinery between backends
The IOV_MAX-chunking / EINTR-retry / WriteZero / advance_slices loop
existed twice — as local_file_ops::write_all_vectored (mmap) and
inlined around pwritev2 in IoUringFile::append_slices — along with a
verbatim collect/cast/filter-empties preamble in both append_batch
impls. Two copies of subtle short-write handling introduced by one
branch will diverge the first time only one of them gets a fix.
Add an io::Write adapter whose write_vectored issues
pwritev2(RWF_APPEND), letting the io_uring append delegate to the
shared write_all_vectored, and hoist the slice collection into
local_file_ops::collect_append_slices. IOV_MAX becomes private to the
one function enforcing it. No behavior change; the existing conformance
tests (including the beyond-IOV_MAX batch) cover both backends through
the shared path.
* io_bridge_object_store: add s3_express to the test config helper
The AwsConfig struct gained the s3_express field; update the
resolve-endpoint test helper accordingly.
* universal_io: run the append conformance suite over the S3 stack
Promote the backend-generic UniversalAppend battery (offsets, batches
across IOV_MAX, empty appends, read-after-append, reopen visibility,
flusher) from a private test into universal_io::conformance, exposed
under the testing feature so backend crates can run the identical
suite. mmap and io_uring keep running it as before; the object-store
bridge now runs it too, over BlobFs/BlobFile with the in-memory
offset-CAS append emulation — so local file system and S3 append
behavior are asserted by the same test. The real write-offset RPC
remains covered by the gated test_native_append_flow integration test.
* Swap order
* universal_io: disambiguate the io_uring crate import
The import reorder dropped the leading `::`, making `io_uring`
ambiguous with this very module (pulled into scope by the
`use super::*` glob) and breaking the build.
* io_bridge: don't materialize the mock object on rejected appends
MutableMockSource::append called get_or_insert_with before validating
the offset, so a rejected stale append against a missing object left an
empty entry behind (exists() flipping true) — a fidelity gap versus the
real backends, where a rejected append has no side effects. Check the
offset against the current length first and only materialize the buffer
on a match.
* universal_io: disambiguate the io_uring crate import
Restore the leading `::` on the io_uring crate import — without it the
name is ambiguous with this very module, which the `use super::*` glob
pulls into scope, and the crate fails to compile. Matches the sibling
files (pool.rs, runtime.rs), which already import via `::io_uring`.
* io_bridge_object_store: treat 404 under a nonzero append offset as a conflict
A missing object while the handle expected a nonzero end-of-file is a
stale view (the object was deleted behind our back) — the same
situation as an offset mismatch, with the same reopen-and-retry
recovery, and it is exactly what the in-memory emulation and the
io_bridge mock already report. The RPC path mapped every 404 to
NotFound instead, so the three implementations disagreed on the same
logical case. Keep NotFound for offset-0 appends, where a 404 is a
genuine missing-target error (e.g. a missing bucket) that retrying
cannot heal.
* Preallocate vector
* universal_io: disallow appends through the disk cache
Appends must go directly to the backing storage (mmap, io_uring, S3) —
the disk cache is strictly read-only again. Remove DiskCache's
UniversalAppend/UniversalFlush impls and the mirror write-through
machinery (LocalState::append_local), reject writeable opens at
DiskCacheFs::open, and drop the writeable/prevent_caching plumbing that
existed solely for cached appends, restoring read-only remote handles.
Attempting to append through the cache is now a compile-time error (the
trait impl no longer exists), and opening a cached handle writeable is
rejected at runtime, covered by a test in each backend variant.
* universal_io: make append idempotent via caller-supplied offset
Append now takes the byte offset where the data must land:
append(offset, data) -> Result<()>. Every backend validates that the
offset equals the current end of file before writing (mmap and io_uring
fstat the fd, object stores validate server-side via
x-amz-write-offset-bytes); on mismatch nothing is written and the append
fails with AppendOffsetConflict. Retrying an already-landed append
therefore conflicts instead of appending twice, and recovery is
re-deriving the offset from len().
BlobFile no longer tracks the object length locally; the store's own
offset check is the compare-and-swap.
* Review remarks
* Validate file length in S3 append response
* Fix linting, we don't mind a large enum variant on index builder
* universal_io: conformance-test stale-handle append conflict recovery
Promote the two-handle conflict scenario from the in-memory BlobFile
test into the backend-generic conformance battery: a second writeable
handle grows the file, the stale handle's append conflicts cleanly (the
offset check runs against the file, not the handle's view), and the
contract's documented recovery — reopen, re-check the length, append at
the real end — lands the data exactly once. Now exercised over mmap,
io_uring, and the object-store stack instead of only the in-memory
emulation.
* io_bridge_object_store: stub-server tests for append response handling
The native append's HTTP state machine was only exercised by the gated
live-store integration test (S3_APPEND_INTEGRATION_TEST=1), so none of
its branches ran in CI. Cover them hermetically against a minimal local
HTTP stub — one connection per canned response, no new dependencies:
- the signed write-offset PUT, and the new-size validation on success
(matching, mismatching, unparseable, and absent size headers — the
absent case at offset zero and past it);
- conflict mapping for 400 InvalidWriteOffset, 412, and 404 under a
nonzero offset, with 404 at offset zero staying NotFound, and a 400
without the conflict code staying a plain error;
- 429/5xx retries re-sending the same offset, giving up after
MAX_ATTEMPTS, and the lost-acknowledgement reconciliation via HEAD
(accepted when the object ends at offset + len, rejected otherwise);
- the status + body excerpt on unexpected failures.
* simple_disk_cache: statically assert the cache stays read-only
Disallowing appends through the disk cache made them a compile-time
error by removing the impls; pin that with assert_not_impl_any so the
UniversalAppend/UniversalFlush/UniversalWrite impls cannot quietly
return. Runtime rejection of writeable opens stays covered per backend
variant.
* Add routing token structure
* Implement routing token in read operation executor as per design doc
* Add TODO to glue routing token to user requests
* Implement routing header for REST API
* Source routing token from request, not from JWT token
* Implement routing token in gRPC API
* Add test
* Review remarks
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Use lower case header name to prevent panic
* Rename header to X-Qdrant-Route-Affinity
* Assert routing consistency in test on all peers
---------
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 7: Workflow does not contain permissions
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 9: Workflow does not contain permissions
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 10: Workflow does not contain permissions
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 19: Workflow does not contain permissions
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 20: Workflow does not contain permissions
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Set permissions in GitHub workflow jobs
---------
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Use assert_matches!
* Add trailing commas
* Use more assert_matches!
Also, drop now redundant `expected blah but got blah` messages because
`assert_matches!` will print these.
* Use debug_assert_matches!
---------
Co-authored-by: xzfc <xzfcpw@gmail.com>
* Fix resharding, on queries filter shards on all shard selectors
* Add failing consensus test: search during resharding with shard keys (#9880)
Reproduces a known bug: after resharding is initialized on a custom
sharded collection with a shard key, searches (with and without the
shard key selector) fail with "does not have enough active replicas",
because the new resharding shard is included in reads before it has
an active replica.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Exempt explicit shard id selection from resharding read filter
Explicit shard id selection is only used by internal per-shard
operations (local shard API, internal gRPC reads), including the
resharding driver reading back migrated points from the new shard.
These must reach the resharding shard before it becomes visible to
user-facing selectors, and filtering them also made per-shard reads
return silently empty results on peers lagging on hashring commits.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Explicitly set resharding filtering per match branch
---------
Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Abort resharding before we abort transfer
* Test resharding-down abort converges when a peer is killed mid-abort
---------
Co-authored-by: tellet-q <elena.dubrovina@qdrant.com>
* Recreate optimizers in non-blocking fashion from consensus calls
* Update comments
* On optimizer config update failure, report error status to local shard
* Add a test to confirm we don't block consensus
* Rerun recreation if called multiple times
* Use atomics instead
* Move to the bottom
* Reformat
* Add test to show cleanup may conflict with update queue
* When invoking clean task, first wait for current update queue
* Don't hold shard holder lock for a long time
* Also assert the clean task finished completely
* [ai] Add integration test for triggering inconsistent resharding state
* [ai] Also add test for resharding down
* Update test
* Resolve resharding idempotency through setting replica states
* Remove resharding down test
* Reformat
* [ai] Remove resharding abort order, abort before setting replica state
* [ai] Resolve test flakiness
* Collapse matches into helper function
* Check preconditions before aborting resharding
* Fix test flakiness by not waiting for a dead node
* Abort resharding before aborting transfer for idempotency
* Update comment
* Release shard holder lock on transfer/reshard abort to prevent deadlock
* Remove now unused shard holder parameter
* Split handle_replica_changes to eliminate need for juggling locking
* In resharding tests, import all utils to enable every_test cleanup
Resolves flakiness I've been seeing in
test_set_replica_dead_clears_resharding_state test
* [ai] Always set default transfer method on entry peer
This will propagate a specific transfer type through consensus to all
peers. It ensures all peers will use the same transfer method when
applying the operation.
* [ai] Enforce the shard method to be specified
* Still allow unspecified transfer method for older peer versions
* Use snapshot shard transfers by default in Qdrant 1.18.0 and up
* New method does not have to be async
* Use stream records based transfer for replicate points with filter
* Make resharding state transitions idempotent on replay
Why: consensus entries may be re-applied after a crash (partial state
on disk) or during raft recovery. The unchecked state-transition
helpers used `debug_assert!` to require a specific starting state, so
a replay would panic in debug or silently overwrite in release.
How to apply: use write_optional so the state file is only touched
when the in-memory state needs to change. This also avoids unnecessary
fsyncs when a replay is a no-op.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Skip replica set creation on resharding start replay
Why: on replay, `create_replica_set` would call `create_shard_dir`,
which removes and recreates the existing shard directory -- wiping
any shard contents that had already been migrated or written since
the first apply.
How to apply: check `contains_shard(shard_id)` before creating the
replica set and pass `None` to `start_resharding_unchecked` when a
replica set with the target shard id is already present. Also
relaxes `check_start_resharding` to return `Ok` (instead of a
swallowed `bad_request`) when a matching resharding state is
persisted, so the caller can fall through each idempotent step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Derive resharding shard count from shard id for idempotency
Why: the start/finish/abort paths used ++/-- on the persisted shard
number with a `debug_assert` pinning the expected starting value. On
replay (e.g. after a crash between the shard holder mutation and the
config save) this either panics or produces a wrong count.
Since resharding always targets the last shard (auto sharding assigns
contiguous ids from zero), the target count is a pure function of the
shard id: `shard_id + 1` for start up, `shard_id` for finish down and
abort up. Set it directly and skip the save when it already matches,
so replay converges to the same value without touching the file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fall through resharding finish/abort steps on replay
Why: the early return on `resharding_state.is_none()` assumed that a
missing state means the operation was fully applied. But a crash can
leave the state cleared while the shard drop, key mapping removal, or
shard count update are still pending. Short-circuiting then skips the
reconciling work those replays need to do.
How to apply: drop the early return so every step runs; each step is
already individually idempotent (check_*, drop_and_remove_shard,
remove_shard_from_key_mapping, the set-based shard count update).
The abort_resharding down-invalidation path now reads nodes from the
router regardless of its variant, so a replay over an already-rolled-
back ring doesn't trip the removed debug_assert!s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Skip shard key mapping write when nothing to remove
Why: `write_optional` returns `Some` whenever the shard key is
present, even if the shard id we're removing is already gone. That
still triggers a JSON rewrite and a cache notification on every
replay of a finished finish/abort.
How to apply: check that the shard id is actually in the set before
returning `Some`. If the set is missing or already doesn't contain
the id, return `None` so the on-disk file and in-memory data are
left untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add tests for resharding replay idempotency
Covers the paths that must not error or panic on replay:
- `check_start_resharding` when matching state is already present
- `start_resharding_unchecked` preserves matching state verbatim
- `finish_resharding_unchecked` when state is already cleared
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Simplify finish_resharding_unchecked closure
Replace a manual `match` on `Option` with `as_ref().map(...)` to
satisfy clippy::manual_map.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: add a test for resharding replay (#8839)
* [ai] add a debug_assert on shard_number (#8846)
* Update lib/collection/src/shards/shard_holder/resharding.rs
Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com>
* Reformat
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: tellet-q <166374656+tellet-q@users.noreply.github.com>
Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com>
* [ai] On shard snapshot transfer recovery, drop existing shard before recovery
* [ai] Add integration test to assert clearing behavior
* [ai] Debug assert our replica is not active when we clear it
* [ai] Tweak assertion
* Fix flaky test, replica may temporarily not be visible
* Replace debug assertion with runtime error
* [ai] Add config boolean to disable URL based snapshot restore
* Merge if-statement
* Comment-out config option by default
* [ai] Also block partial snapshots from remote URLs
* [ai] Only run clock consistency test when staging feature is present
* [ai] Add integration test
* [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
* [ai] Allow peer to bootstrap with existing URI if peer has no shards
Prompt:
I have a Qdrant cluster in distributed mode with some nodes registered
in consensus. If I bootstrap a new node with an URL that is already used
it is rejected and an error is returned. I would like to change this
behavior and allow this to happen. This would effectively replace a peer
because internally we'd drop the existing peer first, and then we'd add
the new peer so we can reuse the same peer URL. We must still reject
bootstrapping with the same URL if the peer that used the URL before us
still has any shards on it.
* [ai] Add test to assert new behavior, can rejoin if empty
Prompt:
Add two tests to assert the new behavior.
The first test should:
- create a cluster
- create a collection
- bootstrap a new peer
- kill and delete the local data for this peer without removing it from consensus
- bootstrap a new peer but reuse the URI of the peer we just killed to rejoin
- bootstrapping is expected to succeed
The second test should:
- create a cluster
- create a collection
- bootstrap a new peer but reuse the URI of the last node
- bootstrapping is expected to fail because the node being replaced has data on it
* [ai] Attempt to fix new tests
* Reformat
* [ai] Fix deadlock when rejoining with same peer URI
* [ai] Add test to ensure existing peer stops consensus on replace
* Add basic test that shows shard holder is locked for too long
* Rework test, move into stream records test
* Fix typos
* Don't lock shard holder for a long time in stream records transfer (#8374)
* [ai] Detach shard holder lock from sending update batch
* Fix clippy warning
* Move stream records delay up to sleep in the middle of the batch
* remove unused code
---------
Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com>
---------
Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com>
* Use array_windows when statically sized windows were used
* Bump MSRV to 1.94
* fix in gpu code
---------
Co-authored-by: Luis Cossío <luis.cossio@outlook.com>
* Replace deduplication binary heap with kmerge
* Rework deduplication finding, chunk point IDs and only keep highest
* Simplify point group iterator
* Add benchmark
* Fix tests, reverse version
* Reformat
* Add integration test to assert all queued updates are also transferred
* Plunge update queue in stream records transfers
* Migrate existing plunger usages to new plunge helper
* Skip test if not compiled with staging flag
* Only send delay operation when staging feature is enabled
* Reformat
* Fix review remarks
* Use RwLock for pending changes in MmapBitSliceBufferedUpdateWrapper
* Use RwLock for pending operations in DatabaseColumnScheduledDeleteWrapper
* Use RwLock for pending updates in DatabaseColumnScheduledUpdateWrapper
* Drop alive guard before reconciliation, we don't touch files after
* Remove redundant clone
* Update comments
* Add test to reproduce broken WAL delta after aborting stream records
* Add staging env var to slow down stream records transfers for test
* Tweak test formatting and utilities a bit
* Add comment to test, link to PR describing bug
* Update test so it still succeeds with patched behavior
* Fix broken WAL delta after stream records abort (#7791)
* Make set_replica_state async
* Add function called when active state of local replica changes
* Add snapshot for newest clocks
* Bump newest clocks snapshot on replica deactivation
* Use newest clocks snapshot during recovery
* Add enum for specifying whether to take or clear clocks snapshot
* Store clock snapshot inside clock map, removing extra file
This greatly simplifies state handling. It also prevent any kind of
desynchronization because all newest clocks are always persisted
atomically.
* Immediately persist clocks after taking snapshot
* Always update snapshot, only take if missing
* Take clock snapshots through each shard flavor, including proxies
* Propagate dedicated functions for taking and clearing clocks snapshot
* Only persist clocks immediately if changed on snapshot/clear
* Simplify recovery point logic, always take clocks snapshot if exists
* Remove unwrap
* Fix typo
* Fix doc comment
* Transfer driver is async, use Tokio sleep
* Reduce visibility
* Move pyproject.toml to root
* Migrate pyproject.toml from Poetry to uv
* Update GH workflows
* Update test script, doc and nix to use uv
* Use latest uv
* Fix uv.lock
* Cleanup shell.nix
* Cleanup
- Explicit `uv sync` is not required, `uv run` will install deps
automatically.
- We don't provide a python package, so the `[build-system]` section
is not needed.
* Fix UV_VERSION inconsistency
---------
Co-authored-by: tellet-q <elena.dubrovina@qdrant.com>
Co-authored-by: xzfc <xzfcpw@gmail.com>
* Separate set and unset lists for each Gridstore pointer update
This makes the list of pointer updates much easier to grasp. The
simplification is desired because this exact structure has been a cause
for bugs multiple times now.
* Add more aggressive debug assertions
* Use consistent terminology
* Correct removal of set
* Simplify drain function
* Add safe guard to ignore empty pointer updates
* Rework tracker getter, explicitly branch variants
* Patch transmute, require Sized type
* rename and adjust comments
- set/unset is now current/to_free
- adjusted descriptions and comments for this nomenclature too
* clippy
* clippyyy
not 4, not 2, but 3 spaces
---------
Co-authored-by: Luis Cossío <luis.cossio@outlook.com>
* Fix Gridstore tracker drain and persist not working properly
* Patch existing test
* Add new test to assert buggy scenario we found
* Mention PR in test
* Add lease structure to invalidate pending flushers after wipe/clear
* We don't use bitmask as barrier anymore
* Add missing early return
* Add test
* Replace flush lease with is alive boolean we also use elsewhere
* Reimplement test, now it does fail on the old implementation
* In test, cover both new fixed and old broken flushing
* Remove old test case because it is flaky
* Fix clippy
* Fix outdated comment
* On Windows ARM64 builds, disable usage of neon
* Also disable optimized popcount on Windows ARM64
* fix quantization build
* revert changes in BQ
---------
Co-authored-by: Ivan Pleshkov <pleshkov.ivan@gmail.com>
* Fix delete_page not dropping sequential mmap
* In Gridstore Page, support using a single mmap
* In Gridstore Page, only open multiple mmaps if supported at runtime
* Support opening single mmap in UniversalMmapChunk
* Rename environment var, log warning when QDRANT_NO_MULTI_MMAP is set
* Support opening single mmap in MmapDenseVectors
* Remove unused result response
* Import LazyLock
* Disable rocksdb compile time feature by default
* Also disable RocksDB feature in segment crate
* Enable RocksDB feature in all CI builds
* Remove extra job for testing non-RocksDB build, it's the default now
* Keep RocksDB structures in generated OpenAPI schema
* Fix obsolete --workspace flag breaking builds with explicit features
* Also build including RocksDB in e2e tests on CI
Tim Visée