perf(filemeta): phase-1~3 rename_data metadata optimization (#3011)

* chore(perf): harden amd64 profiling benchmark flow

* fix(profiling): isolate bench buckets and map protobuf conflict

* perf: avoid blocking owned local writes

* style: format profile admin handler

* docs: clarify observability trace validation

* perf: reduce mkdir overhead on local writes

* perf: add rename_data meta microbenchmark

* perf(filemeta): fast-path data_dir decode in version meta

* perf(filemeta): collapse data-dir lookup into one scan

* perf(filemeta): reduce scan allocs and refresh meta bench

* perf(ecstore): skip mkdir path on read-only open

* perf(filemeta): single-pass unshared data-dir scan

* perf(filemeta): add two-key inline remove fast path

* perf(filemeta): compare remove-two keys by bytes first

* bench(ecstore): add remove_two-only micro benchmark

* bench(ecstore): stabilize rename_data meta benchmark timing

* bench(ecstore): align rename_data path with remove_two

* perf(filemeta): avoid uuid string alloc in remove_two

* perf(filemeta): add fast-path for empty inline data

* perf(filemeta): streamline add_version match branch

* perf(filemeta): fast-return remove_key on miss

* perf(filemeta): speed up add_version insertion lookup

* style(ecstore): normalize formatting in perf-tuning files

* refactor(filemeta): unify inline data removal paths

crates/ecstore/Cargo.toml

@@ -145,5 +145,9 @@ harness = false
 name = "comparison_benchmark"
 harness = false
 
+[[bench]]
+name = "rename_data_meta_benchmark"
+harness = false
+
 [lib]
 doctest = false

crates/ecstore/benches/rename_data_meta_benchmark.rs

@@ -0,0 +1,118 @@
+// Copyright 2024 RustFS Team
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use criterion::{BatchSize, BenchmarkId, Criterion, criterion_group, criterion_main};
+use rustfs_filemeta::{ErasureAlgo, FileInfo, FileMeta};
+use std::hint::black_box;
+use std::time::Duration;
+use time::OffsetDateTime;
+use uuid::Uuid;
+
+const VERSION_COUNT_CASES: &[usize] = &[1, 8, 32, 64];
+const BENCH_BASE_TIME_UNIX_SECS: i64 = 1_700_000_000;
+
+fn make_file_info(version_id: Uuid, data_dir: Uuid, size: i64, mod_time: OffsetDateTime) -> FileInfo {
+    FileInfo {
+        version_id: Some(version_id),
+        data_dir: Some(data_dir),
+        size,
+        mod_time: Some(mod_time),
+        metadata: [("etag".to_string(), format!("etag-{version_id}"))].into_iter().collect(),
+        erasure: rustfs_filemeta::ErasureInfo {
+            algorithm: ErasureAlgo::ReedSolomon.to_string(),
+            data_blocks: 4,
+            parity_blocks: 2,
+            block_size: 1024 * 1024,
+            index: 1,
+            distribution: vec![1, 2, 3, 4, 5, 6],
+            ..Default::default()
+        },
+        ..Default::default()
+    }
+}
+
+fn build_meta_with_versions(version_count: usize) -> FileMeta {
+    let mut meta = FileMeta::new();
+    let base_time = OffsetDateTime::from_unix_timestamp(BENCH_BASE_TIME_UNIX_SECS).expect("valid bench base timestamp");
+    for i in 0..version_count {
+        let fi = make_file_info(Uuid::new_v4(), Uuid::new_v4(), 64 * 1024, base_time - Duration::from_secs(i as u64));
+        meta.add_version(fi).expect("seed add_version should succeed");
+    }
+    meta
+}
+
+fn bench_rename_data_meta_path(c: &mut Criterion) {
+    let mut group = c.benchmark_group("rename_data_meta");
+    group.sample_size(20);
+    group.measurement_time(Duration::from_secs(10));
+
+    for &version_count in VERSION_COUNT_CASES {
+        let seeded = build_meta_with_versions(version_count);
+        let dst_buf = seeded.marshal_msg().expect("marshal seeded meta");
+        let base_time = OffsetDateTime::from_unix_timestamp(BENCH_BASE_TIME_UNIX_SECS).expect("valid bench base timestamp");
+        let replace_version_id = seeded
+            .versions
+            .first()
+            .and_then(|v| v.header.version_id)
+            .unwrap_or(Uuid::nil());
+
+        group.bench_with_input(BenchmarkId::new("read_modify_write", version_count), &version_count, |b, _| {
+            b.iter(|| {
+                let mut xlmeta = FileMeta::load(black_box(&dst_buf)).expect("load dst meta");
+                let search_version_id = Some(replace_version_id);
+                let has_old_data_dir = xlmeta.find_unshared_data_dir_for_version(search_version_id);
+                if let Some(old_data_dir) = has_old_data_dir {
+                    let _ = xlmeta.data.remove_two(replace_version_id, old_data_dir);
+                }
+                let fi = make_file_info(replace_version_id, Uuid::new_v4(), 64 * 1024, base_time + Duration::from_millis(1));
+                xlmeta.add_version(fi).expect("add new version");
+                let out = xlmeta.marshal_msg().expect("marshal updated meta");
+                black_box(out);
+            });
+        });
+
+        let mut prepared = FileMeta::load(&dst_buf).expect("load prepared meta");
+        if let Some(old_data_dir) = prepared.find_unshared_data_dir_for_version(Some(replace_version_id)) {
+            let _ = prepared.data.remove_two(replace_version_id, old_data_dir);
+        }
+        group.bench_with_input(BenchmarkId::new("add_version_marshal_only", version_count), &version_count, |b, _| {
+            b.iter_batched(
+                || prepared.clone(),
+                |mut xlmeta| {
+                    let fi = make_file_info(replace_version_id, Uuid::new_v4(), 64 * 1024, base_time + Duration::from_millis(1));
+                    xlmeta.add_version(fi).expect("add new version");
+                    let out = xlmeta.marshal_msg().expect("marshal updated meta");
+                    black_box(out);
+                },
+                BatchSize::SmallInput,
+            );
+        });
+
+        group.bench_with_input(BenchmarkId::new("remove_two_only", version_count), &version_count, |b, _| {
+            b.iter(|| {
+                let mut xlmeta = FileMeta::load(black_box(&dst_buf)).expect("load dst meta");
+                let removed = if let Some(old_data_dir) = xlmeta.find_unshared_data_dir_for_version(Some(replace_version_id)) {
+                    xlmeta.data.remove_two(replace_version_id, old_data_dir).expect("remove two")
+                } else {
+                    false
+                };
+                black_box(removed);
+                black_box(xlmeta);
+            });
+        });
+    }
+}
+
+criterion_group!(benches, bench_rename_data_meta_path);
+criterion_main!(benches);

crates/ecstore/src/disk/local.rs

@@ -1194,7 +1194,9 @@ impl LocalDisk {
             skip_parent = self.root.as_path();
         }
 
-        if let Some(parent) = path.as_ref().parent() {
+        if let Some(parent) = path.as_ref().parent()
+            && parent != skip_parent
+        {
             os::make_dir_all(parent, skip_parent).await?;
         }
 
@@ -1203,6 +1205,11 @@ impl LocalDisk {
         Ok(f)
     }
 
+    async fn open_file_read_only(&self, path: impl AsRef<Path>) -> Result<File> {
+        let f = super::fs::open_file(path.as_ref(), O_RDONLY).await.map_err(to_file_error)?;
+        Ok(f)
+    }
+
     #[allow(dead_code)]
     fn get_metrics(&self) -> DiskMetrics {
         DiskMetrics::default()
@@ -2137,7 +2144,7 @@ impl DiskAPI for LocalDisk {
         let file_path = self.get_object_path(volume, path)?;
         check_path_length(file_path.to_string_lossy().as_ref())?;
 
-        let f = self.open_file(file_path, O_RDONLY, volume_dir).await?;
+        let f = self.open_file_read_only(file_path).await?;
 
         Ok(Box::new(f))
     }
@@ -2154,7 +2161,7 @@ impl DiskAPI for LocalDisk {
         let file_path = self.get_object_path(volume, path)?;
         check_path_length(file_path.to_string_lossy().as_ref())?;
 
-        let mut f = self.open_file(file_path, O_RDONLY, volume_dir).await?;
+        let mut f = self.open_file_read_only(file_path).await?;
 
         let meta = f.metadata().await?;
         let end_offset = offset.checked_add(length).ok_or(DiskError::FileCorrupt)?;
@@ -2513,32 +2520,41 @@ impl DiskAPI for LocalDisk {
 
         // TODO: Healing
 
-        let search_version_id = fi.version_id.or(Some(Uuid::nil()));
+        let version_id = fi.version_id.unwrap_or_default();
+        let search_version_id = Some(version_id);
+        let no_inline = fi.data.is_none() && fi.size > 0;
 
         // Check if there's an existing version with the same version_id that has a data_dir to clean up
-        let has_old_data_dir = {
-            xlmeta.find_version(search_version_id).ok().and_then(|(_, ver)| {
-                // shard_count == 0 means no other version shares this data_dir
-                ver.get_data_dir()
-                    .filter(|&data_dir| xlmeta.shard_data_dir_count(&search_version_id, &Some(data_dir)) == 0)
-            })
-        };
+        // Reuse one metadata scan to find the version data_dir and determine whether it is shared.
+        let has_old_data_dir = xlmeta.find_unshared_data_dir_for_version(search_version_id);
         if let Some(old_data_dir) = has_old_data_dir.as_ref() {
-            let _ = xlmeta.data.remove(vec![search_version_id.unwrap_or_default(), *old_data_dir]);
+            let _ = xlmeta.data.remove_two(version_id, *old_data_dir);
         }
 
-        xlmeta.add_version(fi.clone())?;
+        xlmeta.add_version(fi)?;
 
         if xlmeta.versions.len() <= 10 {
             // TODO: Sign
         }
 
-        let new_dst_buf = xlmeta.marshal_msg()?;
-
-        self.write_all(src_volume, format!("{}/{}", &src_path, STORAGE_FORMAT_FILE).as_str(), new_dst_buf.into())
-            .await?;
         if let Some((src_data_path, dst_data_path)) = has_data_dir_path.as_ref() {
-            let no_inline = fi.data.is_none() && fi.size > 0;
+            let src_file_parent = src_file_path.parent().unwrap_or(src_volume_dir.as_path());
+            let meta_skip_parent = if no_inline {
+                src_file_parent
+            } else {
+                src_volume_dir.as_path()
+            };
+            let new_dst_buf = xlmeta.marshal_msg()?;
+
+            self.write_all_private(
+                src_volume,
+                format!("{}/{}", &src_path, STORAGE_FORMAT_FILE).as_str(),
+                new_dst_buf.into(),
+                true,
+                meta_skip_parent,
+            )
+            .await?;
+
             if no_inline && let Err(err) = rename_all(&src_data_path, &dst_data_path, &skip_parent).await {
                 let _ = self.delete_file(&dst_volume_dir, dst_data_path, false, false).await;
                 info!(
@@ -2547,6 +2563,10 @@ impl DiskAPI for LocalDisk {
                 );
                 return Err(err);
             }
+        } else {
+            let new_dst_buf = xlmeta.marshal_msg()?;
+            self.write_all(src_volume, format!("{}/{}", &src_path, STORAGE_FORMAT_FILE).as_str(), new_dst_buf.into())
+                .await?;
         }
 
         if let Some(old_data_dir) = has_old_data_dir {

crates/ecstore/src/disk/os.rs

@@ -215,24 +215,29 @@ pub async fn os_mkdir_all(dir_path: impl AsRef<Path>, base_dir: impl AsRef<Path>
         return Ok(());
     }
 
-    if let Some(parent) = dir_path.as_ref().parent() {
-        // Without recursion support, fall back to create_dir_all
-        if let Err(e) = super::fs::make_dir_all(&parent).await {
-            if e.kind() == io::ErrorKind::AlreadyExists {
-                return Ok(());
-            }
-
-            return Err(e);
-        }
-        // Box::pin(os_mkdir_all(&parent, &base_dir)).await?;
-    }
-
     if let Err(e) = super::fs::mkdir(dir_path.as_ref()).await {
         if e.kind() == io::ErrorKind::AlreadyExists {
             return Ok(());
         }
 
-        return Err(e);
+        if e.kind() != io::ErrorKind::NotFound {
+            return Err(e);
+        }
+
+        if let Some(parent) = dir_path.as_ref().parent() {
+            // Fall back to creating the missing parent chain only when the direct mkdir proves it is required.
+            if let Err(parent_err) = super::fs::make_dir_all(parent).await
+                && parent_err.kind() != io::ErrorKind::AlreadyExists
+            {
+                return Err(parent_err);
+            }
+        }
+
+        if let Err(retry_err) = super::fs::mkdir(dir_path.as_ref()).await
+            && retry_err.kind() != io::ErrorKind::AlreadyExists
+        {
+            return Err(retry_err);
+        }
     }
 
     Ok(())

crates/filemeta/src/filemeta.rs

@@ -282,6 +282,11 @@ impl FileMeta {
             fi.version_id = Some(Uuid::nil());
         }
 
+        if fi.data.is_none() && self.data.after_version().is_empty() {
+            let version = FileMetaVersion::from(fi);
+            return self.add_version_filemata(version);
+        }
+
         let version_key = data_key_for_version(fi.version_id);
         let mut next_data = self.data.clone();
 
@@ -317,46 +322,30 @@ impl FileMeta {
         }
 
         let vid = version.get_version_id();
-
-        // Match existing version for replace; null version: None and Some(nil) are equivalent
-        let matches = |h: &Option<Uuid>| {
-            let v_null = vid.is_none() || vid == Some(Uuid::nil());
-            let h_null = h.is_none() || *h == Some(Uuid::nil());
-            (v_null && h_null) || (vid == *h)
+        let vid_is_null = vid.is_none() || vid == Some(Uuid::nil());
+        let existing_idx = if vid_is_null {
+            self.versions
+                .iter()
+                .position(|v| v.header.version_id.is_none() || v.header.version_id == Some(Uuid::nil()))
+        } else {
+            self.versions.iter().position(|v| v.header.version_id == vid)
         };
 
-        if let Some(fidx) = self.versions.iter().position(|v| matches(&v.header.version_id)) {
+        if let Some(fidx) = existing_idx {
             return self.set_idx(fidx, version);
         }
 
-        // append placeholder to find insert position
-        let placeholder = FileMetaShallowVersion {
-            header: FileMetaVersionHeader {
-                mod_time: None, // None sorts before any real mod_time
-                ..Default::default()
-            },
-            meta: Vec::new(),
-        };
-        self.versions.push(placeholder);
-
         let mod_time = version.get_mod_time();
         let new_shallow = FileMetaShallowVersion::try_from(version)?;
-
-        for (idx, exist) in self.versions.iter().enumerate() {
-            let ex_mt = exist.header.mod_time;
-            let insert_here = match (ex_mt, mod_time) {
-                (None, _) => true, // placeholder: always insert before
-                (Some(em), Some(nm)) => em <= nm,
-                (Some(_), None) => false,
-            };
-            if insert_here {
-                self.versions.insert(idx, new_shallow);
-                self.versions.pop(); // remove placeholder
-                return Ok(());
-            }
-        }
-        self.versions.pop(); // remove placeholder on fallback
-        Err(Error::other("add_version failed"))
+        let insert_pos = match mod_time {
+            Some(nm) => self.versions.partition_point(|exist| match exist.header.mod_time {
+                Some(em) => em > nm,
+                None => false,
+            }),
+            None => self.versions.partition_point(|exist| exist.header.mod_time.is_some()),
+        };
+        self.versions.insert(insert_pos, new_shallow);
+        Ok(())
 
         // if !ver.valid() {
         //     return Err(Error::other("attempted to add invalid version"));

crates/filemeta/src/filemeta/inline_data.rs

@@ -13,8 +13,48 @@
 // limitations under the License.
 
 use super::*;
+use std::collections::HashSet;
 
 impl FileMeta {
+    pub fn find_unshared_data_dir_for_version(&self, version_id: Option<Uuid>) -> Option<Uuid> {
+        let vid = version_id.unwrap_or_default();
+        let mut target_data_dir = None;
+        let mut target_selected = false;
+        let mut other_data_dirs = HashSet::new();
+
+        for version in self
+            .versions
+            .iter()
+            .filter(|v| v.header.version_type == VersionType::Object && v.header.uses_data_dir())
+        {
+            let is_target_version = version.header.version_id.unwrap_or_default() == vid;
+            if is_target_version {
+                if target_selected {
+                    continue;
+                }
+
+                target_selected = true;
+                target_data_dir = FileMetaVersion::decode_data_dir_from_meta(&version.meta).unwrap_or_default();
+                if let Some(dir) = target_data_dir
+                    && other_data_dirs.contains(&dir)
+                {
+                    return None;
+                }
+                continue;
+            }
+
+            let dir = FileMetaVersion::decode_data_dir_from_meta(&version.meta).unwrap_or_default();
+            if let Some(dir) = dir {
+                if target_data_dir == Some(dir) {
+                    return None;
+                }
+                other_data_dirs.insert(dir);
+            }
+        }
+
+        target_data_dir
+    }
+
     pub fn shard_data_dir_count(&self, vid: &Option<Uuid>, data_dir: &Option<Uuid>) -> usize {
         let vid = vid.unwrap_or_default();
         self.versions
@@ -58,3 +98,70 @@ impl FileMeta {
             .count()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use s3s::header::X_AMZ_RESTORE;
+    use time::format_description::well_known::Rfc3339;
+    use time::{Duration, OffsetDateTime};
+
+    fn make_file_info(version_id: Uuid, data_dir: Uuid) -> FileInfo {
+        let restore_header = format!(
+            "ongoing-request=\"false\", expiry-date=\"{}\"",
+            (OffsetDateTime::now_utc() + Duration::days(1))
+                .format(&Rfc3339)
+                .expect("format restore expiry"),
+        );
+        FileInfo {
+            version_id: Some(version_id),
+            data_dir: Some(data_dir),
+            size: 64 * 1024,
+            mod_time: Some(OffsetDateTime::now_utc()),
+            metadata: [
+                ("etag".to_string(), format!("etag-{version_id}")),
+                (X_AMZ_RESTORE.as_str().to_string(), restore_header),
+            ]
+            .into_iter()
+            .collect(),
+            erasure: ErasureInfo {
+                algorithm: ErasureAlgo::ReedSolomon.to_string(),
+                data_blocks: 4,
+                parity_blocks: 2,
+                block_size: 1024 * 1024,
+                index: 1,
+                distribution: vec![1, 2, 3, 4, 5, 6],
+                ..Default::default()
+            },
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn find_unshared_data_dir_for_version_returns_data_dir_when_unique() {
+        let target_version = Uuid::new_v4();
+        let target_data_dir = Uuid::new_v4();
+        let mut meta = FileMeta::new();
+        meta.add_version(make_file_info(target_version, target_data_dir))
+            .expect("seed target version");
+        meta.add_version(make_file_info(Uuid::new_v4(), Uuid::new_v4()))
+            .expect("seed non-shared version");
+
+        let got = meta.find_unshared_data_dir_for_version(Some(target_version));
+        assert_eq!(got, Some(target_data_dir));
+    }
+
+    #[test]
+    fn find_unshared_data_dir_for_version_returns_none_when_shared() {
+        let target_version = Uuid::new_v4();
+        let shared_data_dir = Uuid::new_v4();
+        let mut meta = FileMeta::new();
+        meta.add_version(make_file_info(target_version, shared_data_dir))
+            .expect("seed target version");
+        meta.add_version(make_file_info(Uuid::new_v4(), shared_data_dir))
+            .expect("seed shared version");
+
+        let got = meta.find_unshared_data_dir_for_version(Some(target_version));
+        assert_eq!(got, None);
+    }
+}

crates/filemeta/src/filemeta/version.rs

@@ -312,6 +312,77 @@ pub struct FileMetaVersion {
 }
 
 impl FileMetaVersion {
+    fn decode_data_dir_from_v2_object(buf: &[u8]) -> Result<Option<Uuid>> {
+        let mut cur = std::io::Cursor::new(buf);
+        let mut fields = rmp::decode::read_map_len(&mut cur)?;
+        let mut version_type = VersionType::Invalid;
+
+        while fields > 0 {
+            fields -= 1;
+
+            let key_len = rmp::decode::read_str_len(&mut cur)? as usize;
+            let mut key_buf = vec![0u8; key_len];
+            cur.read_exact(&mut key_buf)?;
+            let key = String::from_utf8(key_buf)?;
+
+            match key.as_str() {
+                "Type" => {
+                    let v: i64 = rmp::decode::read_int(&mut cur)?;
+                    version_type = VersionType::from_u8(v as u8);
+                }
+                "V2Obj" => {
+                    if version_type != VersionType::Object {
+                        skip_msgp_value(&mut cur)?;
+                        continue;
+                    }
+
+                    let mut first = [0u8; 1];
+                    cur.read_exact(&mut first)?;
+                    if first[0] == 0xc0 {
+                        return Ok(None);
+                    }
+
+                    let mut prepend = PrependByteReader {
+                        byte: Some(first[0]),
+                        inner: &mut cur,
+                    };
+                    let mut obj_fields = rmp::decode::read_map_len(&mut prepend)?;
+                    let mut data_dir: Option<Uuid> = None;
+
+                    while obj_fields > 0 {
+                        obj_fields -= 1;
+
+                        let obj_key_len = rmp::decode::read_str_len(&mut prepend)? as usize;
+                        let mut obj_key_buf = vec![0u8; obj_key_len];
+                        prepend.read_exact(&mut obj_key_buf)?;
+                        let obj_key = String::from_utf8(obj_key_buf)?;
+
+                        if obj_key == "DDir" {
+                            let bin_len = rmp::decode::read_bin_len(&mut prepend)? as usize;
+                            if bin_len != 16 {
+                                return Err(Error::other(format!("DDir must be 16 bytes, got {bin_len}")));
+                            }
+                            let mut raw = [0u8; 16];
+                            prepend.read_exact(&mut raw)?;
+                            let id = Uuid::from_bytes(raw);
+                            data_dir = if id.is_nil() { None } else { Some(id) };
+                            break;
+                        }
+
+                        skip_msgp_value(&mut prepend)?;
+                    }
+
+                    return Ok(data_dir);
+                }
+                _ => {
+                    skip_msgp_value(&mut cur)?;
+                }
+            }
+        }
+
+        Ok(None)
+    }
+
     pub fn valid(&self) -> bool {
         if !self.version_type.valid() {
             return false;
@@ -367,6 +438,9 @@ impl FileMetaVersion {
 
     // decode_data_dir_from_meta reads data_dir from meta TODO: directly parse only data_dir from meta buf, msg.skip
     pub fn decode_data_dir_from_meta(buf: &[u8]) -> Result<Option<Uuid>> {
+        if let Ok(data_dir) = Self::decode_data_dir_from_v2_object(buf) {
+            return Ok(data_dir);
+        }
         Ok(Self::try_from(buf)?.get_data_dir())
     }
 
@@ -3213,4 +3287,31 @@ mod tests {
         assert_eq!(fi.mod_time, Some(sample_mod_time()));
         assert_eq!(fi.metadata.get("x-rustfs-test").map(String::as_str), Some("gone"));
     }
+
+    #[test]
+    fn decode_data_dir_from_meta_extracts_v2_object_fast_path() {
+        let data_dir = Uuid::new_v4();
+        let version = FileMetaVersion {
+            version_type: VersionType::Object,
+            object: Some(MetaObject {
+                version_id: Some(Uuid::new_v4()),
+                data_dir: Some(data_dir),
+                erasure_algorithm: ErasureAlgo::ReedSolomon,
+                erasure_m: 2,
+                erasure_n: 4,
+                erasure_block_size: 1024 * 1024,
+                erasure_index: 1,
+                erasure_dist: vec![1, 2, 3, 4, 5, 6],
+                bitrot_checksum_algo: ChecksumAlgo::HighwayHash,
+                size: 64 * 1024,
+                mod_time: Some(OffsetDateTime::now_utc()),
+                ..Default::default()
+            }),
+            ..Default::default()
+        };
+
+        let encoded = version.marshal_msg().expect("marshal");
+        let decoded = FileMetaVersion::decode_data_dir_from_meta(&encoded).expect("decode data_dir");
+        assert_eq!(decoded, Some(data_dir));
+    }
 }

crates/filemeta/src/filemeta_inline.rs

@@ -23,6 +23,141 @@ pub struct InlineData(Vec<u8>);
 const INLINE_DATA_VER: u8 = 1;
 
 impl InlineData {
+    fn contains_key_by<F>(&self, mut should_remove: F) -> Result<bool>
+    where
+        F: FnMut(&[u8]) -> bool,
+    {
+        let buf = self.after_version();
+        if buf.is_empty() {
+            return Ok(false);
+        }
+
+        let mut scan_cur = Cursor::new(buf);
+        let mut scan_fields_len = rmp::decode::read_map_len(&mut scan_cur)? as usize;
+
+        while scan_fields_len > 0 {
+            scan_fields_len -= 1;
+
+            let str_len = rmp::decode::read_str_len(&mut scan_cur)? as usize;
+            let key_start = scan_cur.position() as usize;
+            let key_end = key_start + str_len;
+            scan_cur.set_position(key_end as u64);
+
+            let bin_len = rmp::decode::read_bin_len(&mut scan_cur)? as usize;
+            let value_start = scan_cur.position() as usize;
+            let value_end = value_start + bin_len;
+            scan_cur.set_position(value_end as u64);
+
+            if should_remove(&buf[key_start..key_end]) {
+                return Ok(true);
+            }
+        }
+
+        Ok(false)
+    }
+
+    fn remove_keys_by<F>(&mut self, mut should_remove: F) -> Result<bool>
+    where
+        F: FnMut(&[u8]) -> bool,
+    {
+        let buf = self.after_version();
+        if buf.is_empty() {
+            return Ok(false);
+        }
+
+        let mut cur = Cursor::new(buf);
+        let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize;
+        let mut keys = Vec::with_capacity(fields_len);
+        let mut values = Vec::with_capacity(fields_len);
+        let mut found = false;
+
+        while fields_len > 0 {
+            fields_len -= 1;
+
+            let str_len = rmp::decode::read_str_len(&mut cur)? as usize;
+            let mut field_buf = vec![0u8; str_len];
+            cur.read_exact(&mut field_buf)?;
+
+            let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize;
+            let start = cur.position() as usize;
+            let end = start + bin_len;
+            cur.set_position(end as u64);
+
+            if should_remove(field_buf.as_slice()) {
+                found = true;
+                continue;
+            }
+
+            keys.push(String::from_utf8(field_buf)?);
+            values.push(buf[start..end].to_vec());
+        }
+
+        if !found {
+            return Ok(false);
+        }
+
+        if keys.is_empty() {
+            self.0 = Vec::new();
+            return Ok(true);
+        }
+
+        self.serialize(keys, values)?;
+        Ok(true)
+    }
+
+    fn remove_two_keys_by_bytes(&mut self, first_key: &[u8], second_key: &[u8]) -> Result<bool> {
+        let buf = self.after_version();
+        if buf.is_empty() {
+            return Ok(false);
+        }
+
+        let same = first_key == second_key;
+        let mut cur = Cursor::new(buf);
+        let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize;
+        let mut keys = Vec::with_capacity(fields_len + 1);
+        let mut values = Vec::with_capacity(fields_len + 1);
+        let mut found = false;
+
+        while fields_len > 0 {
+            fields_len -= 1;
+
+            let str_len = rmp::decode::read_str_len(&mut cur)? as usize;
+            let mut field_buf = vec![0u8; str_len];
+            cur.read_exact(&mut field_buf)?;
+
+            let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize;
+            let start = cur.position() as usize;
+            let end = start + bin_len;
+            cur.set_position(end as u64);
+
+            let should_remove = if same {
+                field_buf.as_slice() == first_key
+            } else {
+                field_buf.as_slice() == first_key || field_buf.as_slice() == second_key
+            };
+
+            if should_remove {
+                found = true;
+                continue;
+            }
+
+            keys.push(String::from_utf8(field_buf)?);
+            values.push(buf[start..end].to_vec());
+        }
+
+        if !found {
+            return Ok(false);
+        }
+
+        if keys.is_empty() {
+            self.0 = Vec::new();
+            return Ok(true);
+        }
+
+        self.serialize(keys, values)?;
+        Ok(true)
+    }
+
     pub fn new() -> Self {
         Self(Vec::new())
     }
@@ -183,115 +318,29 @@ impl InlineData {
     }
 
     pub fn remove_key(&mut self, key: &str) -> Result<bool> {
-        let buf = self.after_version();
-        if buf.is_empty() {
+        let key_bytes = key.as_bytes();
+        if !self.contains_key_by(|candidate| candidate == key_bytes)? {
             return Ok(false);
         }
-
-        let mut cur = Cursor::new(buf);
-
-        let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize;
-        let mut keys = Vec::with_capacity(fields_len);
-        let mut values = Vec::with_capacity(fields_len);
-        let mut found = false;
-
-        while fields_len > 0 {
-            fields_len -= 1;
-
-            let str_len = rmp::decode::read_str_len(&mut cur)?;
-
-            let mut field_buff = vec![0u8; str_len as usize];
-
-            cur.read_exact(&mut field_buff)?;
-
-            let find_key = String::from_utf8(field_buff)?;
-
-            let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize;
-            let start = cur.position() as usize;
-            let end = start + bin_len;
-            cur.set_position(end as u64);
-
-            if find_key == key {
-                found = true;
-                continue;
-            }
-
-            keys.push(find_key);
-            values.push(buf[start..end].to_vec());
-        }
-
-        if !found {
-            return Ok(false);
-        }
-
-        if keys.is_empty() {
-            self.0 = Vec::new();
-            return Ok(true);
-        }
-
-        self.serialize(keys, values)?;
-        Ok(true)
+        self.remove_keys_by(|candidate| candidate == key_bytes)
     }
 
     pub fn remove(&mut self, remove_keys: Vec<Uuid>) -> Result<bool> {
-        let buf = self.after_version();
-        if buf.is_empty() {
-            return Ok(false);
-        }
-        let mut cur = Cursor::new(buf);
-
-        let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize;
-        let mut keys = Vec::with_capacity(fields_len + 1);
-        let mut values = Vec::with_capacity(fields_len + 1);
-
-        let remove_key = |found_key: &str| {
-            for key in remove_keys.iter() {
-                if key.to_string().as_str() == found_key {
-                    return true;
-                }
-            }
-            false
-        };
-
-        let mut found = false;
-
-        while fields_len > 0 {
-            fields_len -= 1;
-
-            let str_len = rmp::decode::read_str_len(&mut cur)?;
-
-            let mut field_buff = vec![0u8; str_len as usize];
-
-            cur.read_exact(&mut field_buff)?;
-
-            let find_key = String::from_utf8(field_buff)?;
-
-            let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize;
-            let start = cur.position() as usize;
-            let end = start + bin_len;
-            cur.set_position(end as u64);
-
-            let find_value = &buf[start..end];
-
-            if !remove_key(&find_key) {
-                values.push(find_value.to_vec());
-                keys.push(find_key);
-            } else {
-                found = true;
-            }
+        let mut encoded_keys = Vec::with_capacity(remove_keys.len());
+        for key in remove_keys {
+            let mut buf = Uuid::encode_buffer();
+            encoded_keys.push(key.hyphenated().encode_lower(&mut buf).to_string().into_bytes());
         }
 
-        if !found {
-            return Ok(false);
-        }
+        self.remove_keys_by(|candidate| encoded_keys.iter().any(|key| candidate == key.as_slice()))
+    }
 
-        if keys.is_empty() {
-            self.0 = Vec::new();
-            return Ok(true);
-        }
-
-        self.serialize(keys, values)?;
-        Ok(true)
+    pub fn remove_two(&mut self, first: Uuid, second: Uuid) -> Result<bool> {
+        let mut first_buf = Uuid::encode_buffer();
+        let mut second_buf = Uuid::encode_buffer();
+        let first_key = first.hyphenated().encode_lower(&mut first_buf).as_bytes();
+        let second_key = second.hyphenated().encode_lower(&mut second_buf).as_bytes();
+        self.remove_two_keys_by_bytes(first_key, second_key)
     }
     fn serialize(&mut self, keys: Vec<String>, values: Vec<Vec<u8>>) -> Result<()> {
         assert_eq!(keys.len(), values.len(), "InlineData serialize: keys/values not match");
@@ -319,3 +368,44 @@ impl InlineData {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn remove_key_miss_keeps_inline_data_unchanged() {
+        let mut data = InlineData::new();
+        data.replace("keep", b"value".to_vec()).expect("seed inline data");
+        let before = data.as_slice().to_vec();
+
+        let removed = data.remove_key("missing").expect("remove_key should succeed");
+
+        assert!(!removed);
+        assert_eq!(data.as_slice(), before.as_slice());
+    }
+
+    #[test]
+    fn remove_two_removes_only_matching_keys() {
+        let first = Uuid::new_v4();
+        let second = Uuid::new_v4();
+        let keep = Uuid::new_v4();
+        let mut data = InlineData::new();
+        data.replace(first.hyphenated().to_string().as_str(), b"first".to_vec())
+            .expect("seed first key");
+        data.replace(second.hyphenated().to_string().as_str(), b"second".to_vec())
+            .expect("seed second key");
+        data.replace(keep.hyphenated().to_string().as_str(), b"keep".to_vec())
+            .expect("seed keep key");
+
+        let removed = data.remove_two(first, second).expect("remove_two should succeed");
+
+        assert!(removed);
+        assert_eq!(data.find(first.hyphenated().to_string().as_str()).expect("find first"), None);
+        assert_eq!(data.find(second.hyphenated().to_string().as_str()).expect("find second"), None);
+        assert_eq!(
+            data.find(keep.hyphenated().to_string().as_str()).expect("find keep"),
+            Some(b"keep".to_vec())
+        );
+    }
+}