mirror of
https://github.com/qdrant/qdrant.git
synced 2026-07-27 13:11:06 -05:00
dev
115 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tim Visée
|
be543561e5 |
Add Logstore and Blobstore wrapper (#9673)
* 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> |
||
|
xzfc
|
48e710ce0b |
Cleanup UniversalRead methods interface (#9934)
* use common::generic_consts::{Random, Sequential};
* UniversalRead::read_batch: generic over E
* UniversalRead::read_batch: pass `AccessPattern` as ZST arg
* UniversalRead::read_bytes_iter: pass `AccessPattern` as ZST arg
* UniversalRead::read_iter: pass `AccessPattern` as ZST arg
* UniversalRead::read: pass `AccessPattern` as ZST arg
* UniversalRead::read_bytes: pass `AccessPattern` as ZST arg
|
||
|
Tim Visée
|
71e6c70458 |
Universal IO: append to file (#9720)
* 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. |
||
|
Andrey Vasnetsov
|
1a61d0b09c |
Fix live-reload staleness of in-place-mutated files on caching backends (#9812)
* Mutate same-operation slots in place in append-only mode With append_only_mutations enabled, every mutation of an existing point clones it to a fresh internal id. Shard-level updates decompose one point write into several SegmentEntry steps (upsert_with_payload issues upsert_point plus set_full_payload/clear_payload), so a single upsert burned one slot per step, leaving a chain of immediately-dead clones. A slot whose version already equals op_num was written by an earlier step of the current operation. It cannot be durable yet — the segment write lock is held across the whole operation, so no flush (and no read-only follower) can have observed it, and versions flush last so a crash discards it and WAL replay re-applies the whole operation. Mutate such slots in place: one operation now allocates exactly one slot regardless of how many steps it decomposes into. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Stamp payload storage version in write_point_parts overwrite_payload mutates payload storage, but the fused write never bumped version_tracker's payload version — the old CoW path did, via set_full_payload. The segment manifest would stamp payload files with a stale version, letting a partial snapshot skip payload storage that contains the moved point's row, so the restored id tracker would point at an offset with no payload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Resync ReadOnlyRoaringFlags from disk on live-reload The flags file is preallocated to power-of-two capacity and mutated in place within its length, which the held handle's reopen() — an append-only-growth contract on caching backends — never picks up: bits the writer changes inside already-cached DiskCache blocks stayed stale forever on shard followers. Drop the `impl LiveReload for ReadOnlyRoaringFlags` altogether: this storage holds arbitrary flags with no notion of points, so a point-delta interface (deleted/new points) did not belong here — open never applied deleted points either. Replace it with an inherent live_reload(fs) that opens a fresh StoredBitSlice (a fresh open always mirrors the current remote bytes), resyncs the materialized bitmap from it, and swaps the handle. The on-disk flags are the sole source of truth. To make refresh-by-fresh-open safe while the old handle is still alive, every DiskCache open now mirrors into a uniquely-named local file (.{pid}-{counter} suffix) and removes it on drop. Mirrors were already truncated on every open (block validity is in-memory only), so the stable name carried no state. Also add trace logging for live-reload consumed mapping changes and pending deltas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Resync immutable id tracker deleted bitmap via fresh handle on live-reload The immutable tracker's id_tracker.deleted file is a fixed-size bitmap whose bits the writer flips in place — its length never changes — so the held handle's reopen(), an append-only-growth contract on caching backends, never picked it up: the pre-deletion state cached at open was served forever and live-reload never reported deletions on shard followers. live_reload now takes the fs, opens a fresh StoredBitSlice (a fresh open always mirrors the current remote bytes), diffs it against the tracker's current state — mappings already reflect every previously reported deletion, so they are the baseline — and swaps the handle in. ReadOnlyIdTrackerEnum and the segment-level reload pass the fs through; the appendable and disk-resident variants keep their no-arg reloads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Resync disk id tracker deleted bitmap via fresh handle on live-reload Same staleness as the immutable tracker: the deleted file is a fixed-size bitmap mutated in place, so reopen() — append-only-growth on caching backends — served the pre-deletion state forever. live_reload now takes the fs, opens a fresh StoredBitSlice, and swaps it into deleted_file, so the per-point get_bit lookups read fresh state from then on too. The deleted_full take/diff/set baseline logic is unchanged. The enum's DiskResident arm passes the fs through. The regression test now covers both trackers over DiskCacheFs; the disk leg was verified to fail under the old reopen-based behavior. The disk tracker's versions file staleness is a separate, still-open case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Introduce header-stateless ReadOnlyTracker for gridstore live-reload The gridstore tracker file is preallocated and mutated in place (header rewrites, slot updates), which the reader's held handle never picked up on caching backends: Tracker::live_reload read the header through the stale handle — before reopen(), and reopen() would not have refreshed cached blocks anyway — concluded "no new pointers", and never reloaded pages. New points' payloads read as empty on shard followers. Replace the reader's tracker with a dedicated ReadOnlyTracker that holds nothing but the storage handle: reads don't need header state (slot addressing is positional, unwritten slots read as None in the zero-initialized file) and readers have no pending-updates buffer. Its live_reload opens a fresh handle and swaps it in. Tracker::live_reload is deleted. A new TrackerRead trait (max_point_offset/get/iter) is implemented by both the writable Tracker and ReadOnlyTracker, and GridstoreView is generic over it, so writer and reader share the read logic. max_point_offset reads the stored header count as plain data through the current handle (fresh as of the last reload) rather than deriving it from slot capacity: sparse vector storage builds total_vector_count from it, and the capacity of the preallocated file (1MB -> 65536 slots) would inflate every follower segment's sparse count. The method is now fallible; consumers propagate the error. GridstoreReader::live_reload refreshes tracker and pages unconditionally — the tracker carries no reliable cheap change signal. Making this incremental again is a deferred follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Re-open last held chunk on chunked-vectors live-reload Chunk files are preallocated to full size, so appended vectors are in-place writes within the existing file length. On caching backends a held handle keeps serving blocks fetched earlier — and a block fetched near the old tail extends into then-unwritten space — so vectors appended into that block read back as stale bytes after a reload. Mirror Pages::live_reload: on a len change (the status file is read through a fresh open every reload, so it stays a reliable gate), drop and re-open the last held chunk — the only one that can have gained vectors — alongside adopting newly created chunk files. Earlier, fully-filled chunks keep their handles; their contents cannot change. Covers dense, multi-dense, and quantized-chunked storages, which all delegate to ChunkedVectorsRead::live_reload. Avoiding the whole-chunk refetch per reload is a deferred follow-up, together with the analogous gridstore pages/tracker case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix lint: fs_err::create_dir_all in tests, drop unnecessary cast CI clippy runs with --all-targets and disallows std::fs methods in favor of fs_err; the new live-reload regression tests used std::fs::create_dir_all directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Tombstone-only deletes on non-appendable segments in append-only mode The tombstone-only delete path existed but was gated on is_append_only(), which requires the segment to be appendable. Deletes landing on non-appendable segments — notably the CoW update deleting the point's old copy — still went through delete_point_internal, which clears the payload row in place before the id-tracker drop. Clearing the payload destroys committed state of an offset that stays visible to live-reload followers until the drop is flushed: a follower refreshing in that window resolves the point through its (unchanged) id-tracker view and reads an empty payload — observed as a one-refresh {} payload flicker on a point update. Writer-side flush ordering cannot close the window, since the follower samples the id tracker and the payload storage at different instants; the versions commit protocol protects inserts exactly because the marker is read first, and deletes have no marker. This is the same failure class for which vector-deletion propagation was disabled (see the comment in delete_point_internal). Gate deletes on the new is_append_only_delete() — append_only_mutations alone, no appendability requirement: tombstoning needs nothing from the segment but the id tracker. Normal deployments keep eager payload clearing and prompt space reuse; clone-based mutations keep requiring appendability via is_append_only(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: chunked vectors reload --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Daniel Boros <dancixx@gmail.com> |
||
|
Luis Cossío
|
1299aa6dae |
[UIO] Add and use Populate::Partial variant (#9769)
* add (and handle) `Populate::Partial` * use partial populate * no OOB error |
||
|
Luis Cossío
|
4a48017fb7 |
add preopen functions to gridstore (#9728)
Additionally, propagate Populate for Tracker |
||
|
Luis Cossío
|
7db2ba3c2e |
[AI + manual] cleanup (#9727)
- get rid of `from_file` abstractions - renames: - `CachedFs` to be the implementation - `CachedReadFs` to be the trait |
||
|
Andrey Vasnetsov
|
e1d901647e |
CachedReadFs: prefetch-backed read-only segment opens (#9712)
* Add CachedReadFs: prefetch-backed read-only universal-io filesystem Snapshots the file listing at construction and serves opens from explicitly prefetched handles (take-once, shared across clones via Arc<Mutex>). A non-prefetched open falls back to a direct open on the inner filesystem, panicking in debug builds and warning in release. CachedFile is a transparent wrapper needed to satisfy the bidirectional UniversalReadFs<File = Self> pinning, following the ReadOnly pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Serve read-only segment opens from CachedReadFs prefetch pool ReadOnlySegment::open builds a per-segment CachedReadFs: the files known in advance (version.info, segment.json) are scheduled before the listing snapshot is taken so their fetch overlaps the listing round-trip, then every remaining listed file is scheduled, running all fetches in parallel instead of serializing them inside component opens. Existence checks and format-detection probes are answered from the snapshot without touching the inner filesystem. Stored handles are taken out of the pool via the new CachedReadFs::take_file, which returns the raw inner file — component types stay over plain S, and CachedFile exists only transiently inside open-read-discard helpers (read_json_via etc.) through the trait impl. The read-only open path takes &CachedReadFs<S::Fs> concretely; storing wrappers gained from-file constructors (StoredBitSlice::from_file, UniversalHashMap::from_file, ReadOnly::from_file, gridstore Tracker::open_cached / Pages::open_cached, read_chunks_cached). Snapshot-less, CachedReadFs is a passthrough to the inner filesystem — used by reload paths, writable build paths that reuse the on-disk index opens, and tests, all of which keep their previous behavior. Components that retain a filesystem for later reloads store the raw inner backend (CachedReadFs::inner), never the stale snapshot. Also: local_list_files now recurses into subdirectories, matching the flat key-prefix semantics of object-store listings; the immutable id tracker probes its defining file via exists (free on the snapshot) instead of a probe-open that would consume the take-once handle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Group imports per nightly rustfmt Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix sparse search bench for open_ro over CachedReadFs CI clippy runs --all-targets; the bench target was missed by the --tests sweep. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Match listing prefixes by path component, not by string The recursive local_list_files compared whole path strings; on Windows a joined prefix mixes `/` and `\` (`shard\index/chunk_`) while walked entry paths use `\` throughout, so nothing ever matched (broke list_files_returns_paths_relative_to_shard_dir on Windows CI). Match the entry name at the prefix's final position against the prefix's final component instead, then walk matched directories exhaustively — same semantics, separator-agnostic. Apply the same component-based matching to the CachedReadFs snapshot filter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * do not cache everything * fmt * dont unwrap files_info * fix clippy * relax debug assertion for now * [AI] refactor into extension trait, relax Fs<->File requirement (#9725) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Luis Cossío <luis.cossio@outlook.com> |
||
|
Arnaud Gourlay
|
f4e863321c | Remove dead code (#9719) | ||
|
Arnaud Gourlay
|
cad112bb1c |
Fix Clippy 1.97 (#9716)
* Remove from_iter_instead_of_collect from workspace lints The lint was removed from clippy (beta) and now triggers renamed_and_removed_lints warnings in every crate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::chunks_exact_to_as_chunks Replace chunks_exact with a constant chunk size by as_chunks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::needless_late_init Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::useless_borrows_in_formatting Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::uninlined_format_args Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix clippy::for_kv_map Iterate map values directly instead of discarding keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Allow clippy::result_large_err on QueueProxyShard::new_from_version The Err variant intentionally hands the LocalShard back to the caller. Same pattern as the existing allow on ForwardProxyShard::new. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Allow clippy::result_unit_err on wait_for_consensus_commit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
fa3d756914 |
[UIO] Split UniversalReadFileOps into read and write traits (#9682)
UniversalReadFileOps mixed read-side operations (from_context, list_files, exists) with mutating ones (create, create_dir, remove, remove_dir, atomic_save). Move the mutating operations to a new UniversalWriteFileOps subtrait, mirroring UniversalRead/UniversalWrite. - UniversalWrite now requires Fs: UniversalWriteFileOps, so generic consumers (gridstore) keep reaching write ops through S::Fs. - ReadOnlyFs drops its runtime-erroring write stubs: read-only is now a compile-time property. - DiskCacheFs implements the write side only when the remote fs does. - BlobFs and the async AsyncRead trait are read-only: the async write methods are removed from AsyncRead and its backends (object store, uio-grpc) along with the tests that exercised them. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Luis Cossío
|
80c9454141 |
[UIO] Include file size in UniversalReadFileOps::list_files (#9675)
* [AI + manual] Include file size in `UniversalReadFileOps::list_files` * Use dedicated `ListedFile` struct instead of `(PathBuf, u64)` pair Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: generall <andrey@vasnetsov.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
8c8a72d120 |
Remove read_multi_iter to fix macOS linker symbol overflow (#9643)
* remove unused iter_offsets
* Replace MultivectorOffsetsStorage::iter_offsets with callback-based for_each_offset
First step of removing the iterator-returning read API (whose deep,
composable generic types blow up mangled symbol size). Convert the
offsets read from an iterator to a callback the caller pushes into:
- trait method iter_offsets -> for_each_offset(ids, FnMut(usize, MultivectorOffset))
returning common::universal_io::Result<()>
- Mmap impl now uses the callback read_batch (drops one read_iter use)
- Ram / Chunked impls push into the callback; Chunked still goes through
iter_vectors for now (converted in a later step)
- the single caller (for_each_in_multi_batch) passes a closure
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Implement read_batch directly on ReadPipeline, not via read_iter
read_batch now drives the pipeline itself (refill-then-wait loop, like
read_multi_iter) and invokes the callback per result, instead of
consuming the iterator returned by read_iter. A step toward removing the
iterator-returning read API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Remove the ReadMulti RPC from the StorageRead gRPC service
ReadMulti was the only real consumer of UniversalRead::read_multi (which
itself relies on read_multi_iter). Removing the RPC end-to-end clears the
path to dropping that read API. StorageReadService keeps all its other
RPCs (ListFiles, FileExists, FileLength, ReadBytes, ReadBytesStream,
ReadWhole, ReadBatch).
- proto: drop `rpc ReadMulti` + ReadMulti{Entry,Request,Response}
- regenerated lib/api + uio-client generated code; drop ReadMulti
validation rules in lib/api/build.rs
- tonic: delete the read_multi handler + its 2 tests
- uio-client: delete Client::read_multi, the mock-server impl, and 2 tests
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Remove UniversalRead::read_multi
Its only real consumer was the StorageRead ReadMulti gRPC handler (removed
in the previous commit); the two wrapper forwarders had no callers. Drop
the trait method and both forwarders (typed/read_only), and remove the
io_uring test that only existed to compare read_multi vs read_multi_iter
(read_multi_iter stays covered by the other tests). Another step toward
removing the iterator-returning read API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Drive ReadPipeline directly in gridstore read_from_pages
Replace the read_multi_iter call in Pages::read_from_pages with a direct
pipeline loop (refill-then-drain), scheduling each multi-page read on its
own page file. Behavior unchanged; another step toward removing the
iterator-returning read_multi_iter.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Drive ReadPipeline directly in gridstore read_batch_from_pages
Replace the second read_multi_iter call (in Pages::read_batch_from_pages)
with a direct pipeline loop, scheduling each (ReadMeta, page, range) on its
own page file and propagating errors via GridstoreError. Single/multi-page
buffering and out-of-order reassembly are unchanged. No more read_multi_iter
in gridstore.
Measured overhead on warm mmap (both paths are zero-copy borrows): ~0.3 ns
per read of fixed control cost, flat across read sizes — well under 0.1% of
a real payload read.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Drive ReadPipeline directly in on-disk postings with_posting_views
Replace read_iter in OnDiskPostings::with_posting_views with a direct
pipeline loop. wait_bytemuck yields a file-borrowed Cow, so postings are
still stored zero-copy in raw_postings (a read_batch swap would have forced
an owned copy of every posting list per query on the mmap backend).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Read on-disk posting headers via read_batch, drop the HeadersBatch iterator
headers_iter now reads headers with the callback read_batch API: each header
is parsed (copied) out of the read bytes, so nothing borrows the file past the
read — no pipeline needed. Since the read is now eager, HeadersBatch holds the
collected Vec<HeaderResult> directly instead of a Box<dyn Iterator>, dropping
the boxing, the dynamic dispatch, and the struct's lifetime parameter.
with_posting_views takes the Vec and still pipelines the posting reads.
Removes the last read_iter use in this file.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Use read_batch in simple_disk_cache populate_from
populate_from reads one byte per block only to fault blocks into the local
cache, discarding the bytes — a no-op-callback read_batch fits exactly. Drives
the same DiskCachePipeline as before; one less read_iter caller.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Implement read_iter directly on ReadPipeline, not via read_multi_iter
read_iter now drives the pipeline itself (refill-then-wait loop, mirroring
read_bytes_iter) instead of mapping its ranges onto self and calling
read_multi_iter. Same signature and iterator contract, so all callers are
unchanged. Leaves iter_vectors as read_multi_iter's only remaining caller.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Revert "Drive ReadPipeline directly in on-disk postings with_posting_views"
This reverts commit
|
||
|
Roman Titov
|
11abd4bed5 |
Unify Borrowed and Owned read pipeline implementations (#9498)
|
||
|
Andrey Vasnetsov
|
9636944402 |
feat: require UserData to implement Debug (#9588)
Add a `Debug` supertrait bound to the `UserData` marker trait and propagate it through the universal-io read pipelines and the read APIs built on them. This lets pipeline user-data (request ids, point ids, internal read metadata) be formatted for diagnostics. Wrapper types that carry user data through the pipelines (`RemoteMeta`, gridstore `ReadMeta`, hashmap `Entry`) gain `Debug`, and the `U: UserData` bound is threaded through `read_vectors`/`read_payloads`/ `read_values`/`iter_vectors` and their implementors in gridstore and segment. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Luis Cossío
|
8fc7b35d7a |
Explicit Populate in payload storage (and ` (#9514)
`GridstoreReader`) - Make the entrypoint of payload storage choose the `Populate` variant - Mutable payload indexes now populate gridstore blockingly before using it to load - Propagate `Populate` into `flags` module - Add `UniversalRead::populate_auto` to know whether a backend chooses to populate or not when `Populate::Auto` |
||
|
Roman Titov
|
efc0c7961b |
Use batched reads in GridstoreView::iter (#9429)
|
||
|
Roman Titov
|
3efb9501ba |
Explicitly propagate fs into Gridstore (#9381)
|
||
|
Roman Titov
|
1e5013bcae | Add io_uring based payload storage type (#9310) | ||
|
Arnaud Gourlay
|
9c71941212 | Harden Gridstore model testing (#9368) | ||
|
Arnaud Gourlay
|
06a2133ea8 |
fix(gridstore): honor pending pointer-unset in batched reads (#9330)
* fix(gridstore): honor pending pointer-unset in batched reads `Tracker::get_batch` only handled the `Some` arm of a point's pending pointer update. When a point had a pending pointer-unset (`current == None`) over a value already flushed to disk, the batched lookup fell through to the stale persisted pointer, so `read_values` resurrected the deleted value while the single-read `get`/`get_value` correctly reported it gone. This split the payload read paths: `retrieve`-by-id (batched `read_payloads`) returned a stale payload while `scroll`/filter (single `get_value`) returned the correct empty one — surfacing as a payload mismatch after a filter-driven clear. Honor the pending update either way in `get_batch`, mirroring `get`. Add a regression test covering put+flush then delete-without-flush; the pre-existing congruence test missed it by only deleting never-flushed points (no stale persisted pointer). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * sigh --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Roman Titov
|
7727bf3f35 | Use batched reads in vector and payload storage (#9113) | ||
|
Tim Visée
|
7041b81f76 |
Use assert_matches! (#9231)
* Use assert_matches! * Add trailing commas * Use more assert_matches! Also, drop now redundant `expected blah but got blah` messages because `assert_matches!` will print these. * Use debug_assert_matches! --------- Co-authored-by: xzfc <xzfcpw@gmail.com> |
||
|
Andrey Vasnetsov
|
079b15d22f |
feat/readonly map index with live reload (#9264)
* live reload function for map index * fmt |
||
|
Daniel Boros
|
4d19a018af |
feat: add open for read only map index (#9223)
* feat: add open methods for map * fix: review comments * feat: detect not-found via error instead of path.exists in map index open Replace the path.exists() pre-check in the read-only appendable map index open path with error-driven detection through ok_not_found(). Generalize OkNotFound over an IsNotFound trait, implemented for io::Error, mmap::Error, UniversalIoError, and GridstoreError so a NotFound surfacing through any layer (including mmap's inner io::Error) maps to Ok(None). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: revert internal error conversion --------- Co-authored-by: generall <andrey@vasnetsov.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Daniel Boros
|
47cfd37881 | feat: add writable option for gridstore open (#9210) | ||
|
Luis Cossío
|
815b55deb0 | flush BitmaskGaps before reopening (#9186) | ||
|
Luis Cossío
|
48363df9d7 |
[UIO] Use UniversalRead::reopen (#9127)
* use universal reopen * remove unused fs args |
||
|
Andrey Vasnetsov
|
daa22f15e6 |
[UIO] Split UniversalReadFileOps into filesystem + file traits (#9151)
* [UIO] Split UniversalReadFileOps into filesystem + file traits
`UniversalReadFileOps` is now an instance-based trait describing a
filesystem handle (list/exists with `&self`, plus `from_context`). A new
`UniversalReadFs: UniversalReadFileOps` subtrait adds the
`open(&self, path, options) -> Self::File` capability with `type File:
UniversalRead`. `UniversalRead` no longer extends `UniversalReadFileOps`
and is purely a file-handle trait.
This separates "filesystem instance" from "file handle". Backends that
need per-instance configuration (S3 bucket name + credentials, mmap
default advice, io_uring runtime, block-cache controller `Arc`) gain a
typed home in `Self::ContextConfig`, and `list_files`/`exists`/`open`
become `&self` methods on the filesystem handle.
Concrete filesystem handles introduced for the three existing backends:
- `MmapFs` — unit struct; `ContextConfig = ()`; produces `MmapFile`.
- `IoUringFs` — carries `prevent_caching`; `ContextConfig =
IoUringConfigContext`; produces `IoUringFile`. `IoUringConfigContext`
becomes the construction input rather than a per-open argument.
- `BlockCacheFs` — carries `Arc<CacheController>`; `ContextConfig =
BlockCacheConfigContext`; produces `CachedSlice`.
`TConfigContext` (universal builder methods) is kept so generic-over-`Fs`
code can still set cross-backend knobs via
`Fs::ContextConfig::default().with_prevent_caching(true)`.
Wrappers (`ReadOnly<S>`, `TypedStorage<S, T>`, `StoredStruct<S, T>`),
higher-level storages (`StoredBitSlice<S>`, `UniversalHashMap<K, V, S>`)
keep `<S: UniversalRead>` parameterization but their `open(...)`
constructors now grow a `fs: &Fs` argument bound by
`Fs: UniversalReadFs<File = S>`. `read_json_via` becomes
`read_json_via(fs: &Fs, path)`.
All test code and benches in `common` updated to construct
`MmapFs`/`IoUringFs` inline as needed. `common` compiles cleanly with
tests and benches. `gridstore`, `segment`, and `tonic` caller updates
are in flight in subsequent commits.
* WIP: gridstore + segment caller sweep (partial)
Threads `fs: &Fs` through gridstore's `BitmaskGaps`, `Bitmask`, `Pages`,
and `Gridstore::new`/`open`/`create_new_page`. Most segment callers
have `OpenOptions { extra: ... }` removed and `S::open(path, opts, ctx)`
sites updated mechanically but the trait change is not yet propagated.
Does NOT compile yet. Tracker still has static `S::open` calls, segment
generic constructors (`MmapInvertedIndex<S>`, `UniversalMapIndex`,
`StoredGeoMapIndex`, etc.) still call `S::open`/`S::list_files`/`S::exists`
statically — they need an `fs: &Fs` parameter added. Tonic API
`StorageReadService<S>` also unconverted.
Committed as branch checkpoint; cascade continues in subsequent work.
* gridstore: thread `fs: &Fs` through Bitmask, BitmaskGaps, Pages, Tracker
Per the new `UniversalReadFs` shape, every constructor/method that opens
files takes an `fs: &Fs` parameter. Gridstore is currently mmap-only,
so the top-level `Gridstore` / `GridstoreReader::open` callers in the
crate pass `&MmapFs` inline. Tests do the same.
gridstore lib + tests now compile cleanly. Segment + tonic cascade
still pending.
* WIP: segment caller sweep — dynamic_stored_flags first
* WIP: segment cascade - id_tracker partial
* common benches: update to new UniversalReadFs::open shape (clippy clean)
* segment flags: thread Fs through BufferedDynamicFlags / Bitvec / Roaring
DynamicStoredFlags::set_len now takes `fs: &Fs`. BufferedDynamicFlags
stores an `Arc<Fs>` so the flusher closure can call `set_len` on resize.
BitvecFlags and RoaringFlags expose a new `Fs` type parameter and the
flag tests now pass `Fs::default()` (MmapFs/IoUringFs via duplicate_item).
Concrete consumers (bool/null index, mmap dense/multi/sparse storages)
pin `Fs = MmapFs` and pass `&MmapFs` to inner opens.
* segment: thread Fs through field-index lifecycle methods
Apply the new UniversalReadFs::open shape across:
- full_text_index (MmapInvertedIndex, MmapFullTextIndex, UniversalPostings)
- geo_index (StoredGeoMapIndex build/open + tests + builders)
- numeric_index lifecycle (UniversalNumericIndex build/open)
- map_index lifecycle (UniversalMapIndex build/open)
- stored_point_to_values (open / from_iter)
Concrete consumers pin Fs = MmapFs and pass &MmapFs inline; generic
open paths thread `fs: &Fs` where Fs: UniversalReadFs<File = S>.
* segment: thread Fs through chunked vectors and id-tracker callers
- ChunkedVectors gains `Fs` generic so add_chunk can call create_chunk
after open. ChunkedVectorsRead/load_config and chunks::{read_chunks,
create_chunk} take `fs: &Fs`. Concrete callers (dense / multi-dense /
sparse / quantized) pass MmapFs inline.
- DenseVectorStorageImpl stores `fs: Fs` so `update_from` can reopen
ImmutableDenseVectors. ImmutableDenseVectors::open takes `fs: &Fs`.
- VectorStorageEnum DenseUring* variants thread IoUringFs alongside
IoUringFile.
- segment_builder + segment_constructor_base pass MmapFs to
ImmutableIdTracker::{new, open}.
- QuantizedStorage::from_file takes `fs: &Fs`; quantized_vectors callers
pass &MmapFs.
* segment: apply nightly rustfmt after Fs refactor
cargo +nightly fmt --all over the segment crate after the
UniversalReadFs cascade. No semantic changes.
* segment: thread Fs through benches and id-tracker tests
Update the dynamic-mmap-flags and buffered-update-bitslice benches to
the new UniversalReadFs::open shape (pass `&MmapFs`). Update the
immutable-id-tracker test suite to forward `&MmapFs` to
`from_in_memory_tracker` / `open`.
* uio: pin Fs via UniversalRead::Fs assoc type; per-call OpenExtra
Two design changes that fall out of the per-instance Fs refactor:
1. Bidirectional Fs ↔ File pinning. `UniversalRead::Fs:
UniversalReadFs<File = Self>` lets generic-over-`S` code refer to
`S::Fs` directly instead of carrying an extra `<Fs: UniversalReadFs<File = S>>`
generic param. `ReadOnly<S>` wraps a file but has no natural
filesystem; a phantom `ReadOnlyFs<S::Fs>` satisfies the constraint
while inherent `ReadOnly::open` keeps taking `&S::Fs` directly.
2. `prevent_caching` moves from filesystem-instance state to per-call
`UniversalReadFs::OpenExtra: Default`. Was previously a knob on
`IoUringConfigContext` / `IoUringFs`, conflating "how this fs is
built" with "how this file is opened." Now `IoUringFs::OpenExtra =
IoUringOpenExtra { prevent_caching }`; mmap and block-cache use `()`.
`IoUringConfigContext` is gone, `TConfigContext` slims to a `Default`
marker.
Tonic StorageReadService holds `Arc<S::Fs>` (was `PhantomData<S>`); its
`new()` builds via `S::Fs::from_context(default)` and the spawn_blocking
closures clone the Arc to call instance methods.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* uio: cfg-gate IoUringOpenExtra import for non-linux builds
The IoUringOpenExtra reexport from `universal_io` is gated on
`target_os = "linux"`. The previous commit left an unconditional import
in `persisted_hashmap/tests.rs`, breaking macOS/Windows CI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* uio: migrate simple_disk_cache to per-instance Fs API
PR #9097 (merged into dev concurrently with this branch) introduced a
`DiskCache<R: UniversalRead>` using the pre-refactor trait shape:
file-handle-as-filesystem (`R::open`, `R::list_files`), an
`OpenOptionsExtra` field on `OpenOptions`, and trait methods without
`&self`. The Fs-instance refactor on this branch removed all three.
Reshape `simple_disk_cache` to match the new design without breaking the
lazy mirror semantics:
- New `DiskCacheFs<R>` is the filesystem handle. Holds a clone of the
remote `R::Fs`; `list_files`/`exists` delegate; `from_context`
forwards to the inner Fs context. `open` constructs a `DiskCache<R>`
via the global `DiskCacheConfig`.
- `DiskCache<R>` now stores `remote_fs: R::Fs` + `remote_extra:
<R::Fs as UniversalReadFs>::OpenExtra`, so lazy remote opens go
through `self.remote_fs.open(path, options, extra)` instead of the
removed `R::open`. `open_with_config` takes the remote Fs + extra
explicitly (no more hard-coded `prevent_caching: true`; callers pass
the appropriate `OpenExtra`).
- `UniversalRead for DiskCache<R>` now declares `type Fs =
DiskCacheFs<R>` (no more `open` method on the file trait).
- Propagate the necessary bounds (`R::Fs: Clone`,
`<R::Fs as UniversalReadFs>::OpenExtra: Clone`,
`R::OwnedReadPipeline<u8, Range<u32>>: Send`) through `pipeline.rs`
free functions and impl blocks that reach into `DiskCache::remote` /
`local_state`.
- Drop the now-removed `extra: _` destructure in `LocalState::new`.
- Tests construct the remote Fs via `R::Fs::from_context(Default::default())`
and exercise `DiskCache::open_with_config`. 17 simple_disk_cache
tests pass; the 3 `empty_read_does_not_materialize_local_file`
failures pre-exist on dev (verified) and are unrelated.
`mold -run cargo clippy --all-targets` clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: nightly fmt on simple_disk_cache migration
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: nightly fmt for local_state imports after rebase
Co-authored-by: Cursor <cursoragent@cursor.com>
* uio: split DiskCacheFs into its own module
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* uio: replace TConfigContext with OpenExtra trait; move DiskCacheConfig onto DiskCacheFs
- Drop the empty TConfigContext marker; ContextConfig is now unconstrained
so backends can require explicit construction.
- Add OpenExtra trait with with_prevent_caching for backend-agnostic
per-call knobs; impl for () (no-op) and IoUringOpenExtra.
- DiskCacheFs now carries Arc<DiskCacheConfig> via the new
DiskCacheFsContext<C>; the prefill flow moves from the deleted
open_with_config into DiskCacheFs::open so Populate::Blocking /
PreferBackground work through the trait API.
- Remove the DiskCacheConfig global; callers must construct the context
explicitly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* style: nightly fmt after OpenExtra refactor
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs: fix spelling — Implementors → Implementers
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: possible panic insetad of error propagation
* fix: missing cfg annotation
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor Agent <agent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Daniel Boros <dancixx@gmail.com>
|
||
|
xzfc
|
97c612c5f9 |
Restore pre-UIO-migration OpenOptions behavior (#9107)
* [ai] segment dynamic_stored_flags: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/common/flags/dynamic_mmap_flags.rs#L115-L146 * [ai] segment immutable_id_tracker: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/id_tracker/immutable_id_tracker.rs#L249-L262 * [ai] segment geo_index: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/geo_index/mmap_geo_index.rs#L239 * [ai] segment geo_index: populate(Auto -> from(!is_on_disk)) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/geo_index/mmap_geo_index.rs#L239 * [ai] segment numeric_index: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/numeric_index/mmap_numeric_index.rs#L171 * [ai] segment map_index: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/map_index/mmap_map_index.rs#L68-L71 * [ai] segment map_index: populate(Auto -> from(!is_on_disk)) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/map_index/mmap_map_index.rs#L71 * [ai] segment full_text_index: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/full_text_index/inverted_index/mmap_inverted_index/mod.rs#L117-L133 * [ai] segment full_text_index: populate(Auto -> from(populate)) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/index/field_index/full_text_index/inverted_index/mmap_inverted_index/mod.rs#L133 * [ai] gridstore bitmask: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/gridstore/src/bitmask/mod.rs#L120 * [ai] gridstore bitmask: advice(Global -> Random) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/gridstore/src/bitmask/mod.rs#L40 * [ai] gridstore bitmask/gaps: advice(Global -> Normal) in create https://github.com/qdrant/qdrant/blob/v1.17.0/lib/gridstore/src/bitmask/gaps.rs#L103 * [ai] gridstore bitmask/gaps: populate(Blocking -> No) in open https://github.com/qdrant/qdrant/blob/v1.17.0/lib/gridstore/src/bitmask/gaps.rs#L119 * [ai] segment quantized_storage: need_sequential(true -> false) https://github.com/qdrant/qdrant/blob/v1.17.0/lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs#L32-L40 * [ai] gridstore pages: advice(Global -> Random) in attach_page https://github.com/qdrant/qdrant/blob/v1.17.0/lib/gridstore/src/page.rs#L38 * [ai] tonic storage_read_api: update OpenOptions for read-only handlers Rationale: - writeable: false — all 6 are read-only APIs; true would fail on read-only mounts. - need_sequential: false — opening a second mmap with MADV_SEQUENTIAL on Linux costs a VMA per call. For one-shot reads driven by a single gRPC request, the second mmap isn't justified — better to put the hint on the primary mmap via advice. - populate: No — these are one-shot reads, no point warming the page cache. - advice: Sequential for read_bytes_stream (streams the file in 1MB chunks) and read_whole (reads everything) — the kernel can read ahead. - advice: Normal for file_length (doesn't read content), read_bytes (single range), read_batch / read_multi (random ranges). |
||
|
xzfc
|
899857c4f5 |
Make OpenOptions explicit (#9104)
* Refactor: make OpenOptions explicit * Refactor: remove unused OpenOptions::disk_parallel * Refactor: do not wrap `advice` in Option * Refactor: Add OpenOptionsExtra |
||
|
Luis Cossío
|
ee739aff79 |
[UIO] Introduce Populate enum (#8946)
* introduce `Populate` enum * fix: apply CodeRabbit auto-fixes Fixed 6 file(s) based on 6 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai> * fix coderabbit fix * Use from rather than into * add `Auto` variant * fix rebase * fix rebase again --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: timvisee <tim@visee.me> |
||
|
Andrey Vasnetsov
|
dc1ec94ea0 |
make storage generic in ReadOnlySparseVectorStorage (#8964)
* make storage generic in ReadOnlySparseVectorStorage * fix tests |
||
|
Luis Cossío
|
6599f545da |
[UIO] Additional UniversalRead/Write usage simplifications (#8961)
* bytemuck::Pod already implies 'static * no supertraits * regions gaps with TypedStorage * StoredBitSlice with TypedStorage * bytemuck for PostingsHeader * bytemuck for TrackerHeader * bytemuck for MmapRange * bytemuck for stored_point_to_values::Header |
||
|
Luis Cossío
|
8aaeb96698 |
[gridstore, UIO] Simplify OptionalPointer (#8945)
* derive bytemuck::Pod on `OptionalPointer`, get rid of legacy transmute * remove one layer of Option in `get_raw` * keep `UniversalRead<u8>` bound, do bytemuck conversion at call site (#8953) Drop the `+ UniversalRead<OptionalPointer>` / `+ UniversalWrite<OptionalPointer>` trait bounds added in the previous two commits. Reads and writes of `OptionalPointer` and `TrackerHeader` now go through `bytemuck::from_bytes` / `bytemuck::bytes_of` on the existing `u8` byte slices instead. This keeps the unsafe-transmute removal but avoids the turbofish noise (`UniversalWrite::<u8>::flusher(&self.storage)`, `<S as UniversalRead<u8>>::open`, …) that two `T`s on `S` forced everywhere. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * extra refactoring after #8955 * explicit zeroed instantiation --------- Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Andrey Vasnetsov
|
64244330b1 |
Move element type from UniversalRead trait to method generics (#8955)
* Move element type from UniversalRead trait to method generics Lifts the `T` parameter off `trait UniversalRead<T>` (and the matching `UniversalWrite<T>`) and onto the read/write methods themselves. The `ReadPipeline` associated type becomes a GAT over `T`. With per-method generics, callers that need to read several element types from one storage just write `S: UniversalRead` instead of stacking `UniversalRead<u8> + UniversalRead<Counts> + ...`. Removes the workarounds the old shape required: - `TypedStorage<S, T>` newtype (sole purpose was disambiguating multi-bounds) - `UniversalReadFamily` HKT shim - `StoredGeoMapIndexStorage` four-bound trait alias - `CachedSlice<T>` is now non-generic; `T` moves to `get_range`/`len` No runtime behavior change: alignment in `IoUringRuntime` and `CachedSlice` is preserved because `T` is still known at each call site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Restore TypedStorage as a typed-access fail-safe Reintroduces `TypedStorage<S, T>` as a transparent wrapper around `UniversalRead`/`UniversalWrite` storage that fixes the element type to `T`. With per-method generics on the underlying traits, callers can otherwise read or write any `T` from the same handle; this wrapper binds it at the type level so accidental cross-type access fails to compile. The wrapper exposes inherent typed methods (`read::<P>`, `read_iter`, `write`, `len`, …) that delegate to the inner storage with `T` fixed. It does not implement `UniversalRead`/`UniversalWrite` itself — those are intentionally avoided to prevent the typed binding from being bypassed via the generic trait methods. Restores the wrapping at the previous call sites: `StoredStruct`'s inner storage, `ImmutableIdTracker`'s version mmap, the geo and numeric index storages, the chunked-vectors chunks, and the immutable dense vector storage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Silence clippy::len_without_is_empty on TypedStorage `TypedStorage::len` returns `Result<u64>` (a fallible byte length from the underlying storage), so an `is_empty` companion would also be fallible and offer nothing over `len()? == 0`. Suppress the lint at the impl block, matching how the underlying `UniversalRead::len` is already exempted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fmt --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
qdrant-cloud-bot
|
cdc3a2124e |
perf(tests): reduce test volume on Windows for IO-heavy tests (#8864)
Windows CI takes ~28min vs ~18min on Linux, with some individual tests running 10-50x slower due to Windows filesystem IO overhead. Since we only need functional compatibility on Windows (not performance testing), reduce iteration counts for the worst offenders: - WAL quickcheck: 10 → 3 iterations (saves ~350s) - Consensus manager proptests: 256 → 10 cases (saves ~100s) - Deferred point tests: reduce loop combinations (saves ~250s) - Gridstore test_behave_like_hashmap: 50k → 10k operations (saves ~200s) Co-authored-by: Cursor Agent <agent@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> |
||
|
Roman Titov
|
dc50c5292c |
Refactor Sparse*QueryScorer for (future) io_uring support (#8759)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Luis Cossío <luis.cossio@outlook.com> Co-authored-by: Andrey Vasnetsov <andrey@vasnetsov.com> |
||
|
Arnaud Gourlay
|
354bbb35e6 |
Delete unused code (#8771)
* Delete unused code * restore initialize_global * drop BadShardSelection * Remove now obsolete allow(dead_code) attributes * Remove more dead code --------- Co-authored-by: timvisee <tim@visee.me> |
||
|
Luis Cossío
|
5dc27660e5 |
[UIO, gridstore] generic Bitmask storage (#8501)
* [AI + manual] generalize BitmaskGaps storage * [AI + manual] use StoredBitSlice in Bitmask * use MmapFile as default storage * rename to read_all * duplicate import |
||
|
Andrey Vasnetsov
|
1ad626d983 |
clearing cache with pageout (#8654)
* use pageout to clear mmap cache * Also clear cache of deleted flags in mmap dense vector storage * Add reference to madvise man pages for probe logic * use deconstruct --------- Co-authored-by: timvisee <tim@visee.me> |
||
|
Tim Visée
|
74e51f7339 |
Claude: simplify codebase (#8627)
* [ai] Replace manual into mappings with Into::into * Reformat * [ai] Use implicit .iter * Don't iterate over keys too * [ai] Replace unwrap_or * Reformat * [ai] Use as_deref and then_some * [ai] Use more to_string * [ai] Use explicitly typed into conversions * Reformat * [ai] More explicit into conversions * Reformat |
||
|
xzfc
|
2d9d953a45 |
UniversalIO: generic RequestId (#8601)
* IoUringState: generic `RequestId` * UniversalRead: generic `RequestId` * Simplify `gridstore::Pages::get_page_value_ranges` Now we don't need two separate `SmallVec`s as we can put `buffer_offset` into `RequestId`. * Better doc comment * Rename `RequestId` -> `Meta` |
||
|
Luis Cossío
|
fa8b441454 |
Allow shorter reads in IoUring (#8466)
* use assume_init_vec * allow shorter reads at EOF * add prevent_caching openoption, allow short read dynamically * codespell * fix rebase |
||
|
Roman Titov
|
bcdf2d2577 |
Replace MmapUniversal with MmapFile (#8505)
|
||
|
Roman Titov
|
a7550f893f |
Universal MmapFile that can read any T (#8493)
|
||
|
Andrey Vasnetsov
|
454f11b515 |
Fix io_uring reads: use byte offset (#8470)
* [AI] use byte offset instead of element range * [AI] change ElementOffset -> ByteOffset |
||
|
xzfc
|
7bd5759105 |
Touch-up UIO traits (#8457)
* UIO traits: require Sized I don't think we ever going to add unsized implementation. So, lets, require it on the whole trait rather than on individual methods. * Move `AccessPattern` to the `common` package Was: segment::vector_storage::vector_storage_base::AccessPattern Now: common::generic_consts::AccessPattern * UIO traits: replace `bool` with `AccessPattern` |
||
|
Luis Cossío
|
ad8334928d |
Use UniversalIo for MmapBitSlice (#8339)
* feat(universal_io): add storage-agnostic BitSliceStorage with read/write support` Add BitSliceStorage<S> generic over UniversalRead<u64>/UniversalWrite<u64>, providing bit-level read and write operations over u64-element storage. Read operations (S: UniversalRead<u64>): - read_all: returns Cow<BitSlice> (zero-copy for mmap, owned for others) - get_bit: single bit read via u64 element fetch - read_bit_range: arbitrary bit range read Write operations (S: UniversalWrite<u64>): - set_bit / replace_bit: single bit write (skips write if unchanged) - write_bit_range: arbitrary bit range write from BitSlice source - fill_bit_range: fill range with a value - set_bits_batch: batch individual bit updates coalesced by element - flusher: flush underlying storage * refactor: replace MmapBitSlice with BitSliceStorage in MmapBitSliceBufferedUpdateWrapper Migrate all deleted-flag bitslice storage from the legacy MmapBitSlice (Deref-based mmap wrapper) to BitSliceStorage<MmapUniversal<u64>> (storage-agnostic universal IO backend). Updated consumers: - MmapMapIndex - MmapNumericIndex - MmapGeoMapIndex - MmapInvertedIndex (fulltext) - ImmutableIdTracker - Benchmark Additionally: - Add BitSliceStorage::create(path, num_bits) to encapsulate file creation + sizing + open (eliminates duplicated size math across 5 call sites) - Add MmapBitSliceStorage type alias for BitSliceStorage<MmapUniversal<u64>> - Add count_ones() convenience method - Use set_bits_batch() in wrapper flusher and all build paths instead of per-bit set_bit() loops (coalesces u64 read-modify-writes) - Fix silent error swallowing in wrapper get(): log + debug_assert on I/O errors instead of .ok().flatten() - Remove dead bitmap_mmap_size function from immutable_id_tracker * use bitvec's approach for offset * less api * misc improvements * refactor write method * move to segment crate * handle creation of file outside of StoredBitSlice logic * fix codespell * document bitwise calculations * coalesce more updates into a single write, batch all writes * use native `extend_from_bitslice` instead of iterating. * clarity refactor * clippyyy * use `u64` as BitStore everywhere * assume iterator is sorted * only use chunk_by for generating runs * fix update wrapper flusher * review fixes * larger set bits batch test * remove reintroduced file * oops, fix new test --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: generall <andrey@vasnetsov.com> |
||
|
qdrant-cloud-bot
|
5d148dcd61 |
Speed up slow unit tests (#8385)
Reduce test parameters across 5 areas to cut test runtime without
sacrificing coverage:
1. HNSW PQ tests: lower vector dimensionality from 131 to 64 for product
quantization variants, cutting PQ codebook training time (~31s -> ~8s)
2. HNSW index build: use 2 threads instead of 1 for index construction;
the tests only check accuracy above a threshold so determinism is not
required
3. Search attempts: reduce from 10 to 5 query vectors per test
4. Rescoring: skip the expensive re-upsert-all-as-zeros rescoring check
for PQ tests (already covered by scalar quantization variants)
5. Near-miss speedups:
- WAL: reduce QuickCheck iterations from 100 to 50
- Gridstore: halve operation counts, drop 64-byte block size case,
reduce proptest cases for gap search
- Continuous snapshot: reduce timeout from 20s to 10s
Made-with: Cursor
Co-authored-by: Cursor Agent <agent@cursor.com>
|