lib/shard/src/update.rs (1)

568-574: Fix proxy 500 on DeleteVectors: add proxy check to prevent in-place delete.

The apply closure attempts to call delete_vector on proxy segments, causing "Delete vector is disabled for proxy segments" errors and HTTP 500 responses. This issue was flagged in a previous review and is confirmed by the failing Python client congruence test reported in the PR comments (test_simple_opt_sparse_vectors_search failing with "Delete vector is disabled for proxy segments: operation 16 on point 50").

Apply this diff to short-circuit when the segment is a proxy, forcing the move path:

             |id, write_segment| {
+                // Never try to mutate a proxy in-place; force move
+                if write_segment.is_proxy() {
+                    return Ok(false);
+                }
                 let mut res = true;
                 for name in vector_names {
                     res &= write_segment.delete_vector(op_num, id, name)?;

This ensures the patch closure (lines 575-579) handles vector removal via the move path, consistent with the non-appendable proxy design.

Based on learnings

lib/shard/src/segment_holder/mod.rs (1)

1002-1027: Do not drop the temporary CoW segment yet.

remove_segment_if_not_needed still drops the just-created appendable segment when it looks empty. Downstream code (e.g. the optimizer, see the panic reported in the coach/bfb runs) still assumes that segment remains present and hits temporary CoW segment must exist. Until every caller tolerates its absence we must keep the segment around—otherwise production workloads crash mid-optimization. Please restore the old behaviour by re‑adding the segment instead of calling drop_data().

-        } else {
-            log::trace!("Dropping temporary segment with no changes");
-            tmp_segment.drop_data()?;
-            Ok(true)
-        }
+        } else {
+            log::trace!("Keeping temporary segment with no changes to satisfy optimizer invariants");
+            self.add_existing_locked(segment_id, tmp_segment);
+            Ok(false)
+        }

lib/shard/src/proxy_segment/segment_entry.rs (1)

227-235: Restore delete‑vector support on proxies.

Turning delete_vector into a hard service_error causes every vector delete routed to a proxy to bubble up as HTTP 500 (see the failing congruence test and manual repro: “Delete vector is disabled for proxy segments: operation 16 on point 50”). During optimization requests still reach proxy segments before the swap completes, so rejecting them breaks client workflows. Please reinstate functional tombstoning/forwarding here rather than returning an error so deletes remain lossless while a shard is proxied.

lib/shard/src/proxy_segment/mod.rs (1)

306-312: Merge should preserve newest change; don’t clobber with older entries

Current merge overwrites blindly; a stale change can replace a newer one across proxies.

Based on learnings

     pub fn merge(&mut self, other: &Self) {
         for (key, change) in &other.changes {
-            self.changes.insert(key.clone(), change.clone());
+            let replace = self
+                .changes
+                .get(key)
+                .map_or(true, |existing| change.version() >= existing.version());
+            if replace {
+                self.changes.insert(key.clone(), change.clone());
+            }
         }
     }

lib/collection/src/collection_manager/optimizers/segment_optimizer.rs (1)

773-821: Add aggregator for proxied vector-level deletes

Mirror proxy_deleted_points/proxy_index_changes with a proxy_deleted_vectors aggregator so both build and finalize paths can apply these operations.

     fn proxy_index_changes(&self, proxies: &[LockedSegment]) -> ProxyIndexChanges {
         let mut index_changes = ProxyIndexChanges::default();
         for proxy_segment in proxies {
             match proxy_segment {
                 LockedSegment::Original(_) => {
                     log::error!("Reading raw segment, while proxy expected");
                     debug_assert!(false, "Reading raw segment, while proxy expected");
                 }
                 LockedSegment::Proxy(proxy) => {
                     let proxy_read = proxy.read();
                     index_changes.merge(proxy_read.get_index_changes())
                 }
             }
         }
         index_changes
     }
+
+    /// Accumulates vector-level deletes made in a given set of proxies.
+    ///
+    /// Not synchronized, but includes at least all deletes before the call.
+    fn proxy_deleted_vectors(&self, proxies: &[LockedSegment]) -> DeletedVectors {
+        let mut deleted_vectors: DeletedVectors = DeletedVectors::new();
+        for proxy_segment in proxies {
+            match proxy_segment {
+                LockedSegment::Original(_) => {
+                    log::error!("Reading raw segment, while proxy expected");
+                    debug_assert!(false, "Reading raw segment, while proxy expected");
+                }
+                LockedSegment::Proxy(proxy) => {
+                    let proxy_read = proxy.read();
+                    for ((point_id, vector_name), versions) in proxy_read.get_deleted_vectors() {
+                        let entry =
+                            deleted_vectors.entry(((*point_id), vector_name.clone())).or_insert(*versions);
+                        entry.operation_version =
+                            entry.operation_version.max(versions.operation_version);
+                        entry.local_version = entry.local_version.max(versions.local_version);
+                    }
+                }
+            }
+        }
+        deleted_vectors
+    }

Note: requires DeletedVectors/ProxyDeletedVector and ProxySegment::get_deleted_vectors (see related suggestions in proxy_segment).

lib/segment/src/data_types/named_vectors.rs (1)

269-271: Consider returning the removed value.

The method discards the Option<CowVector<'a>> returned by self.map.remove(key). Standard Rust collections like HashMap and BTreeMap return Option<V> from remove operations to indicate whether the key existed and provide the removed value. Returning it here would offer more flexibility to callers.

Apply this diff to return the removed value:

-    pub fn remove_ref(&mut self, key: &VectorName) {
-        self.map.remove(key);
+    pub fn remove_ref(&mut self, key: &VectorName) -> Option<CowVector<'a>> {
+        self.map.remove(key)
     }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: timvisee
PR: qdrant/qdrant#7157
File: lib/shard/src/segment_holder/mod.rs:808-814
Timestamp: 2025-09-01T11:42:06.964Z
Learning: In Qdrant's segment holder, panicking when no segments exist during flush_all is intentional and preferred over graceful error handling, as having zero segments could permanently corrupt the WAL by acknowledging u64::MAX. The maintainers consider this condition impossible and use the panic as a fail-fast safety mechanism to prevent data corruption.

Learnt from: generall
PR: qdrant/qdrant#6403
File: lib/collection/src/collection_manager/holders/proxy_segment.rs:1178-1197
Timestamp: 2025-04-20T19:08:45.034Z
Learning: In the ProxySegment implementation, overwriting previous index changes when inserting a DeleteIfIncompatible operation is intentional. When an incompatible index is detected, the DeleteIfIncompatible operation should take precedence over any previously scheduled operations on that index.