tests/consensus_tests/test_shard_snapshot_deadlock.py (2)

36-36: Drop unused variable bindings to silence F841 and reduce noise.

-    upsert = executor.submit(upsert_random_points, peer_url, 10)
+    executor.submit(upsert_random_points, peer_url, 10)

-    telemetry = executor.submit(requests.get, f"{peer_url}/telemetry", params = { "details_level": "1" })
+    executor.submit(requests.get, f"{peer_url}/telemetry", params={"details_level": "1"}, timeout=5)

Also applies to: 40-40

---

43-45: Slightly relax the root GET timeout to reduce flakiness on slower CI runners.
0.5s can be tight; 1s still catches deadlocks while avoiding sporadic failures.

-    resp = requests.get(f"{peer_url}/", timeout = 0.5)
+    resp = requests.get(f"{peer_url}/", timeout=1.0)

lib/collection/src/shards/replica_set/telemetry.rs (2)

1-1: Avoid importing Deref as _; prefer idiomatic as_ref()/pattern matching.
The extra import adds cognitive load without benefit here.

-use std::ops::Deref as _;
+// (no need for Deref import)

---

45-60: (Optional) Apply the same lock-release pattern to get_telemetry_data above for consistency.
Not part of this hunk, but the same risk exists there.

lib/collection/src/shards/shard.rs (1)

76-86: Telemetry.status is set only for Local shards; proxies return None.

Calls via Shard::Proxy/ForwardProxy/QueueProxy return telemetry without status, unlike the Local path. Either set status for all variants here, or set it inside each proxy’s get_telemetry_data to keep behavior consistent.

Would you like a patch to set status in proxy and queue_proxy shards’ get_telemetry_data?

lib/collection/src/shards/proxy_shard.rs (1)

148-150: Also populate telemetry.status here to avoid None when called directly.

Set status from the wrapped local shard so direct ProxyShard calls produce complete telemetry (parity with Shard::Local path).

Apply:

 pub async fn get_telemetry_data(&self, detail: TelemetryDetail) -> LocalShardTelemetry {
-    self.wrapped_shard.get_telemetry_data(detail).await
+    let mut t = self.wrapped_shard.get_telemetry_data(detail).await;
+    t.status = Some(self.wrapped_shard.local_shard_status().await.0);
+    t
 }

lib/collection/src/shards/queue_proxy_shard.rs (1)

190-195: Mirror Local path: populate telemetry.status for queue proxy too.

Avoid returning telemetry without status when using QueueProxyShard directly.

Apply:

 pub async fn get_telemetry_data(&self, detail: TelemetryDetail) -> LocalShardTelemetry {
-    self.inner_unchecked()
-        .wrapped_shard
-        .get_telemetry_data(detail)
-        .await
+    let local = &self.inner_unchecked().wrapped_shard;
+    let mut t = local.get_telemetry_data(detail).await;
+    t.status = Some(local.local_shard_status().await.0);
+    t
 }

lib/collection/src/shards/local_shard/telemetry.rs (4)

15-27: Avoid taking the read-lock and spawning when detail < Level4.

You still acquire a blocking read-lock (and spawn) even when Level < 4. Short-circuit to skip both the spawn and the lock; also handle JoinError inline to avoid the extra unwrap/log block.

Apply:

-        let segments = self.segments.clone();
-
-        let segments = tokio::task::spawn_blocking(move || {
-            let segments = segments.read();
-
-            if detail.level >= DetailsLevel::Level4 {
-                segments
-                    .iter()
-                    .map(|(_id, segment)| segment.get().read().get_telemetry_data(detail))
-                    .collect()
-            } else {
-                vec![]
-            }
-        })
-        .await;
-
-        if let Err(err) = &segments {
-            log::error!("Failed to get telemetry: {err}");
-        }
-
-        let segments = segments.unwrap_or_default();
+        let segments = if detail.level >= DetailsLevel::Level4 {
+            let segments = self.segments.clone();
+            match tokio::task::spawn_blocking(move || {
+                let segments = segments.read();
+                segments
+                    .iter()
+                    .map(|(_id, segment)| segment.get().read().get_telemetry_data(detail))
+                    .collect::<Vec<_>>()
+            })
+            .await
+            {
+                Ok(v) => v,
+                Err(err) => {
+                    log::error!("failed to get telemetry: {err}");
+                    Vec::new()
+                }
+            }
+        } else {
+            Vec::new()
+        };

Also applies to: 29-34

---

48-56: Run status and size-stats concurrently to cut tail latency.

Both are independent blocking tasks; join them.

-        let status = self.get_optimization_status().await;
-
-        let SizeStats {
-            num_vectors,
-            vectors_size_bytes,
-            payloads_size_bytes,
-            num_points,
-        } = self.get_size_stats().await;
+        let (status, size_stats) =
+            tokio::join!(self.get_optimization_status(), self.get_size_stats());
+        let SizeStats {
+            num_vectors,
+            vectors_size_bytes,
+            payloads_size_bytes,
+            num_points,
+        } = size_stats;

---

80-97: Minor: prefer Arc::clone and unify error message casing.

Tiny readability/style nits: favor Arc::clone(&self.segments) and keep error-case consistent with other logs (“failed …”).

-        let segments = self.segments.clone();
+        let segments = std::sync::Arc::clone(&self.segments);
@@
-            Err(err) => OptimizersStatus::Error(format!("failed to get optimizers status: {err}")),
+            Err(err) => OptimizersStatus::Error(format!("failed to get optimizers status: {err}")),

Add once at top (outside this hunk) if you prefer the shorter form:

use std::sync::Arc;

---

99-134: Unify error handling style (match on await result).

Mirror the pattern used above to keep logging and fallback localized.

-        let stats = tokio::task::spawn_blocking(move || {
+        let stats = tokio::task::spawn_blocking(move || {
             // ...
-        })
-        .await;
-
-        if let Err(err) = &stats {
-            log::error!("failed to get size stats: {err}");
-        }
-
-        stats.unwrap_or_default()
+        })
+        .await
+        .map_err(|err| {
+            log::error!("failed to get size stats: {err}");
+            err
+        })
+        .unwrap_or_default()

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro