lib/common/memory/src/fadvise.rs (2)

22-27: Avoid the extra PathBuf allocation in clear_disk_cache.

fs_err::File::open already accepts any AsRef<Path> and clones the path internally for error context. Cloning here adds an avoidable allocation. Please pass the original &Path directly.

-    match File::open(file_path.to_path_buf()) {
+    match File::open(file_path) {

---

48-55: Skip cloning the path before opening the OneshotFile.

Similar to the above, fs_err::File::open can take the borrowed path and will capture it for diagnostics on its own. Dropping the manual to_path_buf() avoids redundant work in a hot path.

-        let file = File::open(path.as_ref().to_path_buf())?;
+        let file = File::open(path.as_ref())?;

lib/quantization/src/encoded_vectors_binary.rs (1)

460-474: Use fs_err for directory creation — optional readability tweak

Behavior is correct and now includes path context via fs_err. Consider a small readability refactor to avoid the chained and_then.

Apply within this block:

-            meta_path
-                .parent()
-                .ok_or_else(|| {
-                    std::io::Error::new(
-                        std::io::ErrorKind::InvalidInput,
-                        "Path must have a parent directory",
-                    )
-                })
-                .and_then(fs::create_dir_all)
-                .map_err(|e| {
-                    EncodingError::EncodingError(format!(
-                        "Failed to create metadata directory: {e}",
-                    ))
-                })?;
+            let dir = meta_path.parent().ok_or_else(|| {
+                EncodingError::EncodingError("Path must have a parent directory".to_string())
+            })?;
+            fs::create_dir_all(dir).map_err(|e| {
+                EncodingError::EncodingError(format!("Failed to create metadata directory: {e}"))
+            })?;

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

11-11: Remove redundant crate import of reqwest.

In Rust 2018, the extern prelude makes reqwest::... usable without use reqwest;. Drop it to avoid a no-op import.

Apply this diff:

-use {fs_err as fs, reqwest};
+use fs_err as fs;

---

37-41: Construction pipeline looks correct; minor readability tweak optional.

fs::File::from_parts(...)->tokio_fs::File::from_std(...) is the right pattern for preserving path context. Consider dropping the explicit turbofish; type inference suffices.

Apply this diff to Line 37:

-    let file = fs::File::from_parts::<&Path>(file, temp_path.as_ref());
+    let file = fs::File::from_parts(file, temp_path.as_ref());

lib/common/common/src/mmap_hashmap.rs (1)

122-127: Minor: avoid unnecessary turbofish and prefer owning the path for clarity.

File::from_parts doesn’t need the <&Path> turbofish; passing an owned PathBuf improves clarity and avoids any lifetime second-guessing.

Apply:

-        let file = File::from_parts::<&Path>(file, temp_path.as_ref());
+        let file = File::from_parts(file, temp_path.as_ref().to_path_buf());

lib/common/io/src/file_operations.rs (1)

10-13: Avoid exposing std::fs::File in the API; accept a generic Write instead.

You can drop the clippy allow and make the API more general by taking &mut dyn Write. This keeps all current call sites working and avoids disallowed type usage in the signature.

Apply:

-#[allow(
-    clippy::disallowed_types,
-    reason = "can't use `fs_err::File` since `atomicwrites` only provides `&mut std::fs::File`"
-)]
 pub fn atomic_save<E, F>(path: &Path, write: F) -> Result<(), E>
 where
     E: From<io::Error>,
-    F: FnOnce(&mut BufWriter<&mut std::fs::File>) -> Result<(), E>,
+    F: FnOnce(&mut dyn Write) -> Result<(), E>,
 {
     let af = AtomicFile::new(path, OverwriteBehavior::AllowOverwrite);
     af.write(|f| {
         let mut writer = BufWriter::new(f);
-        write(&mut writer)?;
+        let mut w: &mut dyn Write = &mut writer;
+        write(&mut w)?;
         writer.flush()?;
         Ok(())
     })

Also applies to: 17-25

lib/common/memory/src/mmap_ops.rs (3)

39-39: Rename via fs_err is fine; consider closing TOCTOU on Windows.

There’s a small race between exists() and rename() if another process creates the target; on Windows rename fails if dest appears in the meantime. Consider using tempfile::NamedTempFile and persist (or atomicwrites, like elsewhere) to make this more robust.

---

46-51: Avoid opening with append(true) and create(true) for read-only mmap.

Appending isn’t needed for read-only mapping, and creating a 0-byte file can cause mapping failures. Prefer opening read-only and ensuring the file exists/size is valid beforehand.

Apply:

-    let file = OpenOptions::new()
-        .read(true)
-        .append(true)
-        .create(true)
-        .open(path)?;
+    let file = OpenOptions::new()
+        .read(true)
+        .open(path)?;

---

80-145: Consider bytemuck/zerocopy instead of custom transmute helpers.

These helpers are brittle and easy to misuse. Libraries like bytemuck or zerocopy provide safer conversions with compile-time checks. This isn’t new code, but worth planning a migration.

As per coding guidelines

lib/segment/src/index/field_index/full_text_index/inverted_index/mmap_inverted_index/mmap_postings.rs (2)

162-167: Tempfile handling: add path context to InvalidInput and drop turbofish.

• Improve diagnostics by attaching a message to InvalidInput.
• The turbofish on File::from_parts is unnecessary; type inference suffices.

Apply this diff:

-        let (file, temp_path) = tempfile::Builder::new()
-            .prefix(path.file_name().ok_or(io::ErrorKind::InvalidInput)?)
-            .tempfile_in(path.parent().ok_or(io::ErrorKind::InvalidInput)?)?
-            .into_parts();
-        let file = File::from_parts::<&Path>(file, temp_path.as_ref());
+        let (file, temp_path) = tempfile::Builder::new()
+            .prefix(
+                path.file_name().ok_or_else(|| io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    format!("invalid path (no file name): {}", path.display()),
+                ))?
+            )
+            .tempfile_in(
+                path.parent().ok_or_else(|| io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    format!("invalid path (no parent directory): {}", path.display()),
+                ))?
+            )?
+            .into_parts();
+        let file = File::from_parts(file, temp_path.as_ref());

