mirror of
https://github.com/qdrant/qdrant.git
synced 2026-09-29 01:17:56 -05:00
dev
915
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Tim ViséeandClaude Fable 5.1
|
81b9a84bd9 |
Fix stale snapshot transfer breaking cluster (#10643)
* Check shard transfer requests against the registered transfer source A sender that restarts replays committed but unapplied consensus entries before it has joined consensus, so a `Start` for a transfer that has since been aborted still spawns its driver. The receiver accepted such a stale sender: `initiate_shard_transfer` only required *some* transfer into the shard, and the pre-download clear in `recover_shard_snapshot` only required the replica not to be a source of truth. A `Partial` replica being populated by another transfer passed both, and got wiped. Have the sender identify itself in the internal `initiate` and shard snapshot `recover` requests, and have the receiver refuse both unless a transfer from that peer into the shard is registered. The field is optional, so requests from older peers keep the previous behavior. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Hold off a shard transfer driver until a consensus leader is established On startup, committed but unapplied consensus entries are replayed before the peer joins consensus. A `Start` or `Restart` replayed this way spawns the transfer driver right away, on a view of consensus that predates the restart: the transfer may already be aborted, and the peer is about to abort every transfer it is part of anyway once a leader is established (`cancel_related_transfers`). That driver still contacted the receiver, which acted on the stale transfer. Make the driver task wait until this peer knows a consensus leader before it touches the remote, so the entries that come with the leader, such as an abort of this transfer, are applied first. Stopping the task cancels the wait like any other stage of the transfer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Test that only the registered source may clear a shard for snapshot recovery Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Test a restarted sender replaying an aborted transfer start on a real cluster A staging-only delay in the sender's `Start` apply, before it registers anything, lets a test kill the sender while the entry is committed but not yet applied. That is the state the incident's sender crashed in: on restart, `Consensus::new` replays the entry before the peer rejoins consensus and spawns the driver of a transfer the cluster has aborted and replaced meanwhile. The test marks the sender `Dead` with an update while it is down, so the transfer aborts and the receiver is recovered from the other replica, then restarts the sender while the receiver sits in `Partial`. Without the fixes on this branch the replayed driver clears the receiver, its download is cut when the sender catches up, the dummy is marked `Active` by the legitimate `Finish`, and the receiver's consensus dies on the next transfer entry. With them the cluster converges. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Treat a dummy local shard as nothing to un-proxify Transfer restarts and aborts un-proxify the sender's local shard when applied. A dummy shard was reported as an unexpected type with a service error, and consensus apply treats a service error as fatal: consensus stops, and on the next start the replay of the same committed entry fails again before the peer opens raft networking, so it never comes back. That is how the receiver of a stale snapshot transfer ended up in a crash loop: its shard had been cleared under a transfer, the dummy was marked `Active` by the legitimate `Finish`, and the next transfer entry naming it as the sender was fatal. A dummy was never proxified, so there is nothing to revert. Return without error for it, and leave every other case as it was. A peer already stuck this way then starts, applies the abort of that transfer or receives a consensus snapshot, and its `Dead` replica is recovered through the normal path, which recreates the shard behind the dummy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Nuance in comment * Require a registered transfer to clear a shard for snapshot recovery The check only ran when the sender identified itself. A sender running an older version doesn't, and neither does the REST endpoint, so both cleared the shard unchecked. Hold them to *some* transfer into this shard being registered, which is what `initiate_shard_transfer` required of them before. The REST endpoint cannot select the shard transfer priority that the clear hangs off, so nothing user-facing changes. * Test that clearing for snapshot recovery needs a registered transfer * On snapshot recovery, source target shard ID from remote shard * Reject immediately if another transfer is ongoing * Add test --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
Roman Titov
|
6a03f4288b |
Implement resharding operations for ConsensusStateMachine (#10697)
|
||
|
陈志谦
|
651ca78d2d |
Fix small typos in comments across segment and storage crates (#10681)
* Fix small typos in comments across segment and storage crates - AddLearnerPeer -> AddLearnerNode (the ConfChangeType name used by the matched WAL entries) - 'adding adding' -> 'adding' in the progress_tracker debug assertion - CanellationToken -> CancellationToken in the snapshots recovery comment - 'any of of' -> 'any of' and 'valuse_set' -> 'value_set' in json_path comments - 'is not not changed' -> 'is not changed' in the mutable null index * Style: collapse the debug_assert to one line per rustfmt Nightly rustfmt prefers the single-line form now that the message fits (follow-up to the typo fix). |
||
|
Roman Titov
|
de42b2c7b9 |
Implement CreateShardKey/RemoveShardKey for consensus state machine (#10666)
|
||
|
Roman Titov
|
f5e75477a0 |
Cleanup ConsensusStateMachine validation and docs (#10658)
|
||
|
Andrey VasnetsovandClaude Fable 5.1
|
1d0c2c1bb2 |
test: wait for consensus catch-up before snapshot recovery on a new peer (#10642)
test_recover_from_snapshot_2 and test_upload_snapshot_2 start snapshot recovery on a freshly joined peer as soon as it lists the collection. The collection appears once the creation entry is applied, while the peer is still replaying the rest of the raft log, including the removal of the killed peer. Recovery then decides which other replicas to remove or mark dead from that stale local view and drops a healthy replica, leaving a shard with a single replica. Add a helper that waits until all peers share the same commit index and have no pending operations, and use it in both tests before recovering. Also fix a misleading comment in the recovery replica cleanup branch. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
|
5e32ea89cb |
Reject snapshot upload without collection config, without exposing the temp path (#10556)
* Reject snapshot upload without collection config before loading it The raw IO error from `CollectionConfigInternal::load` embedded the server-side temporary path in the API response. Check for the file first and return a fixed bad-input error instead. Part of #10553 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A3o9eWSZNMa6WAs5F2HMZC * Fix missing-config check to run before restore_snapshot loads config The path-leak guard lived after Collection::restore_snapshot, but that function already calls CollectionConfigInternal::load and surfaced the temp path as a 500. Require a regular config.json file before loading, and cover a directory-shaped config entry in the openapi test. * Use a valid empty TAR in the missing-config snapshot upload test Avoid depending on malformed-archive handling; exercise the missing collection-config path with a real TAR that has no entries. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: qdrant-cloud-bot <111755117+qdrant-cloud-bot@users.noreply.github.com> |
||
|
Andrey VasnetsovandClaude Fable 5.1
|
81bb80a5d3 |
Expose id tracker memory placement in collection config (#10597)
Add `id_tracker: { memory: cold | pinned }` to CollectionParams,
CollectionParamsDiff and CreateCollection (REST + gRPC `IdTrackerParams`),
mirroring `payload: { memory }`. `cold` builds the disk-resident id tracker,
`pinned` the in-RAM immutable one. Unset keeps the current behavior: the
`serverless_compatible` feature flag decides.
The requested placement is persisted as an optional `id_tracker_memory` on
SegmentConfig (skipped when unset, so existing configs are unchanged); the
segment builder resolves it through `SegmentConfig::id_tracker_memory_placement`
instead of reading the feature flag directly.
The config mismatch optimizer rebuilds non-appendable segments whose effective
placement differs from the requested one. Appendable segments are skipped: they
always use the mutable tracker and get the current config when indexed.
`cached` is rejected by validation: the disk mapping reader has no
populate-on-open path.
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
|
||
|
|
93e91e1da4 |
Do not claim an unfinished operation when flushing (#10577)
* fix: do not claim an unfinished operation when flushing A flush pass can capture a segment between the separately locked steps of one update operation. Persisting it under that operation's version marks the segment clean while the rest is still in memory, so every later pass skips it and the WAL acknowledge moves past the operation. Clamp what a flush claims to the last fully applied operation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix rustfmt in alias_mapping test after merging dev --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: qdrant-cloud-bot <111755117+qdrant-cloud-bot@users.noreply.github.com> |
||
|
cui fliter
|
595a87d884 |
Fix alias persistence state divergence on save failure (#10506)
Signed-off-by: cuishuang <imcusg@gmail.com> |
||
|
Roman TitovandClaude Opus 5
|
9224014e93 |
Optimizations and improvements for ConsensusStateMachine (#10533)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tim Visée
|
7a3f3e98fc |
Document cancel safety for shard initialization (#10462)
* Document cancel safety for shard initialization * Make init_empty_local_shard cancel safe, leave dummy placeholder |
||
|
Roman TitovandClaude Opus 5
|
8185a2692e |
Validate consensus against ConsensusStateMachine (#10469)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Roman TitovandClaude Opus 5
|
5476473bec |
Validate UpdateCollection operation *before* applying (#10436)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Roman TitovandClaude Opus 5
|
0e11b6ee8b |
Apply ChangeAliases operation atomically (#10425)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Tim Visée
|
9687da6c41 |
Use more direct calls (#10347)
* Direct call for shard transfer method and keys * Reuse cardinality estimate in sparse plain search * Avoid recounting available points in segment size info * Avoid cloning segment config when updating quantization * Avoid cloning search request for load profile * Direct call for counting read-only segments * Avoid re-reading point range for values count * Direct call to check replica states when initializing collection * Direct call to look up transfer on restart * Direct call for shard replicas after snapshot recovery * Direct call for local replica states in health check * Direct call for payload index schema keys when applying state * Direct calls for sharding method and key mapping when creating shard key * Direct call to check if peer has shards * Direct call for sharding method and keys when dropping shard key * Avoid cloning collection params for group by ordering * Avoid cloning collection params in local shard search * Direct call for peer address when sending Raft messages * Direct call for peer address in who_is * Avoid cloning remote query batch request * Avoid cloning operation in queue proxy update * Avoid cloning gRPC search groups request * Fetch cluster status once in cluster telemetry * Direct call to validate transfer exists on finish * Direct call for sharding method when dropping shard key * Avoid cloning peer address map when listing peers * Avoid cloning peer address map when adding peer to known * Avoid cloning shard key mapping when routing writes with fallback * Avoid cloning shard key mapping when checking resharding start * Avoid cloning gRPC recommend groups request * Avoid cloning operation when retaining forwarded point IDs * Direct call for counting collections in telemetry * Direct call to validate transfer exists on recovery * Direct call for shard IDs by shard key * Direct call for shard keys * Direct call to check if peer has shards in consensus * Direct call for replica state on transfer recovery * Direct call to check for active replicas when routing writes with fallback * Direct call to validate transfer exists on abort |
||
|
tellet-q
|
fdfe5b8bd5 |
Revert "feat(consensus): warn when applying a single entry stalls the consensus thread (#10215)" (#10241)
This reverts commit
|
||
|
|
e0110f3fe8 |
feat: optional dial9 Tokio telemetry behind a dial9 feature (#10442)
* Add optional dial9 Tokio telemetry behind a `dial9` feature Integrate dial9 so storage runtimes can emit production-friendly Tokio traces. Recording is off unless the crate is built with `--features dial9` and DIAL9_ENABLED=true is set at runtime; with the feature off, runtime construction is byte-for-byte unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7X6MkjY3P7wpP2MfHdTDY * Enable dial9 CPU and schedule profiling Turn on cpu-profiling and sched events behind the same `dial9` feature, add the DIAL9_CPU_* / DIAL9_SCHEDULE_* env knobs, and document the frame pointer rustflags the stack unwinder needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7X6MkjY3P7wpP2MfHdTDY * Harden dial9 env parsing and the writer-failure path - Reset Cargo.lock to the branch point and re-resolve, so the diff is additive instead of re-resolving unrelated packages. This drops the heck 0.5.0 -> 0.4.1 downgrade, which sat in the default build graph and would have changed proto codegen identifier casing. The remaining non-additive entry, toml_parser 1.0.9 -> 1.1.3, is forced by proc-macro-crate via dial9-trace-format-derive. - Parse DIAL9_* booleans the way dial9 does, accepting 1/y/yes/on and 0/n/no/off and warning on anything else. `str::parse::<bool>` took only exact lowercase true/false, so DIAL9_CPU_PROFILE_ENABLED=0 silently left 99 Hz sampling on and DIAL9_ENABLED=1 silently left recording off. - Require the numeric knobs to be positive. A zero disk budget made dial9 evict everything and stop recording within seconds while the log still reported telemetry enabled. - Treat a set-but-empty DIAL9_TRACE_DIR as unset. It skipped the /tmp fallback and wrote up to the full budget into the working directory, which is /qdrant next to storage/ in the official image. - Return a disabled guard as soon as the trace writer fails, before with_cpu_profiling and with_sched_events run. Those start their profilers eagerly, opening a perf event per thread and installing a process-global signal handler that build() would then discard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7X6MkjY3P7wpP2MfHdTDY * Correct the dial9 docs and give them their own section - `--cfg tokio_unstable` is required for any task data at all, not merely for fuller coverage: dial9's poll, spawn and terminate hooks are all `#[cfg(tokio_unstable)]`, and nothing in the repo sets the flag. Without it there is no task timeline and DIAL9_TASK_TRACKING_ENABLED does nothing. - Document `-C debuginfo=2`. `[profile.perf]` inherits `release` and sets no `debug` key, so the documented build symbolized off the ELF symtab with inlined callees collapsed and no file or line, unlike `[profile.bench]` which sets `debug = true` for this reason. - Move the dial9 material out from between the feature list and the prose that belongs to it. Those paragraphs describe `tracing` instrumentation and read as dial9's when the example is wedged in front of them, which points readers at `#[tracing::instrument]` for a tool that records Tokio runtime events and no tracing spans. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y7X6MkjY3P7wpP2MfHdTDY * Use cfg_select! --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: timvisee <tim@visee.me> |
||
|
Roman TitovandClaude Opus 5
|
f83f336d36 |
Implement UpdateCollection for consensus state machine (#10403)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Roman TitovandClaude Opus 5
|
bc6102bcd1 |
Implement CreateCollection and DeleteCollection for consensus state machine (#10397)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Roman Titov
|
4e58e27568 |
Implement more operations on ConsensusStateMachine [2/2] (#10339)
* fixup! Implement `ChangeAliases` operation * fixup! Add `ChangeAliases` to replay-safety tests * fixup! Add `ChangeAliases` tests * De-slop ⛷️ * De-slop 🏂 * Add `TestSlowDown` and `TestTransientError` actions These would have to be implemented on `TableOfContent` when switching to `ConsensusStateMachine` as main consensus impl * Handle more stupid corner-cases for `ChangeAliases` prop tests |
||
|
Roman TitovandClaude Opus 5
|
fda22ff9a4 |
Implement more operations on ConsensusStateMachine [1/2] (#10309)
* Add `AliasMapping::remove` and `AliasMapping::rename` methods Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Implement `ChangeAliases` operation Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `ChangeAliases` to replay-safety tests Multi-action operations that rename an alias are skipped in the convergence property: the current implementation does not replay them convergently, and the machine reproduces that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `ChangeAliases` tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add test-only `PeerMetadata::new` constructor Test state needs peers at a version other than this build. The `version` field is crate-private and `current()` is the only constructor, so gate the new one on the `testing` feature and enable it for the `storage` test build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Implement `UpdatePeerMetadata` operation Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `UpdatePeerMetadata` to replay-safety tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `UpdatePeerMetadata` tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Implement `UpdateClusterMetadata` operation Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `UpdateClusterMetadata` to replay-safety tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `UpdateClusterMetadata` tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Implement `SetQuotaConfig` operation Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `SetQuotaConfig` to replay-safety tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `SetQuotaConfig` tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Implement `TestSlowDown` and `TestTransientError` operations Both are node-local: one sleeps, the other fails at random. They plan no actions, like `Nop`, so the replay-safety properties have nothing to add. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add `TestSlowDown` and `TestTransientError` tests Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fixup! Add `UpdatePeerMetadata` to replay-safety tests * fixup! Add `UpdateClusterMetadata` tests --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7e7f19fc41 |
Keep consensus operation awaiters alive for concurrent waiters (#10338)
* Keep consensus operation awaiters alive for concurrent waiters Callers proposing an identical consensus operation deduplicate onto one broadcast channel, and the map holds its only sender. Removing the entry on timeout therefore closed the channel for every other waiter, failing their still in-flight operation with "Channel sender dropped". Only remove the entry once no receiver is left, dropping our own receiver first so the last caller out cleans up. Apply the same to await_for_multiple_operations, which registered awaiters but never deregistered them when it timed out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add debug assert to ensure we clean up consensus operation waiters * Deregister consensus operation awaiters when the waiter is dropped Dispatcher::submit_collection_meta_op registers the expected operations before proposing, then drops that future unpolled whenever the proposal itself fails. The awaiters stayed in the map with no receiver left, so the next identical request deduplicated onto a dead entry and never heard back. This is what tripped the new debug assert in CI: a rejected create-collection left a SetShardReplicaState awaiter behind, and the next run of the same test hit it. Move registration into an OperationAwaiters guard that deregisters on drop, so timeout, drop-before-poll and request cancellation are all covered by one path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Close the race the awaiter debug assert trips on The assert is sound only if no one can observe an entry whose receivers are all gone. Both cleanup sites dropped their receiver before taking the map lock, so a concurrent register could see exactly that and panic. Drop the receiver while holding the lock instead, and take that lock once per batch rather than once per operation: creating a collection registers an awaiter per replica, on the mutex the consensus thread needs for every entry it applies. Collect the awaiters into the guard as we go, so giving up part way still deregisters the ones already registered, and only build the broadcast channel when the operation is not already in-flight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: timvisee <tim@visee.me> |
||
|
Roman TitovandClaude Opus 5
|
2067eb3398 |
Implement simple consensus operations on ConsensusStateMachine (#10280)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Roman Titov
|
0b2b4a4e72 |
Implement ConsensusStateMachine prototype (#10220)
* Add `ConsensusStateMachine` type
* Add `AliasMapping::get` method
* Add `ConsensusStateMachine::apply` method stub
* Add `AliasMapping::iter` and `AliasMapping::insert` methods
* Make `vector_name_schema::add_vector_to_config` function public
* Add generic replay-safety property tests
* Add `Nop` test
* Fix clippy
Clippy is stupid af 🙄
|
||
|
Anton Karpov
|
b88becd3b5 |
docs: parameter names in doc comments that the signatures do not have (#10290)
11 names across 7 files. Renames that did not reach the comment above them (further_searches for further_results, query_context for segment_query_context, block_ranges for local_block_ranges, op for operation twice, request for requests, max_threads for max_kmeans_threads), and 3 arguments that were removed from a signature and left documented (is_on_disk, collection_params, search_runtime_handle with timeout). Documentation only, no behaviour change. |
||
|
Arnaud GourlayandClaude Opus 5
|
896deaeeef |
Fix clippy warnings from Rust 1.98 beta (#10265)
* Fix clippy warnings from Rust 1.98 beta
* drop a redundant trait import in an io_bridge test module, it already
arrives through `use super::*`
* rewrite two `chunks_exact(CONST)` sites as `as_chunks::<{ CONST }>()`
for the new `chunks_exact_to_as_chunks` lint
* return `bool` from `wait_for_consensus_commit` instead of
`Result<(), ()>`, which `result_unit_err` now flags on `async fn`. Its
only caller did `.is_ok()` on it
* allow `result_large_err` on `QueueProxyShard::new_from_version`, which
hands the `LocalShard` back to the caller on failure. Mirrors the allow
already on `ForwardProxyShard::new`
* migrate three `Atomic::fetch_update` calls to `try_update`, the name it
is renamed to in 1.99. The new name already exists at our 1.97 MSRV
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* keep guarantee on caller
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
tellet-q
|
dfca67f5ed |
feat(consensus): warn when applying a single entry stalls the consensus thread (#10215)
* feat(consensus): warn when applying a single entry stalls the consensus thread longer than a threshold |
||
|
Ivan PleshkovandClaude Opus 5
|
86b9330628 |
transfer: send raw payloads, behind feature flags (#10066)
A raw point can carry its payload as the byte blob it is stored as, mirroring `PointStructRaw.raw_payload` on the internal gRPC API. The blob travels from the sending node into the receiving node's WAL untouched, so the sender never parses the payload it read and neither node builds a protobuf value tree for it. It is parsed exactly once, where the operation is unpacked for apply (`process_point_operation`), because that is the first place the parsed form is actually needed: `set_full_payload` goes through the payload index, which cannot be updated from bytes. The gRPC boundary therefore only checks the encoding tag and rejects a point that sets both payload fields, the way the enclosing request already rejects both `points` and `raw_points`. Moving the parse onto the apply path makes its error classification load-bearing, so a malformed blob is reported as `OperationError::MalformedPayloadBlob` — the payload sibling of `MalformedVectorBlob`, mapped to `CollectionError::BadInput` for the same reason: a bad blob that reached the WAL has to be skipped on replay instead of crash-looping recovery. Three consequences of the blob living that long are handled explicitly rather than by convention: - `decode_payload_raw` takes the blob only once it has parsed, so a failure leaves the point holding it instead of holding neither representation. - `upsert_points_raw` and `sync_points_raw` refuse a point that still carries a blob. They read the parsed payload, so such a point would otherwise be stored with no payload at all, and a `debug_assert!` would not catch it in release. - `is_equal_to` compares blob to stored blob as bytes. A differing encoding costs a redundant upsert on sync, never a skipped one. The `raw_payload_transfer` bench measures the trade, per 100-point batch (one transfer batch) at payloads of ~200 B / ~700 B / ~7 KB: - Sender, storage bytes to wire: 16x / 37x / 113x faster. This is where the whole win is — no parse of the blob that was read, no value tree built. - WAL encode: 5x / 11x / 25x faster, writing a byte string instead of a map. - Receiver, wire to applicable point: 1.09x / 1.10x / 1.06x. Near neutral, as it swaps walking a prost value tree for a JSON parse. - Wire bytes: ~6% smaller. WAL bytes: 10-32% *larger*, because the blob is JSON while a parsed payload is written as a compact CBOR map. The WAL growth is accepted rather than fixed: decoding earlier to win those bytes back costs a second full deserialization, and would leave the receiving side with a `payload_raw` that is never populated. Making the blob itself compact belongs in the payload storage encoding (`RawPayloadEncoding` is the extension point for it), not here. Two flags, both off by default and both sender-only (nodes accept raw points and raw payloads regardless), read where the transfer batch is prepared: - `transfer_raw_points` transfers every collection as raw points, not only those whose vector storage would drift in a decode-encode round-trip. - `transfer_raw_payloads` ships the blob a raw read hands out; without it the prepared batch decodes it back into the parsed payload, and the wire message is exactly what it is today. Neither is enabled by `all`: a node only accepts them once it runs a version that understands them, so they can only be switched on a release later. Nothing enforces that yet — the transfer has no peer-version gate. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Yash Singh
|
895c30b17b |
fix(storage): cap tracing id in bytes, not chars, in extract_tracing_id (#9957)
extract_tracing_id guarded on value.len() (bytes) but truncated with chars().take(MAX_TRACING_ID_LEN) (chars), so a multibyte header could exceed the documented byte cap by up to 4x. Truncate on a UTF-8 char boundary at or below MAX_TRACING_ID_LEN bytes. Adds regression tests. |
||
|
|
0805d3f422 |
Add /profiler/consensus_lag to measure apply lag between peers (#10090)
* Add /profiler/consensus_lag to measure apply lag between peers Raft commit index advances on a peer whose apply loop is stalled, so the existing signals - `raft_info.commit` and the `all_nodes_have_same_commit` test helper - report a stuck peer as healthy. Nothing exposes how long a peer has been behind at *applying* entries, which is what shard transfer's `await_consensus_sync` barrier actually waits on. Each peer now keeps a ring of the last 32 entries it applied, stamped with its own wall clock and the time that entry took to apply. The ring is in memory on ConsensusManager, not in Persistent, so the on-disk format is untouched. `/profiler/consensus_lag` collects those rings from every peer over a new internal RPC and lines them up on the entry indices they share. Each entry is measured from whichever peer applied it first, so a lag is never negative; the peer that is first can differ per entry, so the baseline is per entry rather than a single chosen peer. Entries only one peer still remembers are excluded, otherwise a peer would be measured against itself. A peer stalled part-way through an entry keeps healthy lag statistics - everything it did apply, it applied on time - so the report carries `behind_entries` and `newest_applied_age_ms` alongside, which is what actually exposes the stall. Peers that fail or time out are listed rather than failing the request: a partial answer is more useful than none when the point is to find a peer that stopped answering. The endpoint follows `/profiler/slow_requests`: manage access, and outside OpenAPI, so no endpoint-count or ACTION_ACCESS guard applies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move applied-entry log into its own module Keeps the new code out of files that are already large. The ring, its entry type and the snapshot served over RPC move to `content_manager/consensus/applied_log.rs`, alongside the other consensus internals; `ConsensusManager` is left with a field, an accessor and the one `record` call in the apply loop. The grpc encoding moves next to the decoding it mirrors, in `common/consensus_lag.rs`, leaving the internal service handler three lines instead of thirty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Test that a consensus stall is still in the report after the peer catches up * Take each peer's applied index from consensus state, not its ring --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: tellet-q <elena.dubrovina@qdrant.com> |
||
|
Roman TitovandClaude Opus 4.8
|
2c40d5f2f1 |
Fix Transfer::Restart (#9786)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Roman TitovandClaude Opus 4.8
|
347fbcaf5b |
Fix SetRepicaState(Dead)/Transfer::Abort/Resharding::Abort` (#9760)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Opus 5
|
aa6c5d8403 |
Global quota API (#10035)
* feat: global quota API Memory and disk are node-wide resources, so configuring their thresholds per collection through strict mode makes little sense. Move them behind a single cluster-wide `QuotaManager`. The quota config is seeded from `storage.quotas` in the settings (and so from env vars), overridden by `quota.json` in the storage directory, and updated cluster-wide through a new `SetQuotaConfig` consensus operation which rewrites that file on every peer. Raft snapshots carry it too, so a peer that joins by snapshot picks it up. Quotas are enforced wherever the strict mode memory and disk checks used to run, but no longer gated behind `strict_mode.enabled`: a value set in an enabled strict mode config still wins per resource, the quota is the default. Rejections name both the condition that tripped and the config that governs it. `GET /quotas` reports the config plus current utilization to global read users; `PUT /quotas` replaces it for global manage users. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: cover the quota endpoints in the API consistency checks `test_all_rest_endpoints_are_covered` and the OpenAPI endpoint count both break on any new REST endpoint. Add `GET`/`PUT /quotas` to `ACTION_ACCESS` with their JWT access tests, and bump the expected API count. The quota endpoints stay out of `REST_ENDPOINT_WHITELIST`: that list is for data-plane endpoints reported per-endpoint in metrics. Also add a Raft snapshot CBOR compatibility test — snapshots are exchanged between peers of different versions during a rolling upgrade, so `quota_config` must be absent-tolerant in both directions. Review feedback: persist through `SaveOnDisk`, which already implements the write-before-swap protocol this was doing by hand; validate the config at both persistence boundaries, since a hand-edited quota file or a config arriving through consensus does not pass the REST handler's validation, and a `0%` limit would reject every update forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: assert seeding a quota from invalid settings persists nothing Follow-up to review feedback claiming `SaveOnDisk::load_or_init` writes the init value before it is validated. It does not — only `SaveOnDisk::new` persists — but the property matters: were seeding to persist first, invalid settings would leave a `quota.json` that fails validation on every subsequent start, and the node could only be recovered by deleting it by hand. Pin it down with a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: make QuotaManager the single reader of memory and disk The quota checks measured memory and disk themselves, while the optimizer and the WAL disk watcher each called `fs4::available_space` behind their own ad-hoc caches. Fold all of it into QuotaManager: it owns the readings, the freshness policy, and the limits they are compared against. Moves the module to `lib/shard`, since the optimizer sits below `storage` and has to reach it; `storage::quota` re-exports it, so consensus, the `/quotas` API and StorageConfig are unchanged. The manager is installed as a process singleton by TableOfContent, ahead of loading any collection. - Callers hand in QuotaLimits overrides instead of a StrictModeConfig, and an override can now only tighten. A collection-level admin could raise `max_disk_usage_percent` past a cluster-wide limit that needed global manage rights to set; ties resolve to the quota so the rejection names the knob that actually has to change. - Measurements are cached for 5s, but a reading at or above its limit is never reused: a rejected client retries, and freeing the resource has to take effect on the next request rather than a TTL later. - `fits_on_disk` sizes an optimization against physical free space only, never the configured limits. Optimizations are what free a full disk, so the quota must not be what stops one. - `percent_of` widens to u128 instead of saturating the multiply, which under-reported utilization (failing open) above ~184 PB. - StorageConfig::quotas is optional; absent means no quota is enforced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: don't recover dead replicas onto a node at a resource limit Recovering a dead replica pulls a whole copy of its shard onto this node. If it is already at its memory or disk quota that transfer cannot finish, and starting it only pushes the node further past the limit. Skip it and reconsider on a later sync, once the resource frees up. Adds QuotaManager::check_capacity for work that lands bytes here without being an update. Unlike fits_on_disk the configured limits do apply: taking on a replica is not what frees a full node, so there is no deadlock to avoid by letting it through. The check is hoisted out of the per-shard loop because a node over its limit re-measures on every call, so checking per dead shard would cost a statvfs each. It is free when no quota is configured. Also trims the comments across the quota module, which had grown well past what the code needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: drop trivial and duplicated quota tests Six tests removed, ~140 lines, with no loss of coverage: - a_rejection_names_the_knob_that_has_to_change asserted that a format! contains its own literals; the message is covered end-to-end by the override test and by test_global_quota.py. - a_node_over_its_quota_has_no_capacity_to_take_on_a_replica was 30 lines for check_capacity, a one-line delegation to check_update the test above it already calls. - a_rejecting_measurement_is_never_served_from_the_cache duplicated the meter test, which proves the same rule with an injected reader instead of inferring it from the real filesystem. - free_space_is_reported_without_enforcing_anything covered a one-line accessor, and its point is what the fits_on_disk test is for. - The two resolve tests and the three meter tests each collapse into one. DiskFit::Unknown keeps its coverage as two lines inside the fits_on_disk test rather than its own fixture. The snapshot compat pair becomes one test: the second only asserted cluster_metadata.is_empty(), which says nothing about quotas — the real check was the deserialize, now an expect that states it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: re-measure free space as the disk fills, and drop a Windows-only assert Two CI failures, both from this branch. e2e test_low_disk: the DiskUsageWatcher I replaced escalated to checking on every call once free space fell below 512 MB. Folding it into the quota manager lost that — available_bytes passed no limit, so a reading was reused for the full 5s however little space was left. On a disk filling as fast as that test fills it, 5s blind is enough to actually run out and the WAL write dies instead of returning "No space left on device". available_bytes now takes a watch_below level and never reuses a reading under it, which is what the old ladder was expressing. The watcher passes max(min_free, 512 MB), so the escalation point is back; above it the 5s cache still costs fewer syscalls than the old 128-call ladder. fits_on_disk gets the same rule by passing required_bytes, so a merge that does not fit re-checks rather than sitting on a stale sample. Windows: fits_on_disk on a missing path was asserted to be Unknown, but GetDiskFreeSpaceEx resolves up to the containing drive and succeeds — as common::disk_usage's own test documents. Dropped; the branch is a two-line else and is not portably reachable. Also renames an_optimization_is_sized_against_the_disk_not_the_quota, which needed explaining to be understood. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: report the quota config in telemetry Reads it from the quota manager rather than the settings, so it is the config the node is actually enforcing: a peer that missed a consensus update reports what it is applying, not what the cluster agreed on. Gated on global access, the same access `GET /quotas` requires, and left out of `PeerTelemetry` — a quota is per-node state, so each peer reports its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: regenerate OpenAPI, and cover the quota in the telemetry key sets Two CI failures from the previous commit. Referencing QuotaConfig from TelemetryData moves its definition earlier in `components/schemas`, because TelemetryData is generated ahead of QuotaStatus. Regenerated rather than hand-patched, so the schema is a pure move. test_telemetry_detail asserts the exact set of top-level telemetry keys. The quota is reported at every level, including 0 — it is three scalars, it is the default the endpoint serves, and it is what explains an update being rejected — so both key sets gain it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: remove `max_disk_usage_percent` from strict mode Disk is a node-wide resource, so a per-collection percentage of it never meant anything a caller could act on: the limit describes how full the *node* is, and which collection the write happens to target has nothing to do with it. The global quota is where it belongs. It shipped in 1.18.2 without documentation, so this drops it outright rather than deprecating. Removal is soft in every direction: StrictModeConfig has no `deny_unknown_fields`, so a client still sending it gets it ignored rather than a 400, and the same struct deserializes the persisted collection config, so collections created on 1.18.2+ keep loading. Proto field 22 is reserved so the number is never reused. The e2e test becomes a quota test — the fixture and the timing are the interesting parts and they carry over unchanged; only how the threshold is configured differs. `max_resident_memory_percent` was documented and stays for now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: enforce the strict mode memory limit outside the quota `max_resident_memory_percent` was folded into the quota as an override, which meant the quota check had to know about strict mode, and retiring the setting would mean unpicking `EffectiveLimit` and `LimitSource` from the resolution logic. It is now a check of its own in `verification/mod.rs`, next to the strict mode checks it belongs with, borrowing only the measurement from the quota manager — which stays the node's single reader of process memory, so both checks still share one reading. Deleting the setting later is deleting one function and its one caller. `QuotaManager::check_update` takes no arguments and consults the quota alone. A collection can still only tighten the limit for itself, because its own check runs in addition rather than in place of the quota's, and each rejection now names the config that has to change without having to carry a `LimitSource` to say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: enforce the quota on the update path, not in strict mode The quota check sat inside `check_strict_mode_toc_batch` only because that was the one place holding the collection's strict mode config. It doesn't need one any more, and the placement had a real cost: coverage depended on each handler remembering to ask for a strict mode check, and four of the internal update RPCs do — `sync_internal`, which moves the most bytes onto a node, does not. It now runs in `Collection::update_from_client` and `update_from_peer`, which every update passes through. `update_from_client` checks ahead of the shard split, so an operation is accepted or refused whole rather than landing on some shards and being refused by others. Classification moves with it, from ~10 `consumes_memory` impls on request DTOs to one exhaustive `CollectionUpdateOperations::consumes_quota`. The internal enum has variants — raw upserts, conditional upserts, the syncs — that have no client-facing request type, so per-DTO impls structurally could not classify them. Shard-transfer syncs stay excluded, as they are today: a transfer is sized up once before it starts, and refusing its batches partway abandons work that is nearly done only for it to restart from the beginning. Index and named-vector creation reach shards through consensus, past this check — a peer must not refuse what the cluster agreed to — so they keep their pre-consensus check, now against the quota manager directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: deprecate `max_resident_memory_percent` in strict mode Same reason the disk threshold went: memory is node-wide, so a per-collection percentage of it caps how full the *node* is, which has nothing to do with which collection is being written to. The node-wide quota caps it once for everything. Unlike the disk threshold this one shipped documented, in 1.18.0, so it keeps working — as a limit a collection can tighten for itself, never lift — and gets the usual markers: `#[deprecated]` on both Rust structs, `[deprecated = true]` on proto field 21, and `deprecated: true` in the OpenAPI schema, which schemars derives from the attribute. The note names 1.21 as the removal. Recording a version matters here: the audit in docs/plans/overdue-deprecations.md found that this repo has never written a removal deadline down, and members of the 1.15.0 deprecation batch are still in tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: reconcile the quota readers with #9891 #9891 landed effective (cgroup) figures in telemetry while this branch was making QuotaManager the single reader of memory and disk. Two collisions, neither of which git sees. `segment::utils::mem::total_memory_bytes` is now a shared accessor with a 5s TTL, so a cgroup resize is picked up. The quota module had its own `OnceLock` copy that froze the value at startup — exactly what #9891 set out to fix — so it delegates to the shared one instead. Telemetry's new `disk_size` called `common::disk_usage::disk_usage` directly. That reader lost its TTL cache on this branch when the caching moved into the quota manager's meter, so it would have taken an uncached `statvfs` on every telemetry request, and it put a second disk reader back in the tree. It goes through `QuotaManager::disk_capacity_bytes` now, sharing the reading the quota check already takes. Verified it still reports the storage filesystem, matching `df`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: split the quota manager by what each half does `manager.rs` had grown to 450 lines holding three separate jobs: owning the config and its file, taking the readings, and comparing one against the other. - `manager/store.rs` — the `Store` enum, `QUOTA_CONFIG_FILE`, and config validation, which is now the store's own business rather than something every caller has to remember to do first. - `manager/measure.rs` — every reading, and `DiskFit`. The "nothing else calls `statvfs` or reads process RSS" claim is now checkable by looking at one file. - `manager/enforce.rs` — `check_update` / `check_capacity` and the threshold comparison. - `manager/mod.rs` — the struct, its construction, and the config accessors: what a reader needs to see first. Tests move with their subject. No behaviour change: `set_config` used to validate before delegating to the store, and now the store validates on write, which is the same order of operations from the outside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: count copy-on-write deletes toward the quota Dropping a vector or a payload key does not free anything on its own: copy-on-write rewrites the point to produce the version without that field, so storage grows first and is only reclaimed once the optimizer gets to it. Gating those as if they were reclaiming space let a full node keep taking writes that make it fuller. Deleting whole points stays exempt. That is the one operation that has to work on a node at its limit, or there is no way back under it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: say that the quota's reported usage is per node `GET /quotas` returns one cluster-wide config and one set of utilization figures, which reads as though both describe the cluster. They do not: memory and disk are node-local, so `usage` is whatever the peer that served the request is seeing, and a peer under its limit says nothing about the others. Also corrects `resident_memory_percent`, which claimed to be a share of total system memory. It is a share of the memory available to the process, which under a cgroup is the limit rather than the host's RAM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: treat a node over its quota as a failed replica, not a bad request A quota rejection described the request as invalid (400) and was classified non-transient, which is how the replica set recognises errors that every replica would produce alike. A quota is the opposite: the input is fine and the answer depends on which machine you ask. On the default `wait=false` path that combination silently dropped the write — `update.rs` only deactivates transient failures when nothing completed — leaving the replica Active and permanently missing data its co-replicas had. It is now `InsufficientStorage`, transient, HTTP 507 / gRPC `ResourceExhausted`. So a node that is out of room is handled like one that is offline: - last active replica, or every replica over quota: nothing could take the write, and the client is told the cluster is out of room. - more than one replica: the full node is deactivated through the same path a dead peer takes, and the update stands if enough replicas accepted it. `check_capacity` already keeps recovery off that node until it has room. The check also moves off `update_from_client`, which applied the coordinator's own limit to the whole operation even when it held no replica of the shards being written. Each replica set now gates its own local write and records the refusal as a failure of this peer, so a node only ever answers for itself. `ResourceExhausted` is shared with rate limiting, and the reverse conversion mapped it straight to `RateLimitExceeded` — a forwarded rejection came back as 429. Statuses now carry a marker so the two stay distinguishable across the wire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: report quota pressure per node, and across the cluster A quota is node-local, so finding out which node has hit one meant asking each of them in turn — and nothing at all showed up in monitoring. `/metrics` gains a `quota_exceeded` gauge for the local node. It is emitted only while the quota is enabled: with it off the value would be a constant 0 that says nothing about the node, and an alert built on it would go quiet rather than fire if someone disabled the quota. Telemetry's `quota` field carries the same verdict alongside the config, since that is where the metric is derived from. `GET /quotas` now answers for the whole cluster. A new `GetQuotaUsage` RPC on the internal `QdrantInternal` service returns what one peer is using, and the handler fans it out to every known peer in parallel. Peers that do not answer are left out rather than failing the request — the nodes that are out of room are exactly the ones most likely to time out, and a partial answer still names them. Outside distributed mode the field is absent rather than a map of one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: report the quota metric per resource `quota_exceeded` was one flag for the whole node, which does not say what to go and fix — disk is freed by deleting or optimizing, memory by unloading. It now carries a `resource` label: quota_exceeded{resource="memory"} 0 quota_exceeded{resource="disk"} 1 A resource with no limit gets no series at all, for the same reason the metric is absent while the quota is disabled: a series that can never reach 1 reads as healthy and would quietly carry an alert that cannot fire. `QuotaManager::exceeded` returns the per-resource verdict, with `None` for a resource this node does not cap. Telemetry reports the same breakdown, since the metric is derived from it. The peer usage RPC keeps a single flag — it sits next to both percentages, so it only has to answer "is this peer refusing writes". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: drop a no-op error conversion the linter caught `check_global_access` already returns a `StorageError`, so mapping it through `StorageError::from` converted the type to itself and tripped `clippy::useless_conversion`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: hold a tripped quota until usage clears a release margin A resource resting on its limit crosses it in both directions on the noise between two readings, and each crossing is expensive: the node refuses a write, its replica is deactivated, usage dips, recovery starts sending a whole shard copy back, and the arriving data pushes it over again. The loop sustains itself, and every lap costs a shard transfer. A limit now trips at its configured value but only clears once usage has fallen 5 percentage points below it, so the crossing has to be real. The margin is floored at 1%, since a limit smaller than the margin would otherwise be impossible to fall back under and would strand the node. The verdict is carried on the manager rather than recomputed, which makes it the thing reporting shows: expect `exceeded` to be set while the utilization next to it is already back under the limit. Rejections say so too, rather than claiming a limit that is no longer exceeded: Disk usage is at 87% of total capacity. It reached the configured limit of 90% and has to fall below 85% before this node takes writes again. Changing the config clears the verdicts. New limits are a deliberate act, and should not be held back by the margin of a limit that no longer exists. Both resources are now evaluated on every check instead of stopping at the first failure, so a verdict is never left behind reporting a reading that has since been superseded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat: make the quota release margin configurable 5 points is a guess about how noisy a deployment's usage is, which is not something one number can be right about: a node whose disk moves in gigabyte steps needs a wider margin than one that creeps, and an operator who wants the old flip-on-every-reading behaviour should be able to ask for it. `release_margin_percent` joins the rest of the quota config, so it seeds from `QDRANT__STORAGE__QUOTAS__RELEASE_MARGIN_PERCENT`, replicates through consensus, and changes with `PUT /quotas`. Defaults to 5 and is filled in when a request omits it, so it always answers with the margin actually in force rather than leaving the caller to assume one. `0` releases as soon as usage is back under the limit. `QuotaConfig` grows a hand-written `Default` for it, since deriving one would have quietly defaulted the margin to 0 and disabled the hysteresis for anyone constructing a config in code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor: leave the release margin unset by default, and hold verdicts in atomics `release_margin_percent` is `null` unless someone sets it, rather than materialising 5 into every config. A quota written today then does not pin a number a later release may want to revise, and `{"enabled": false}` still round-trips as itself. `QuotaConfig::limits` resolves it, next to `enabled`, so enforcement never sees the unset case. The verdicts move from a `Mutex<QuotaExceeded>` to one `AtomicBool` per resource. They are judged independently and nothing reads them as a pair, so the lock only added contention to the path every update takes; a verdict that races a concurrent check is re-decided by the next one from a fresh reading. That also drops the tri-state. Only "was this over its limit" has to survive between checks — whether a resource is enforced at all follows from the config and the reading, so it is derived when reporting rather than stored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: drop a comment arguing with a design that was never here Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Shruti Sharma
|
e294c03c35 |
fix: clear joint consensus fields on first peer reinit (#10059)
* fix: clear joint consensus fields on first peer reinit * drop redundant consensus tests * fix spelling error |
||
|
Andrey VasnetsovandClaude Opus 5
|
c124db6905 |
Make deprecated on_disk_payload optional in the REST schema (#10020)
* Make deprecated `on_disk_payload` optional in the API schema `CollectionParams::on_disk_payload` was the last place the deprecated flag was still a bare bool, in both the REST schema and the gRPC `CollectionParams` message. Clients generated from those schemas model it as a required bool, so removing the field in a future version would break them. Make it optional in both schemas while keeping it populated on every path that builds `CollectionParams`, so responses and the persisted collection config still carry a value and existing clients keep working until it is removed. The gRPC change is wire-compatible: proto3 `optional` only adds a synthetic oneof for presence tracking, the field number and wire type are unchanged. It does change what an absent field means, though, so decode absent as `false` when reading a remote peer's collection info -- peers that predate the change encode a plain bool, which is omitted from the wire when false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Keep gRPC `on_disk_payload` a plain bool Adding proto3 `optional` would let a new client tell "unset" from `false`, but it also changes what an absent field means. A pre-upgrade server encodes a plain bool, which is omitted from the wire when false, so a client generated from the new proto would decode `false` as "unset" -- and upgrading the client first is the order we recommend. gRPC does not need the change anyway: protobuf tolerates a missing field by design, so a gRPC client does not break at runtime when we stop sending this one. The breakage this addresses is on the REST side, where a generated model with a required non-nullable bool fails to deserialize a response that omits the field. JSON encodes `false` explicitly, so the REST schema change carries no equivalent ambiguity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0a16a62f99 |
feat: io_uring setting to control which components use the io_uring backend (#10008)
* feat: `io_uring` setting to control which components use the io_uring backend A few components have both an mmap and an io_uring variant reading the very same files: the immutable dense vector storages, the single-file TurboQuant storage, and the mmap payload storage. Until now the choice was a side effect of `async_scorer` — a vector-search knob — plus, for the payload storage, a feature flag that was parked off because io_uring is ~2x slower than mmap when the data fits the page cache (#9310, #9409). Add `storage.performance.io_uring`, optional, with two modes: - unset (default): unchanged behaviour. The vector storages keep following `async_scorer`; the payload storage stays on mmap. - `disabled`: no component uses io_uring. - `auto`: a component uses io_uring when its memory placement is `cold` (data is left on disk, so reads hit the disk and there is something to gain), its feature flag allows it, and the kernel supports io_uring. Components meant to sit in RAM keep using mmap. The decision lives in one place, `segment::common::io_uring::use_io_uring`, so the openers no longer each reach for the async-scorer global. Kernel support is now probed up front through `is_io_uring_supported()` instead of opening a file and falling back on error. `async_payload_storage` now defaults to on: it no longer decides anything by itself, it only lifts the ban, and the payload storage no longer follows `async_scorer` at all — so turning it on cannot silently move an existing `async_scorer: true` deployment onto the slower path. Which backend a component ended up on depends on the config, the placement and the kernel at once, so report it in `SegmentInfo`: `vector_data[name].io_backend` and `payload_storage_io_backend`, both `"mmap" | "io_uring"`, absent for components that have no such choice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Trim comments, drop trivial tests Two tests were only restating their own implementation: `test_mode_round_trip` round-tripped the encode/decode pair next to it, and `test_io_uring_config` checked that serde deserializes a two-variant enum. The mode matrix test stays, it is the one that pins the semantics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Flatten `IoBackend` in OpenAPI, derive `JsonSchema` for `IoUringMode` Per-variant doc comments on a plain string enum make schemars emit a `oneOf` of anonymous single-value objects instead of a flat `enum`. Move the variant descriptions into the enum doc, as `Memory` and friends already do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Update lib/segment/src/vector_storage/turbo/turbo_vector_storage.rs Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> * Update lib/segment/src/types.rs Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> * Update lib/segment/src/types.rs Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> * Update lib/segment/src/types.rs Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> * Update lib/segment/src/types.rs Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> * upd openapi schema * Update lib/common/common/src/flags.rs Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> * Require kernel io_uring support in the async-scorer fallback `use_io_uring` returned `get_async_scorer()` verbatim when the `io_uring` setting is unset, so an enabled async scorer on a kernel without io_uring opened the io_uring storage, failed, and fell back to mmap with an error log per segment. Gate that branch on `is_io_uring_supported()` too, like `Auto` already is, so the component just stays on mmap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * upd openapi schema --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Roman Titov <ffuugoo@users.noreply.github.com> |
||
|
Andrey VasnetsovandClaude Fable 5
|
94fdd0e746 |
Support memory placement in service-level storage config defaults (#9950)
* Support memory placement in service-level storage config defaults Follow-up to #9684: `storage.payload.memory` and `storage.collection.vectors.memory` set service-wide placement defaults for newly created collections, deprecating `storage.on_disk_payload` and `storage.collection.vectors.on_disk`. Defaults resolve as: request `memory` > request legacy flag > service `memory` > service legacy flag; exactly one level is filled to avoid spurious memory-vs-legacy mismatch warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Mark hnsw_index.on_disk deprecated in config.yaml, document memory option Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Fable 5
|
59742bbb26 |
docs: fix collection metadata removal description (#9907)
Setting metadata to an empty object does not clear it: the update merges key by key, so an empty object is a no-op (and over gRPC an empty map is indistinguishable from an absent one). Per-key removal via null values is the mechanism that was actually implemented and tested in #7123. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Ivan Pleshkov
|
9a20fc49e4 |
[TQDT] TQ roundtrip: raw vectors in grpc, WAL and apply internal operation (#9813)
* raw vector grpc apply are you happy fmt clean up are you happy clippy review remarks review remarks * fix after rebase |
||
|
Vedant Baldwaandtimvisee
|
842ddfae10 |
fix: validate lookup_from collection for query and recommend APIs (#9531)
* fix: validate lookup_from collection in query APIs * Move validation to the bottom of the struct implementation * fix: preserve lookup_from missing collection error --------- Co-authored-by: timvisee <tim@visee.me> |
||
|
Andrey VasnetsovandClaude Fable 5
|
2735d40ecc |
Reset first_voter and prune address book on first-peer --reinit (#9785)
* Reset first_voter and prune address book on first-peer --reinit
A peer removed from consensus and killed after `RemoveNode(self)` was
committed but before it was applied keeps the old cluster's
`first_voter` and peer addresses in `raft_state.json`. First-peer
`--reinit` reset `conf_state` to a single voter (itself) but left both
untouched (`first_voter` is in fact never reset, even when the removal
is applied).
Both values are served to peers bootstrapping onto the reinitialized
cluster. A joining peer seeds its initial `conf_state` with the
advertised `first_voter`, and Raft conf-changes are deltas on top of
that base - so a stale `first_voter` permanently corrupts the joining
peer's voter set: it ends up with {old first peer, itself}, missing the
actual leader. Its `/readyz` then treats the still-alive old peer as a
cluster member and waits for the old cluster's commit index, which its
own consensus never reaches.
This only manifests when the reinitialized leader replicates its log as
plain entries (nothing applied before the kill, so the log anchor is
index 0). If the leader sends a snapshot instead, the snapshot's full
`conf_state` heals the corrupted seed - which is why
test_reinit_removed_peer only failed sporadically on CI.
Fix first-peer `--reinit` to behave like founding a fresh cluster:
reset `first_voter` to this peer (not `None`, or `recover_first_voter`
would re-derive the old first voter from the retained Raft log) and
prune `peer_address_by_id` to this peer only.
The kill-before-apply state is now injected deterministically into
test_reinit_removed_peer, reproducing the exact CI failure against the
unfixed binary. Since `--reinit` now prunes the address book, the
stale-address injection in
test_reinit_removed_peer_readyz_ignores_old_cluster moved to a restart
without `--reinit`, so it keeps exercising the /readyz `conf_state`
membership filter from #9688.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fix test_reinit_consensus expecting stale address book after --reinit
The test waited for cluster size 2 right after starting the reinitialized
first peer, before the second peer was even started. That only passed
because first-peer --reinit used to keep the old cluster's addresses in
the address book - the stale state the previous commit removes. A
reinitialized first peer is a fresh single-member cluster; the other
peers re-join and re-register their new addresses right after.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Arnaud GourlayandClaude Opus 4.8
|
35bbf0487a |
Remove unnecessary clippy allow attributes (#9775)
Remove 8 `#[allow(clippy::...)]` attributes that no longer suppress any lint. Each was verified redundant by rewriting it to `#[expect(...)]` and confirming the workspace stays clippy-clean under the CI config (`cargo clippy --workspace --all-targets --all-features -- -D warnings`). Attribute-only deletions, no behavior change. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Arnaud Gourlay
|
f4e863321c | Remove dead code (#9719) | ||
|
Arnaud GourlayandClaude Fable 5
|
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> |
||
|
|
ab0d3ecc62 |
Add unified memory: cold|cached|pinned placement parameter for collection components (#9684)
* Add unified `memory: cold|cached|pinned` placement parameter for collection components
Introduce a single `memory` parameter that controls how each collection
component's data is held in RAM, replacing the inconsistent zoo of
`on_disk` / `always_ram` / `on_disk_payload` flags:
- `cold`: not pre-loaded from disk, cached with usage
- `cached`: pre-populated into page cache on load, evictable under pressure
- `pinned`: materialized on heap, never evicted by cache pressure
The parameter is available on dense vectors, HNSW config, all quantization
configs, the sparse index, all payload field index types, and payload
storage (as a new `payload: { memory }` sub-object on collection params).
When set, it overrides the deprecated legacy flag; when unset, behavior is
unchanged. Legacy flags are marked deprecated (Rust + proto) but keep
working; conflicts are resolved in favor of `memory` with a warning.
New capabilities enabled by the tri-state model:
- HNSW graph links can be pinned (first production caller of the existing
`GraphLinksResidency::Pinned`)
- sparse mmap index, quantized vectors and on-disk payload field indexes
gain a `cached` tier (mmap + populate on open)
`pinned` is rejected by API validation for components without a heap
variant (dense vector storage, payload storage). Low-memory mode degrades
placements at load time via `Memory::clamp_to_low_memory`, matching the
existing `prefer_disk`/`skip_populate` behavior. Effective-placement
comparison in the config-mismatch optimizer avoids spurious rebuilds when
the same placement is expressed through the new parameter.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix gpu-gated tests for the new `memory` field
CI clippy runs with --all-features, which compiles the gpu-gated tests
that were missed locally: add the `memory` field to config literals and
allow deprecated placement params, same as in the rest of the tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add OpenAPI tests for memory placement, keep sparse config downgrade-clean
- OpenAPI tests: create/update collections with `memory` on every component,
assert the parameters are echoed in collection info, assert legacy-only
collections expose no new fields, and assert `pinned` is rejected (422)
for dense vector storage and payload storage on both create and update.
- Persist only the explicitly requested `memory` parameter in
`sparse_index_config.json` instead of the legacy-resolved placement, so
configurations using only the deprecated `on_disk` flag keep byte-identical
files that older Qdrant versions load without unknown fields.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Validate collection meta ops at construction, not only in the API layer
The `memory: pinned` rejection for dense vectors and payload storage
lived in `Validate` impls on the internal request types, which only ran
through the REST actix extractor. gRPC validates just the proto message,
so a gRPC client could persist `pinned` where it is not supported and
have it silently treated as `cached`.
Run the derived validation in `CreateCollectionOperation::new` and
`UpdateCollectionOperation::new` instead: the constructors are the
common chokepoint for all API paths, before the operation is proposed
to consensus. This covers every validator on these types, not just the
`memory` checks, and keeps consensus-apply unaffected so mixed-version
clusters never reject already-committed operations.
`UpdateCollectionOperation::new` becomes fallible; `remove_replica` now
uses `new_empty` since it carries no user config. Regression tests drive
the gRPC conversion path and assert `InvalidArgument` for `pinned` on
create and update, with `cold`/`cached` accepted as a control.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
||
|
Andrey VasnetsovandClaude Opus 4.8
|
b590f929ce |
Fix reinit failing with "removed all voters" on a removed peer (#9654)
* Fix reinit failing with "removed all voters" on a removed peer When `--reinit` is run on the first peer, `conf_state` is reset to a single voter (this peer), but the Raft log is left untouched. If this peer had been removed from consensus before reinit, its log still holds a committed-but-unapplied `RemoveNode(self)` conf-change. On startup that entry is replayed on top of the freshly reset single-voter config, and Raft aborts with "removed all voters", so the node can never start. Resetting only the apply-progress queue is not enough: a fresh single-node leader re-commits and re-applies any log entries still physically present beyond `commit`, re-triggering the failure. The stale tail has to be physically dropped from the WAL. On the first-peer reinit path, discard committed-but-unapplied entries inherited from the previous cluster: truncate the WAL to the last applied index, pin `commit` to it and clear the apply-progress queue. The entry at `commit` is retained as the snapshot anchor, so the first peer can still serve snapshots to bootstrapping peers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add regression test for reinit of a removed peer Starts a 2-node cluster, gracefully removes the second peer from consensus (so it commits `RemoveNode(self)` into its own WAL), then reinitializes it as a fresh first peer. Before the fix this panicked on startup with "removed all voters"; the test asserts the peer comes back online, elects itself leader, and can still seed a fresh bootstrapping peer. Verified the test fails against the pre-fix binary with exactly: Failed to apply configuration change entry Caused by: Error in Raft consensus: removed all voters Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Andrey VasnetsovandClaude Fable 5
|
f3bb3f43f4 |
Restart consensus thread on failure instead of stopping permanently (#9662)
* Restart consensus thread on failure instead of stopping permanently If the consensus loop failed (e.g. due to a transient I/O error such as running out of disk space), the consensus thread stopped permanently and the only way to recover was a full service restart. Now the consensus thread rebuilds the Raft node from persisted state and retries with exponential backoff (1s initial, doubled up to 5 min cap, retrying indefinitely). Rebuilding from persisted state is equivalent to a process restart, so this introduces no new recovery semantics. The `reinit` logic is never repeated on restart, and initial startup remains fail-fast. The consensus message channel is created outside of `Consensus` so that internal gRPC handlers and the forward-proposals thread keep their senders across restarts, and messages buffered during the outage are drained after recovery. While in restart backoff, the node reports the existing `StoppedWithErr` cluster status with restart attempt info appended, and flips back to `Working` after a successful restart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add staging TestTransientError op and consensus restart integration test New staging-only cluster operation `test_transient_error` (mirroring `test_slow_down`): applying it fails with the given probability on the targeted peer, stopping its consensus thread with a service error. The peer re-applies the entry on every consensus thread restart, rolling the probability again, so it exercises the consensus restart loop end to end. Probability 1.0 simulates a permanently failing operation. The integration test runs a 3-peer cluster, poisons a follower through its own API (so the simulated failure is reported deterministically in the response), and asserts that the restart loop engages, the remaining peers keep serving consensus operations with a quorum, and the failed peer eventually recovers and catches up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix import order to satisfy rustfmt Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Tim Visée
|
2c241a0b64 |
Respond HTTP 405 on cluster endpoints when in standalone mode (#9431)
* Return HTTP 405 on cluster endpoints when running in standalone mode * Add test * Skip some tests if not running in distributed mode |
||
|
|
1fda60b38a |
perf: use Entry API to avoid redundant map double-lookups (#9500)
* perf: use Entry API to avoid redundant map double-lookups Replace get_mut/contains_key followed by insert with the entry API across several maps, collapsing two hash lookups into one. Limited to sites where the key is Copy or already owned and moved, so no extra key clone is added to any hot path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * More Entry API usage in mutable_geo_index --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: xzfc <xzfcpw@gmail.com> |