lib/segment/src/common/mod.rs (1)

247-248: BYTES_IN_MB constant is correct and consistent

BYTES_IN_MB matches 1024 * 1024 and fits existing usage; no behavioral issues. If you want to avoid duplicating the literal, you could optionally define it in terms of BYTES_IN_KB (e.g., BYTES_IN_KB * BYTES_IN_KB), but it’s fine as-is.

lib/collection/src/operations/snapshot_storage_ops.rs (1)

9-9: Download metrics and directory handling look good; consider small sequencing/type tweaks

The added timing/throughput logging and parent directory creation are solid and improve observability and robustness; the main behavior looks correct.

Two minor follow-ups to consider:

1. Log after integrity check instead of before it
   Right now the "Object storage snapshot download completed" debug log is emitted before the file-size consistency check. If the size mismatch branch is hit, logs will still say “completed”, which can be slightly misleading when debugging corrupted/partial downloads. Reordering so that the debug log comes after the file_meta.len() != total_size as u64 check would keep logs aligned with actual success.

2. Optional: align counter type with on-disk length
   total_size is inferred as usize and later compared to file_meta.len() via as u64. If you ever expect very large snapshots, you may want to make total_size a u64 to match file_meta.len() and avoid any theoretical usize overflow on 32‑bit targets. If you do that, you can also drop the as u64 cast in the comparison. Based on learnings, this mirrors the overflow‑resilience approach used elsewhere in the repo.

If you’d like to apply (1), an example diff for the tail of the function would look like:

-    // ensure flush
-    file.flush()
-        .await
-        .map_err(|e| CollectionError::service_error(format!("Failed to flush file: {e}")))?;
-
-    let download_duration = download_start_time.elapsed();
-    let total_size_mb = total_size as f64 / BYTES_IN_MB as f64;
-    let download_speed_mbps = total_size_mb / download_duration.as_secs_f64();
-    log::debug!(
-        "Object storage snapshot download completed: path={}, size={:.2} MB, duration={:.2}s, speed={:.2} MB/s",
-        target_path.display(),
-        total_size_mb,
-        download_duration.as_secs_f64(),
-        download_speed_mbps
-    );
-
-    // check len to file len
-    let file_meta = tokio_fs::metadata(target_path).await?;
+    // ensure flush
+    file.flush()
+        .await
+        .map_err(|e| CollectionError::service_error(format!("Failed to flush file: {e}")))?;
+
+    // check len to file len before logging success
+    let file_meta = tokio_fs::metadata(target_path).await?;
     if file_meta.len() != total_size as u64 {
         return Err(CollectionError::service_error(format!(
             "Downloaded file size does not match the expected size: {} != {}",
             file_meta.len(),
             total_size
         )));
     }
-    Ok(())
+
+    let download_duration = download_start_time.elapsed();
+    let total_size_mb = total_size as f64 / BYTES_IN_MB as f64;
+    let download_speed_mbps = if download_duration.as_secs_f64() > 0.0 {
+        total_size_mb / download_duration.as_secs_f64()
+    } else {
+        0.0
+    };
+    log::debug!(
+        "Object storage snapshot download completed: path={}, size={:.2} MB, duration={:.2}s, speed={:.2} MB/s",
+        target_path.display(),
+        total_size_mb,
+        download_duration.as_secs_f64(),
+        download_speed_mbps
+    );
+
+    Ok(())

Also applies to: 200-258

lib/storage/src/content_manager/snapshots/download.rs (1)

7-7: Remote snapshot download metrics look correct; minor naming/logging nit

The added timing and byte‑counting logic is straightforward and correctly computes MB and MB/s using BYTES_IN_MB; the overall behavior of download_file is preserved.

One small nit: the variable is called download_speed_mbps, but the log message says "speed={:.2} MB/s". To avoid confusion between megabits (Mbps) and megabytes (MB/s), consider renaming the variable (e.g., download_speed_mb_s) or adjusting the log string to match whatever unit you intend to standardize on across the codebase.

Also applies to: 33-33, 56-61, 66-75

src/actix/api/snapshot_api.rs (1)

21-21: Partial snapshot download‑from‑peer metrics and flow look good

The added timing/byte tracking inside recover_partial_snapshot_from is wired correctly:

• download_start_time scopes the measurement to the remote partial snapshot fetch.
• total_bytes_downloaded is accumulated per chunk and returned via StorageResult::Ok((collection, partial_snapshot_temp_path, total_bytes_downloaded)).
• The match on create_partial_snapshot_result cleanly preserves the EmptyPartialSnapshot fast‑path and error propagation, and metrics are only logged for real downloads.
• The debug log includes path, size in MB, duration, speed, and shard_id, which is useful for tracing.

Given that very similar MB/s logging now exists in several places, you might eventually consider a small shared helper (e.g., a utility that takes start_time, bytes, and a label/context) to avoid copy‑pasting the same conversion and format string, but that’s purely a future cleanup and not required for this PR.

Also applies to: 722-813

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: IvanPleshkov
Repo: qdrant/qdrant PR: 7043
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:86-90
Timestamp: 2025-08-15T11:42:00.297Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs, overflow protection for encoded_storage_size computation (quantized_vector_size * vectors_count) is implemented in PR #7048, using checked_mul with u64 casting to prevent silent overflow.