---

5-5: Remove unused Path import.

After dropping the turbofish, Path is no longer needed.

Apply this diff:

-use std::path::{Path, PathBuf};
+use std::path::PathBuf;

lib/segment/src/index/field_index/numeric_index/mmap_numeric_index.rs (1)

204-207: Make wipe tolerant to already-missing files; log non-NotFound dir errors.

Avoid failing the wipe if some files were removed concurrently, and don’t silently swallow unexpected remove_dir errors.

Apply this diff:

-        for file in files {
-            fs::remove_file(file)?;
-        }
-        let _ = fs::remove_dir(path);
+        for file in files {
+            if let Err(err) = fs::remove_file(&file) {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    return Err(err.into());
+                }
+            }
+        }
+        if let Err(err) = fs::remove_dir(&path) {
+            if err.kind() != std::io::ErrorKind::NotFound {
+                log::warn!("Failed to remove index directory {}: {err}", path.display());
+            }
+        }

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

247-249: Import swap is fine; avoid to_str() for paths in metadata.

While reviewing the import change, noticed later in the test you do fs::metadata(dir.path().join("open-1").to_str().unwrap()). Prefer passing a Path directly to avoid lossy UTF-8 conversion:

Example adjustment in the test body:

let metadata = fs::metadata(dir.path().join("open-1")).unwrap();

This also keeps the richer path context from fs_err intact.

lib/segment/src/segment_constructor/segment_constructor_base.rs (2)

953-959: Cleanup: ignore NotFound to reduce log noise and TOCTOU races.

When removing files after a failed migration, NotFound is benign; don’t log it as an error.

Apply:

-                if let Err(err) = fs::remove_file(&file) {
-                    log::error!(
-                        "ID tracker migration to mutable failed, failed to remove mutable file {} for cleanup: {err}",
-                        file.display(),
-                    );
-                }
+                if let Err(err) = fs::remove_file(&file) {
+                    if err.kind() != std::io::ErrorKind::NotFound {
+                        log::error!(
+                            "ID tracker migration to mutable failed, failed to remove mutable file {} for cleanup: {err}",
+                            file.display(),
+                        );
+                    }
+                }

---

1454-1461: Avoid pre-check; handle NotFound after remove_dir_all to prevent TOCTOU.

Directly attempt remove_dir_all; ignore NotFound, log others.

Apply:

-            if storage_dir.is_dir()
-                && let Err(err) = fs::remove_dir_all(&storage_dir)
-            {
-                log::error!(
-                    "Payload storage migration to mmap failed, failed to remove mmap files in {} for cleanup: {err}",
-                    storage_dir.display(),
-                );
-            }
+            if let Err(err) = fs::remove_dir_all(&storage_dir) {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    log::error!(
+                        "Payload storage migration to mmap failed, failed to remove mmap files in {} for cleanup: {err}",
+                        storage_dir.display(),
+                    );
+                }
+            }

lib/collection/src/common/file_utils.rs (1)

42-51: Create parent directories too.

create_dir will fail if parents don’t exist. Prefer create_dir_all.

Apply:

-        tokio_fs::create_dir(&to).await.map_err(|err| {
+        tokio_fs::create_dir_all(&to).await.map_err(|err| {
             CollectionError::service_error(format!(
                 "failed to move directory {} to {}: \
                  failed to create destination directory: \
                  {err}",
                 from.display(),
                 to.display(),
             ))
         })?;

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

82-99: Avoid usize cast for file sizes; use u64 and convert safely.

Prevents potential overflow on 32-bit and improves correctness for very large files.

Apply:

-    let file_meta = tokio_fs::metadata(local_source_path).await?;
-    let file_size = file_meta.len() as usize;
+    let file_meta = tokio_fs::metadata(local_source_path).await?;
+    let file_size = file_meta.len();

-    if file_size > MAX_UPLOAD_SIZE {
+    if file_size > MAX_UPLOAD_SIZE {
         return Err(CollectionError::service_error(format!(
             "File size exceeds the maximum upload size: {MAX_UPLOAD_SIZE}"
         )));
     }

-    if file_size > DEFAULT_CHUNK_SIZE * MAX_PART_NUMBER {
-        let chunk_size = ((file_size - 1) / MAX_PART_NUMBER) + 1; // ceil((file_size) / MAX_PART_NUMBER)
-        return Ok(chunk_size);
-    }
-    Ok(DEFAULT_CHUNK_SIZE)
+    if file_size > DEFAULT_CHUNK_SIZE * MAX_PART_NUMBER {
+        let chunk_size_bytes = ((file_size - 1) / MAX_PART_NUMBER) + 1; // ceil((file_size) / MAX_PART_NUMBER)
+        let chunk_size = usize::try_from(chunk_size_bytes).map_err(|_| {
+            CollectionError::service_error(
+                "Calculated chunk size exceeds addressable memory on this platform",
+            )
+        })?;
+        return Ok(chunk_size);
+    }
+    Ok(DEFAULT_CHUNK_SIZE as usize)

Also update the constants to u64:

-    const DEFAULT_CHUNK_SIZE: usize = 50 * 1024 * 1024;
-    const MAX_PART_NUMBER: usize = 10000;
-    const MAX_UPLOAD_SIZE: usize = 5 * 1024 * 1024 * 1024 * 1024;
+    const DEFAULT_CHUNK_SIZE: u64 = 50 * 1024 * 1024;
+    const MAX_PART_NUMBER: u64 = 10_000;
+    const MAX_UPLOAD_SIZE: u64 = 5 * 1024 * 1024 * 1024 * 1024;

clippy.toml (1)

16-96: Potentially invalid disallowed paths (std doesn’t define these).

Entries like:

• std::fs::File::{lock, lock_shared, try_lock, try_lock_shared, unlock} (Lines 21-23, 31-33)
• std::fs::soft_link (Line 49)

don’t exist in std. Consider removing them or marking as allow-invalid to avoid Clippy config issues.

Apply this minimal adjustment to avoid toolchain breakage:

-  { path = "std::fs::File::lock", replacement = "fs_err::File::lock" },
-  { path = "std::fs::File::lock_shared", replacement = "fs_err::File::lock_shared" },
+  { path = "std::fs::File::lock", replacement = "fs_err::File::lock", allow-invalid = true },
+  { path = "std::fs::File::lock_shared", replacement = "fs_err::File::lock_shared", allow-invalid = true },
@@
-  { path = "std::fs::File::try_lock", replacement = "fs_err::File::try_lock" },
-  { path = "std::fs::File::try_lock_shared", replacement = "fs_err::File::try_lock_shared" },
-  { path = "std::fs::File::unlock", replacement = "fs_err::File::unlock" },
+  { path = "std::fs::File::try_lock", replacement = "fs_err::File::try_lock", allow-invalid = true },
+  { path = "std::fs::File::try_lock_shared", replacement = "fs_err::File::try_lock_shared", allow-invalid = true },
+  { path = "std::fs::File::unlock", replacement = "fs_err::File::unlock", allow-invalid = true },
@@
-  { path = "std::fs::soft_link", replacement = "fs_err::soft_link" },
+  { path = "std::fs::soft_link", replacement = "fs_err::soft_link", allow-invalid = true },

Please confirm Clippy loads this config cleanly in CI. If it complains, drop these entries instead. As per coding guidelines.

lib/collection/src/shards/shard_holder/mod.rs (1)

989-991: Avoid blocking create_dir_all in async context.

Prefer tokio_fs::create_dir_all to prevent blocking the runtime thread.

Apply:

-        if !temp_dir.exists() {
-            fs::create_dir_all(temp_dir)?;
-        }
+        if !temp_dir.exists() {
+            tokio_fs::create_dir_all(temp_dir).await?;
+        }

lib/segment/src/vector_storage/dense/memmap_dense_vector_storage.rs (1)

308-310: Drop unnecessary PathBuf allocation.

OpenOptions::open accepts AsRef; cloning the path isn’t needed.

Apply:

-fn open_append<P: AsRef<Path>>(path: P) -> io::Result<File> {
-    let path = path.as_ref().to_path_buf();
-    OpenOptions::new().append(true).open(path)
-}
+fn open_append<P: AsRef<Path>>(path: P) -> io::Result<File> {
+    OpenOptions::new().append(true).open(path)
+}

lib/collection/src/shards/replica_set/snapshots.rs (1)

134-201: Avoid blocking read_dir in async function.

fs::read_dir and Path metadata checks run on the core runtime thread. Prefer tokio_fs::read_dir to prevent blocking during snapshot enumeration.

Apply this pattern:

-        for segment_entry in fs::read_dir(segments_path)? {
-            let segment_path = segment_entry?.path();
+        let mut rd = tokio_fs::read_dir(&segments_path).await?;
+        while let Some(segment_entry) = rd.next_entry().await? {
+            let segment_path = segment_entry.path();
             // ... rest of the loop unchanged ...
-        }
+        }

Optionally consider replacing other per-iteration stat calls with async equivalents if this path proves hot.

lib/segment/src/utils/fs.rs (1)

61-63: Add EXDEV fallback for cross-device moves.

fs::rename fails with CrossDeviceLink across mount points. Add a fallback: copy+remove for files; for directories, create dest_path, recursively move contents, then remove source.

Apply this diff:

-            fs::rename(&path, &dest_path)
-                .map_err(|err| failed_to_move_error(&path, &dest_path, err))?;
+            if let Err(err) = fs::rename(&path, &dest_path) {
+                if err.kind() == std::io::ErrorKind::CrossDeviceLink {
+                    if path.is_dir() {
+                        fs::create_dir_all(&dest_path).map_err(|e| {
+                            failed_to_move_error(&path, &dest_path, e)
+                        })?;
+                        move_all_impl(base, &path, &dest_path)?;
+                        fs::remove_dir_all(&path)
+                            .map_err(|e| failed_to_move_error(&path, &dest_path, e))?;
+                    } else {
+                        fs::copy(&path, &dest_path)
+                            .map_err(|e| failed_to_move_error(&path, &dest_path, e))?;
+                        fs::remove_file(&path)
+                            .map_err(|e| failed_to_move_error(&path, &dest_path, e))?;
+                    }
+                } else {
+                    return Err(failed_to_move_error(&path, &dest_path, err));
+                }
+            }

lib/collection/src/common/snapshots_manager.rs (1)

274-275: Avoid blocking fs ops on async paths: use tokio_fs::create_dir_all.

These synchronous create_dir_all calls block the async runtime. Prefer the fs_err Tokio wrappers.

Apply this diff:

-            fs::create_dir_all(target_dir)?;
+            tokio_fs::create_dir_all(target_dir).await?;

-            fs::create_dir_all(target_dir)?;
+            tokio_fs::create_dir_all(target_dir).await?;

-            fs::create_dir_all(target_dir)?;
+            tokio_fs::create_dir_all(target_dir).await?;

Also applies to: 306-307, 417-417

lib/collection/src/shards/local_shard/mod.rs (1)

165-171: Handle NotFound races when deleting shard data.

Between exists() and remove_dir_all, the directory can disappear. Treat NotFound as benign.

Apply this diff:

-        if wal_path.exists() {
-            tokio_fs::remove_dir_all(wal_path).await?;
-        }
+        if wal_path.exists() {
+            if let Err(e) = tokio_fs::remove_dir_all(&wal_path).await {
+                if e.kind() != std::io::ErrorKind::NotFound {
+                    return Err(e.into());
+                }
+            }
+        }
@@
-        if segments_path.exists() {
-            tokio_fs::remove_dir_all(segments_path).await?;
-        }
+        if segments_path.exists() {
+            if let Err(e) = tokio_fs::remove_dir_all(&segments_path).await {
+                if e.kind() != std::io::ErrorKind::NotFound {
+                    return Err(e.into());
+                }
+            }
+        }

lib/segment/tests/integration/segment_builder_test.rs (1)

70-70: Optional: factor out a tiny helper to reduce repetition

If you want to DRY the repeated directory-count pattern across tests, consider:

fn dir_count<P: AsRef<std::path::Path>>(p: P) -> usize {
    fs::read_dir(p).unwrap().count()
}

Then replace occurrences like fs::read_dir(dir.path()).unwrap().count() with dir_count(dir.path()).

Also applies to: 74-74, 88-88, 156-156, 160-160, 174-174, 289-289, 293-293, 307-307, 580-580, 597-597

lib/segment/src/index/field_index/numeric_index/mmap_numeric_index.rs (3)

137-146: Fix over-allocation: compute bytes, then align to 64 bytes

Current length uses 64 * ceil(bits/64), which allocates 8× more bytes than needed. Compute bytes = ceil(bits/8) first, then align bytes to 64.

Apply this diff:

-            const BITS_IN_BYTE: usize = 8;
-            let deleted_flags_count = in_memory_index.point_to_values.len();
-            let deleted_file = create_and_ensure_length(
-                &deleted_path,
-                BITS_IN_BYTE
-                    * BITS_IN_BYTE
-                    * deleted_flags_count.div_ceil(BITS_IN_BYTE * BITS_IN_BYTE),
-            )?;
+            const BITS_IN_BYTE: usize = 8;
+            let deleted_flags_count = in_memory_index.point_to_values.len();
+            // Compute exact bytes needed for `deleted_flags_count` bits, then align to 64 bytes.
+            let bytes_needed = deleted_flags_count.div_ceil(BITS_IN_BYTE);
+            let align_bytes = BITS_IN_BYTE * BITS_IN_BYTE; // 64 bytes
+            let file_len = align_bytes * bytes_needed.div_ceil(align_bytes);
+            let deleted_file = create_and_ensure_length(&deleted_path, file_len)?;

---

200-207: Make wipe idempotent and resilient

• Ignore NotFound when removing files (safe to call repeatedly).
• Prefer remove_dir_all at the end to clean up any empty subdirs left by subcomponents.

Apply this diff:

     pub fn wipe(self) -> OperationResult<()> {
         let files = self.files();
         let Self { path, .. } = self;
         for file in files {
-            fs::remove_file(file)?;
+            if let Err(err) = fs::remove_file(&file) {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    return Err(OperationError::from(err));
+                }
+            }
         }
-        let _ = fs::remove_dir(path);
+        // Best-effort: remove the index directory even if nested dirs remain.
+        let _ = fs::remove_dir_all(&path);
         Ok(())
     }

---

322-329: Clarify values_range_size semantics vs. deletions (optional)

end_index - start_index counts raw span, not filtered (non-deleted) items. If callers expect the iterator’s effective size, consider self.values_range_iterator(...).count() or rename the method to reflect “span.”

lib/segment/src/id_tracker/mutable_id_tracker.rs (4)

134-146: Include path in load error messages for consistency with fs-err.

Top-level error mapping for load paths omits the file path; add it for parity with store paths.

-        let mappings = if has_mappings {
-            load_mappings(&mappings_path).map_err(|err| {
-                OperationError::service_error(format!("Failed to load ID tracker mappings: {err}"))
-            })?
+        let mappings = if has_mappings {
+            load_mappings(&mappings_path).map_err(|err| {
+                OperationError::service_error(format!(
+                    "Failed to load ID tracker mappings ({}): {err}",
+                    mappings_path.display(),
+                ))
+            })?
         } else {
             PointMappings::default()
         };
@@
-        let internal_to_version = if has_versions {
-            load_versions(&versions_path).map_err(|err| {
-                OperationError::service_error(format!("Failed to load ID tracker versions: {err}"))
-            })?
+        let internal_to_version = if has_versions {
+            load_versions(&versions_path).map_err(|err| {
+                OperationError::service_error(format!(
+                    "Failed to load ID tracker versions ({}): {err}",
+                    versions_path.display(),
+                ))
+            })?
         } else {
             vec![]
         };

Based on learnings

---

724-733: Optional: add context when taking versions writer into inner (consistency).

Currently relies on From<io::Error>; add the path in the message like mappings.

-    let file = writer.into_inner().map_err(|err| err.into_error())?;
+    let file = writer.into_inner().map_err(|e| {
+        let err = e.into_error();
+        OperationError::service_error(format!(
+            "Failed to finalize ID tracker point versions writer ({}): {err}",
+            versions_path.display(),
+        ))
+    })?;

---

411-420: Optional: consider preserving path context in OneshotFile errors.

OneshotFile::open likely wraps std::fs::File, so errors here may miss the path context fs-err provides. If feasible, adapt OneshotFile to hold/wrap fs_err::File (e.g., via fs_err::File::from_parts::<&Path> as used elsewhere) so subsequent ops (metadata/seek) retain path context.

---

441-476: Doc mismatch: iterator no longer yields “(change, bytes_read)”.

The docs still mention returning a tuple, but the iterator yields OperationResult<MappingChange>.

-/// Each non-errorous item is a tuple of the mapping change and the number of bytes read so far.
+/// Each item is a mapping change (`Ok`) or an error (`Err`) if malformed data is encountered.

lib/segment/src/vector_storage/async_io.rs (4)

124-135: Use checked 64-bit offset math to avoid overflow and clarify intent

Offset can overflow on large datasets or narrow platforms. Compute it in u64 with checked_add/checked_mul and surface a clear error.

-            let offset = self.header_size + self.raw_size * point as usize;
+            // Compute file offset safely in u64 to match io_uring expectations.
+            let offset = (self.header_size as u64)
+                .checked_add(
+                    (self.raw_size as u64)
+                        .checked_mul(point as u64)
+                        .ok_or_else(|| OperationError::service_error("offset multiplication overflow"))?,
+                )
+                .ok_or_else(|| OperationError::service_error("offset addition overflow"))?;
@@
-            .offset(offset as _)
+            .offset(offset as _)

---

137-142: Document unsafe rationale at submission site

Add a brief SAFETY comment explaining buffer lifetime and invariants for io_uring SQE push.

             unsafe {
-                // self.io_uring.submission().push(&read_e).unwrap();
+                // SAFETY:
+                // - `buffer.as_mut_ptr()` points into `self.buffers`, which outlives the I/O and
+                //   is not moved while the request is in flight.
+                // - We only reuse `buffer_id` after completion (see completion handling below).
+                // - `offset`/`len` are validated above.
                 io_uring.submission().push(&read_e).map_err(|err| {
                     OperationError::service_error(format!("Failed using io-uring: {err}"))
                 })?;
             }

---

180-189: Return richer OS errors from CQE failures

Map negative CQE results to std::io::Error for human-friendly diagnostics (errno message), aligning with the PR’s better error context goal.

-        if result < 0 {
-            return Err(OperationError::service_error(format!(
-                "io_uring operation failed with {result} error"
-            )));
+        if result < 0 {
+            let errno = -result;
+            let io_err = std::io::Error::from_raw_os_error(errno);
+            return Err(OperationError::service_error(format!(
+                "io_uring read failed: {io_err} (errno {errno})"
+            )));

---

194-195: Avoid transmute-style conversions for slices

Consider replacing transmute_from_u8_to_slice with bytemuck::cast_slice (or zerocopy) to tighten safety contracts. This aligns with our guideline to avoid transmute_* helpers in new code. If T: PrimitiveVectorElement implies Pod/FromBytes, this should be feasible; otherwise, keep as-is.

As per coding guidelines

Example (if T: bytemuck::Pod):

let vector: &[T] = bytemuck::cast_slice(buffer);

lib/quantization/src/encoded_vectors_binary.rs (1)

460-474: Use fs_err for directory creation — optional simplification.

Logic is correct and benefits from fs_err’s pathful errors. You could simplify the flow to avoid combinators for readability.

-        if let Some(meta_path) = meta_path {
-            meta_path
-                .parent()
-                .ok_or_else(|| {
-                    std::io::Error::new(
-                        std::io::ErrorKind::InvalidInput,
-                        "Path must have a parent directory",
-                    )
-                })
-                .and_then(fs::create_dir_all)
-                .map_err(|e| {
-                    EncodingError::EncodingError(format!(
-                        "Failed to create metadata directory: {e}",
-                    ))
-                })?;
+        if let Some(meta_path) = meta_path {
+            let parent = meta_path.parent().ok_or_else(|| {
+                std::io::Error::new(
+                    std::io::ErrorKind::InvalidInput,
+                    "Path must have a parent directory",
+                )
+            })?;
+            fs::create_dir_all(parent).map_err(|e| {
+                EncodingError::EncodingError(format!("Failed to create metadata directory: {e}",))
+            })?;
             atomic_save_json(meta_path, &metadata).map_err(|e| {
                 EncodingError::EncodingError(format!("Failed to save metadata: {e}",))
             })?;
         }

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

117-121: Swap to tokio_fs::metadata — and consider removing unwraps.

Change itself is good. While here, consider avoiding panics on non‑UTF8 names:

-    let name = path.file_name().unwrap().to_str().unwrap();
+    let name = path
+        .file_name()
+        .map(|s| s.to_string_lossy().into_owned())
+        .unwrap_or_else(|| path.as_os_str().to_string_lossy().into_owned());

lib/common/io/src/storage_version.rs (1)

28-36: Preserve path context on read errors using fs_err::read_to_string

file.read_to_string won't carry path context if it fails. Since the PR’s goal is richer IO errors, prefer fs_err::read_to_string(&version_file) and handle NotFound similarly.

-        let mut contents = String::new();
-        let mut file = match File::open(&version_file) {
-            Ok(file) => file,
-            Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
-                return Ok(None);
-            }
-            Err(err) => return Err(err.into()),
-        };
-        file.read_to_string(&mut contents)?;
+        let contents = match fs_err::read_to_string(&version_file) {
+            Ok(s) => s,
+            Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
+                return Ok(None);
+            }
+            Err(err) => return Err(err.into()),
+        };

lib/collection/src/config.rs (1)

262-264: Use fs_err::read_to_string for better diagnostics

read_to_string on a File omits path context. Replace with fs_err::read_to_string(&config_path) to keep enriched errors consistent with the PR’s intent.

-        let mut contents = String::new();
-        let mut file = File::open(config_path)?;
-        file.read_to_string(&mut contents)?;
-        Ok(serde_json::from_str(&contents)?)
+        let contents = fs_err::read_to_string(&config_path)?;
+        Ok(serde_json::from_str(&contents)?)

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

139-147: Avoid TOCTOU: call create_dir_all unconditionally

The pre-check with Path::exists() is unnecessary and slightly racy. create_dir_all is idempotent.

-        // Ensure temp_path exists
-        if !self.temp_path().exists() {
-            fs::create_dir_all(self.temp_path()).map_err(|err| {
-                CollectionError::service_error(format!(
-                    "Could not create temp directory `{}`: {}",
-                    self.temp_path().display(),
-                    err
-                ))
-            })?;
-        }
+        // Ensure temp_path exists (idempotent)
+        fs::create_dir_all(self.temp_path()).map_err(|err| {
+            CollectionError::service_error(format!(
+                "Could not create temp directory `{}`: {}",
+                self.temp_path().display(),
+                err
+            ))
+        })?;

lib/segment/src/vector_storage/dense/memmap_dense_vector_storage.rs (1)

306-310: Avoid extra allocation in open_append

to_path_buf() is unnecessary here; fs_err::OpenOptions::open accepts AsRef<Path>. This saves an allocation without changing behavior.

 fn open_append<P: AsRef<Path>>(path: P) -> io::Result<File> {
-    let path = path.as_ref().to_path_buf();
-    OpenOptions::new().append(true).open(path)
+    OpenOptions::new().append(true).open(path)
 }

lib/collection/src/shards/local_shard/mod.rs (3)

164-171: Avoid TOCTOU on removal: treat NotFound as success.

exists() + remove_dir_all() can race. Consider ignoring ErrorKind::NotFound to make cleanup idempotent.

-        if wal_path.exists() {
-            tokio_fs::remove_dir_all(wal_path).await?;
-        }
+        if wal_path.exists() {
+            if let Err(err) = tokio_fs::remove_dir_all(&wal_path).await {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    return Err(err.into());
+                }
+            }
+        }

-        if segments_path.exists() {
-            tokio_fs::remove_dir_all(segments_path).await?;
-        }
+        if segments_path.exists() {
+            if let Err(err) = tokio_fs::remove_dir_all(&segments_path).await {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    return Err(err.into());
+                }
+            }
+        }

---

347-353: Be lenient on leftover segment purge (ignore NotFound).

During concurrent startup/cleanup, the segment directory might already be gone; ignore ENOENT.

-                        fs::remove_dir_all(&segment_path).map_err(|err| {
+                        fs::remove_dir_all(&segment_path).or_else(|err| {
+                            if err.kind() == std::io::ErrorKind::NotFound {
+                                Ok(())
+                            } else {
+                                Err(err)
+                            }
+                        }).map_err(|err| {
                             CollectionError::service_error(format!(
                                 "failed to remove leftover segment {}: {err}",
                                 segment_path.display(),
                             ))
                         })?;

---

1168-1173: Ignore NotFound when deleting clock files.

Races can surface if files are concurrently removed; treat ENOENT as success.

-        if newest_clocks_path.exists() {
-            tokio_fs::remove_file(newest_clocks_path).await?;
-        }
+        if newest_clocks_path.exists() {
+            if let Err(err) = tokio_fs::remove_file(&newest_clocks_path).await {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    return Err(err.into());
+                }
+            }
+        }

-        if oldest_clocks_path.exists() {
-            tokio_fs::remove_file(oldest_clocks_path).await?;
-        }
+        if oldest_clocks_path.exists() {
+            if let Err(err) = tokio_fs::remove_file(&oldest_clocks_path).await {
+                if err.kind() != std::io::ErrorKind::NotFound {
+                    return Err(err.into());
+                }
+            }
+        }

lib/collection/src/shards/replica_set/snapshots.rs (1)

290-298: Make cleanup idempotent: ignore NotFound on removals.

Removals during recovery can race with prior cleanup or partial state. Treat ENOENT as success to avoid spurious failures.

-                    tokio_fs::remove_dir_all(&segment_path)
-                        .await
-                        .map_err(|err| {
+                    if let Err(err) = tokio_fs::remove_dir_all(&segment_path).await {
+                        if err.kind() != io::ErrorKind::NotFound {
+                            return Err(CollectionError::service_error(format!(
+                                "failed to remove outdated segment {}: {err}",
+                                segment_path.display(),
+                            )));
+                        }
+                    }
-                        })?;

-                                tokio_fs::remove_file(&path).await.map_err(|err| {
-                                    CollectionError::service_error(format!(
-                                        "failed to remove outdated segment file {}: {err}",
-                                        path.display(),
-                                    ))
-                                })?;
+                                if let Err(err) = tokio_fs::remove_file(&path).await {
+                                    if err.kind() != io::ErrorKind::NotFound {
+                                        return Err(CollectionError::service_error(format!(
+                                            "failed to remove outdated segment file {}: {err}",
+                                            path.display(),
+                                        )));
+                                    }
+                                }

-                    tokio_fs::remove_dir_all(&wal_path).await.map_err(|err| {
-                        CollectionError::service_error(format!(
-                            "failed to remove WAL {}: {err}",
-                            wal_path.display(),
-                        ))
-                    })?;
+                    if let Err(err) = tokio_fs::remove_dir_all(&wal_path).await {
+                        if err.kind() != io::ErrorKind::NotFound {
+                            return Err(CollectionError::service_error(format!(
+                                "failed to remove WAL {}: {err}",
+                                wal_path.display(),
+                            )));
+                        }
+                    }

-                tokio_fs::remove_file(&shard_flag).await?;
+                if let Err(err) = tokio_fs::remove_file(&shard_flag).await {
+                    if err.kind() != io::ErrorKind::NotFound {
+                        return Err(err.into());
+                    }
+                }

Also applies to: 335-341, 375-381, 410-410

lib/collection/src/common/snapshots_manager.rs (1)

274-275: Avoid blocking the async runtime: use tokio_fs::create_dir_all in async fns

These synchronous directory creations can block the runtime under load. Switch to fs_err::tokio’s async create_dir_all and await it.

@@
-        if let Some(target_dir) = target_path.parent() {
-            fs::create_dir_all(target_dir)?;
-        }
+        if let Some(target_dir) = target_path.parent() {
+            tokio_fs::create_dir_all(target_dir).await?;
+        }
@@
-        if let Some(target_dir) = local_path.parent()
+        if let Some(target_dir) = local_path.parent()
             && !target_dir.exists()
         {
-            fs::create_dir_all(target_dir)?;
+            tokio_fs::create_dir_all(target_dir).await?;
         }
@@
-        if let Some(target_dir) = local_path.parent()
+        if let Some(target_dir) = local_path.parent()
             && !target_dir.exists()
         {
-            fs::create_dir_all(target_dir)?;
+            tokio_fs::create_dir_all(target_dir).await?;
         }

Also applies to: 306-307, 416-417

lib/collection/src/collection/snapshots.rs (1)

7-7: Good switch to fs_err::File for richer errors during archive creation

Using fs_err::File surfaces the path on failure when creating the archive, which aligns with the PR’s goals. Looks good.

Minor: File::create inside an async fn can be wrapped in spawn_blocking if you anticipate slow FS in some environments, but it’s typically fine for a single create.

Also applies to: 83-83

lib/collection/benches/prof.rs (1)

6-7: Bench: fs_err integration LGTM

• Using fs_err::{self as fs, File} in benches is consistent.
• create_dir_all via fs_err adds path context; unwraps are acceptable in benches.

Optionally replace the unwrap with expect("failed to create benchmark dir") for a clearer panic.

Also applies to: 65-65

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: generall
PR: qdrant/qdrant#6854
File: lib/segment/src/index/query_estimator.rs:320-327
Timestamp: 2025-07-11T11:35:21.549Z
Learning: In test code for Qdrant's query estimator (lib/segment/src/index/query_estimator.rs), simplified ID resolution logic using `id.to_string().parse().unwrap()` is acceptable for testing purposes and doesn't need to match production code's `id_tracker.internal_id()` approach. Test code can use mock implementations that serve the testing goals.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_vectors.rs:857-857
Timestamp: 2025-08-15T11:41:10.629Z
Learning: In `lib/segment/src/vector_storage/quantized/quantized_vectors.rs`, the multivector offset storage has two different patterns: for RAM storage, offsets are collected into Vec<MultivectorOffset> and used directly; for MMAP storage, offsets are consumed to create a file via create_offsets_file_from_iter, then the file is loaded back as MultivectorOffsetsStorageMmap. The direct consumption of offsets iterator in the MMAP case is intentional.

Learnt from: timvisee
PR: qdrant/qdrant#6150
File: lib/segment/src/id_tracker/mutable_id_tracker.rs:62-71
Timestamp: 2025-03-12T16:23:44.715Z
Learning: In the MutableIdTracker implementation in Qdrant, the error case where versions file exists but mappings file is missing is considered a system-level error (potential corruption) rather than a user error, and is handled via logging and debug assertions without surfacing user-facing errors.

Learnt from: generall
PR: qdrant/qdrant#6635
File: lib/segment/src/index/field_index/full_text_index/text_index.rs:784-832
Timestamp: 2025-06-14T20:35:10.603Z
Learning: Functions gated with `#[cfg(feature = "testing")]` within `#[cfg(test)]` modules are safe from compilation issues since both the function and its call sites are restricted to test builds. The additional feature gate is often used for optional heavy test utilities.

Learnt from: CR
PR: qdrant/qdrant#0
File: .github/review-rules.md:0-0
Timestamp: 2025-08-29T08:16:28.521Z
Learning: Applies to **/{tests,benches}/**/*.rs : Using .unwrap() and panic!() in tests and benchmarks is acceptable and should not be flagged

Learnt from: generall
PR: qdrant/qdrant#7100
File: lib/segment/src/index/field_index/full_text_index/inverted_index/mutable_inverted_index.rs:82-100
Timestamp: 2025-08-20T15:03:18.522Z
Learning: The merge_postings_iterator function in lib/segment/src/index/field_index/full_text_index/inverted_index/postings_iterator.rs handles deduplication internally, so there's no need to add .dedup() when using it for OR-merge operations in inverted indexes.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7193
File: lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs:17-30
Timestamp: 2025-09-01T11:19:26.371Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_chunked_mmap_storage.rs, the ChunkedMmapVectors underlying implementation does not validate against zero-sized vectors, so adding such validation in QuantizedChunkedMmapStorage::new is unnecessary and would be redundant.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#7043
File: lib/segment/src/vector_storage/quantized/quantized_multivector_storage.rs:340-347
Timestamp: 2025-08-18T10:56:43.707Z
Learning: In lib/segment/src/vector_storage/quantized/quantized_multivector_storage.rs, the create_offsets_file_from_iter function intentionally requires paths to have a parent directory and returns an error if path.parent() is None. This was a conscious design decision to ensure proper path validation.

Learnt from: IvanPleshkov
PR: qdrant/qdrant#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.

Learnt from: timvisee
PR: qdrant/qdrant#6644
File: lib/gridstore/src/blob.rs:41-57
Timestamp: 2025-06-06T07:52:38.478Z
Learning: In the Blob trait implementations for gridstore (lib/gridstore/src/blob.rs), the maintainer prefers using assert! for input validation over Result-based error handling, as they don't expect invalid inputs in real scenarios and changing to Result would require updating the entire trait interface.

Learnt from: generall
PR: qdrant/qdrant#6323
File: lib/segment/src/vector_storage/quantized/quantized_mmap_storage.rs:12-16
Timestamp: 2025-04-07T23:31:22.515Z
Learning: The `populate()` method in `QuantizedMmapStorage` does not return a `Result` because the underlying `mmap.populate()` method doesn't return a Result either. Adding an unnecessary `Ok(())` would cause Clippy to complain.