diff --git a/seaweed-volume/src/storage/disk_location.rs b/seaweed-volume/src/storage/disk_location.rs index a8458fb6b..7464fc955 100644 --- a/seaweed-volume/src/storage/disk_location.rs +++ b/seaweed-volume/src/storage/disk_location.rs @@ -13,6 +13,7 @@ use std::sync::Arc; use tracing::warn; use crate::config::MinFreeSpace; +use crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars; use crate::storage::erasure_coding::ec_shard::{ EcVolumeShard, DATA_SHARDS_COUNT, ERASURE_CODING_LARGE_BLOCK_SIZE, ERASURE_CODING_SMALL_BLOCK_SIZE, @@ -1095,49 +1096,6 @@ fn rm_if_present(path: String) -> io::Result<()> { } } -/// Remove the bitrot checksum sidecars for a base file name: the legacy -/// `.ecsum` (generation 0) and any versioned `.ecsum.v`. -/// Already-gone is success; returns the first real removal failure (and surfaces -/// a directory-scan error) so a stale sidecar left behind is not reported as -/// cleaned. Mirrors Go's removeBitrotSidecars. -fn remove_bitrot_sidecars(base: &str) -> io::Result<()> { - use crate::storage::erasure_coding::ec_bitrot::BITROT_SIDECAR_EXT; - let rm = |path: std::path::PathBuf| -> io::Result<()> { - match fs::remove_file(&path) { - Err(e) if e.kind() != io::ErrorKind::NotFound => Err(e), - _ => Ok(()), - } - }; - let mut first_err: Option = None; - let mut record = |res: io::Result<()>| { - if let Err(e) = res { - if first_err.is_none() { - first_err = Some(e); - } - } - }; - record(rm(format!("{}{}", base, BITROT_SIDECAR_EXT).into())); - let path = std::path::Path::new(base); - if let (Some(parent), Some(fname)) = (path.parent(), path.file_name()) { - let prefix = format!("{}{}.v", fname.to_string_lossy(), BITROT_SIDECAR_EXT); - match fs::read_dir(parent) { - Ok(entries) => { - for entry in entries.flatten() { - if entry.file_name().to_string_lossy().starts_with(&prefix) { - record(rm(entry.path())); - } - } - } - Err(e) if e.kind() != io::ErrorKind::NotFound => record(Err(e)), - Err(_) => {} - } - } - match first_err { - Some(e) => Err(e), - None => Ok(()), - } -} - fn ec_data_shards_from_vif(directory: &str, idx_directory: &str, collection: &str, vid: VolumeId) -> usize { for dir in [directory, idx_directory] { let vif = format!("{}.vif", volume_file_name(dir, collection, vid)); diff --git a/seaweed-volume/src/storage/erasure_coding/ec_bitrot.rs b/seaweed-volume/src/storage/erasure_coding/ec_bitrot.rs index 1d959ce81..c3583b05e 100644 --- a/seaweed-volume/src/storage/erasure_coding/ec_bitrot.rs +++ b/seaweed-volume/src/storage/erasure_coding/ec_bitrot.rs @@ -146,6 +146,52 @@ pub fn bitrot_sidecar_path(base: &str, generation: u32) -> String { } } +/// Remove the bitrot checksum sidecars for a base file name: the legacy +/// `.ecsum` (generation 0) and any versioned `.ecsum.v`. +/// +/// Already-gone is success; returns the first real removal failure (and surfaces +/// a directory-scan error) so a stale sidecar left behind is not reported as +/// cleaned. Mirrors Go's `RemoveBitrotSidecars` / `removeBitrotSidecars`. +pub fn remove_bitrot_sidecars(base: &str) -> io::Result<()> { + use std::fs; + use std::path::Path; + + let rm = |path: std::path::PathBuf| -> io::Result<()> { + match fs::remove_file(&path) { + Err(e) if e.kind() != io::ErrorKind::NotFound => Err(e), + _ => Ok(()), + } + }; + let mut first_err: Option = None; + let mut record = |res: io::Result<()>| { + if let Err(e) = res { + if first_err.is_none() { + first_err = Some(e); + } + } + }; + record(rm(format!("{}{}", base, BITROT_SIDECAR_EXT).into())); + let path = Path::new(base); + if let (Some(parent), Some(fname)) = (path.parent(), path.file_name()) { + let prefix = format!("{}{}.v", fname.to_string_lossy(), BITROT_SIDECAR_EXT); + match fs::read_dir(parent) { + Ok(entries) => { + for entry in entries.flatten() { + if entry.file_name().to_string_lossy().starts_with(&prefix) { + record(rm(entry.path())); + } + } + } + Err(e) if e.kind() != io::ErrorKind::NotFound => record(Err(e)), + Err(_) => {} + } + } + match first_err { + Some(e) => Err(e), + None => Ok(()), + } +} + /// Returns a fresh random per-encode identity used to detect a stale sidecar /// left behind by an in-place re-encode. pub fn new_encode_uuid() -> Vec { @@ -553,6 +599,40 @@ mod tests { assert_eq!(bitrot_sidecar_path("/d/1", 3), "/d/1.ecsum.v3"); } + /// Mirrors Go's TestRemoveBitrotSidecars: legacy + versioned sidecars go, + /// unrelated files (a shard, a longer-vid sidecar) survive, absent is success. + #[test] + fn test_remove_bitrot_sidecars() { + let tmp = tempfile::TempDir::new().unwrap(); + let base = tmp.path().join("5").to_str().unwrap().to_string(); + for p in [ + format!("{}.ecsum", base), + format!("{}.ecsum.v1", base), + format!("{}.ecsum.v7", base), + ] { + std::fs::write(&p, b"x").unwrap(); + } + let keep_shard = format!("{}.ec00", base); + let keep_other_vid = format!("{}0.ecsum", base); + std::fs::write(&keep_shard, b"x").unwrap(); + std::fs::write(&keep_other_vid, b"x").unwrap(); + + remove_bitrot_sidecars(&base).unwrap(); + + for p in [ + format!("{}.ecsum", base), + format!("{}.ecsum.v1", base), + format!("{}.ecsum.v7", base), + ] { + assert!(!std::path::Path::new(&p).exists(), "{} should be removed", p); + } + assert!(std::path::Path::new(&keep_shard).exists()); + assert!(std::path::Path::new(&keep_other_vid).exists()); + + // Already-gone is success. + remove_bitrot_sidecars(&base).unwrap(); + } + #[test] fn test_is_pow2_multiple_of_1mib() { assert!(is_pow2_multiple_of_1mib(1 << 20)); // 1 MiB diff --git a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs index e67418a8b..261574073 100644 --- a/seaweed-volume/src/storage/erasure_coding/ec_volume.rs +++ b/seaweed-volume/src/storage/erasure_coding/ec_volume.rs @@ -1453,6 +1453,21 @@ impl EcVolume { let _ = fs::remove_file(format!("{}.ecj", data_base)); let _ = fs::remove_file(format!("{}.vif", data_base)); } + // Go's Destroy() also removes bitrot checksum sidecars so a later + // volume-id reuse cannot load stale protection, and so + // collection.delete does not leave orphaned .ecsum files. + // ecx_actual_dir is always one of these two dirs. + let _ = crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars( + &self.base_name(), + ); + if self.dir_idx != self.dir { + let idx_base = crate::storage::volume::volume_file_name( + &self.dir_idx, + &self.collection, + self.volume_id, + ); + let _ = crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars(&idx_base); + } self.ecx_file = None; self.ecj_file = None; } @@ -1463,6 +1478,82 @@ mod tests { use super::*; use tempfile::TempDir; + /// `destroy()` must remove co-located `.ecsum` sidecars (Go Destroy parity). + /// Without this, `collection.delete` leaves orphaned bitrot files that + /// inflate EC-health scanners after the shards are gone. + #[test] + fn test_destroy_removes_bitrot_sidecar() { + use crate::storage::needle_map::NeedleMapKind; + use crate::storage::volume::Volume; + + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + let mut v = Volume::new( + dir, + dir, + "ec1c", + VolumeId(2074), + NeedleMapKind::InMemory, + None, + None, + 0, + Version::current(), + ) + .unwrap(); + for i in 1..=3 { + let data = format!("needle {}", i); + let mut n = Needle { + id: NeedleId(i), + cookie: Cookie(i as u32), + data: data.as_bytes().to_vec(), + data_size: data.len() as u32, + ..Needle::default() + }; + v.write_needle(&mut n, true).unwrap(); + } + v.sync_to_disk().unwrap(); + v.close(); + crate::storage::erasure_coding::ec_encoder::write_ec_files( + dir, + dir, + "ec1c", + VolumeId(2074), + 10, + 4, + ) + .unwrap(); + + let base = crate::storage::volume::volume_file_name(dir, "ec1c", VolumeId(2074)); + let ecsum = format!("{}.ecsum", base); + assert!( + std::path::Path::new(&ecsum).exists(), + "precondition: encode must write generation-0 .ecsum" + ); + assert!( + std::path::Path::new(&format!("{}.ec00", base)).exists(), + "precondition: encode must write shards" + ); + + let mut vol = EcVolume::new(dir, dir, "ec1c", VolumeId(2074)).unwrap(); + // Mount at least one local shard so destroy's shard loop has work. + vol.add_shard(EcVolumeShard::new(dir, "ec1c", VolumeId(2074), 0)) + .unwrap(); + vol.destroy(); + + assert!( + !std::path::Path::new(&format!("{}.ec00", base)).exists(), + "destroy should remove shards" + ); + assert!( + !std::path::Path::new(&ecsum).exists(), + "destroy should remove co-located .ecsum (Go Destroy parity)" + ); + assert!( + !std::path::Path::new(&format!("{}.ecx", base)).exists(), + "destroy should remove .ecx" + ); + } + /// Mounting an EC volume loads and validates its generation-0 `.ecsum` /// sidecar, so `bitrot_protection()` reports `On` with the parsed manifest. #[test] diff --git a/seaweed-volume/src/storage/store.rs b/seaweed-volume/src/storage/store.rs index da5aa177e..48567d12a 100644 --- a/seaweed-volume/src/storage/store.rs +++ b/seaweed-volume/src/storage/store.rs @@ -934,46 +934,108 @@ impl Store { /// Delete EC shard files from disk. pub fn delete_ec_shards(&mut self, vid: VolumeId, collection: &str, shard_ids: &[u32]) { - // Delete shard files from disk - for loc in &self.locations { + // Delete shard files from disk, tracking which locations actually held one. + let mut deleted_at = vec![false; self.locations.len()]; + for (i, loc) in self.locations.iter().enumerate() { for &shard_id in shard_ids { let shard = EcVolumeShard::new(&loc.directory, collection, vid, shard_id as u8); - let path = shard.file_name(); - let _ = std::fs::remove_file(&path); + if std::fs::remove_file(shard.file_name()).is_ok() { + deleted_at[i] = true; + } } } // Also unmount if mounted self.unmount_ec_shards(vid, shard_ids); + // Per-disk: when this delete removed the location's last shards, the local + // bitrot sidecar is orphaned — remove it (Go deleteEcShardIdsForEachLocation + // when found && existingShardCount == 0). The `deleted_at` gate keeps a + // delete that never touched a disk from stripping a sidecar it does not + // own: a shared -dir.idx sibling with surviving shards, or an ec.rebuild + // index-prep copy that lands .ecx/.ecsum before any shard. + for (i, loc) in self.locations.iter().enumerate() { + if !deleted_at[i] || Self::location_has_ec_shards(loc, collection, vid) { + continue; + } + let data_base = + crate::storage::volume::volume_file_name(&loc.directory, collection, vid); + let _ = crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars(&data_base); + if loc.idx_directory != loc.directory { + // A single -dir.idx is shared by every location: leave the idx-base + // sidecar alone while any sibling still holds shards of this volume. + let idx_in_use = self.locations.iter().any(|other| { + other.idx_directory == loc.idx_directory + && Self::location_has_ec_shards(other, collection, vid) + }); + if !idx_in_use { + let idx_base = crate::storage::volume::volume_file_name( + &loc.idx_directory, + collection, + vid, + ); + let _ = + crate::storage::erasure_coding::ec_bitrot::remove_bitrot_sidecars(&idx_base); + } + } + } + // If all shards are gone, remove .ecx and .ecj files from both idx and data dirs let all_gone = self.check_all_ec_shards_deleted(vid, collection); if all_gone { for loc in &self.locations { let idx_base = crate::storage::volume::volume_file_name(&loc.idx_directory, collection, vid); + let data_base = + crate::storage::volume::volume_file_name(&loc.directory, collection, vid); let _ = std::fs::remove_file(format!("{}.ecx", idx_base)); let _ = std::fs::remove_file(format!("{}.ecj", idx_base)); // Also try data directory in case .ecx/.ecj were created before -dir.idx if loc.idx_directory != loc.directory { - let data_base = - crate::storage::volume::volume_file_name(&loc.directory, collection, vid); let _ = std::fs::remove_file(format!("{}.ecx", data_base)); let _ = std::fs::remove_file(format!("{}.ecj", data_base)); } + // A shard-only disk also drops its stale .vif (Go + // removeEcSharedIndexFiles): a live .idx means this disk still + // hosts the source volume and the .vif belongs to it. Unexpected + // stat errors count as present so a transient failure never + // strips a live volume's .vif. + let has_idx = [&idx_base, &data_base].iter().any(|base| { + match std::fs::metadata(format!("{}.idx", base)) { + Ok(_) => true, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => false, + Err(_) => true, + } + }); + if !has_idx { + let _ = std::fs::remove_file(format!("{}.vif", data_base)); + } } } } + /// True if `loc` still has any on-disk EC shard for this volume. An + /// unexpected stat error (permission, I/O) counts as "exists" so a + /// transient failure never classifies live shards as gone and deletes + /// their sidecar or shared index. + fn location_has_ec_shards(loc: &DiskLocation, collection: &str, vid: VolumeId) -> bool { + for shard_id in 0..MAX_SHARD_COUNT as u8 { + let shard = EcVolumeShard::new(&loc.directory, collection, vid, shard_id); + match std::fs::metadata(shard.file_name()) { + Ok(_) => return true, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(_) => return true, + } + } + false + } + /// Check if all EC shard files have been deleted for a volume. /// Uses MAX_SHARD_COUNT to support non-standard EC configurations. fn check_all_ec_shards_deleted(&self, vid: VolumeId, collection: &str) -> bool { for loc in &self.locations { - for shard_id in 0..MAX_SHARD_COUNT as u8 { - let shard = EcVolumeShard::new(&loc.directory, collection, vid, shard_id); - if std::path::Path::new(&shard.file_name()).exists() { - return false; - } + if Self::location_has_ec_shards(loc, collection, vid) { + return false; } } true @@ -1682,6 +1744,116 @@ mod tests { ); } + /// Deleting a disk's last shard removes that disk's now-orphaned `.ecsum`; + /// a disk this delete never changed keeps its sidecar. Disk 0 models + /// ec.rebuild's index-prep state (`.ecsum` landed, no shards yet) — an + /// unrelated shard delete must not strip its just-copied protection. + #[test] + fn test_delete_ec_shards_sidecar_gated_on_local_delete() { + let (mut store, _tmp) = make_ec_target_test_store(2); + let collection = "c"; + let vid = VolumeId(7); + + let base0 = volume_file_name(&store.locations[0].directory, collection, vid); + let base1 = volume_file_name(&store.locations[1].directory, collection, vid); + std::fs::write(format!("{}.ecsum", base0), b"x").unwrap(); + std::fs::write(format!("{}.ec00", base1), b"x").unwrap(); + std::fs::write(format!("{}.ec01", base1), b"x").unwrap(); + std::fs::write(format!("{}.ecsum", base1), b"x").unwrap(); + + // Disk 1 still has .ec01 afterwards: both sidecars survive. + store.delete_ec_shards(vid, collection, &[0]); + assert!(std::path::Path::new(&format!("{}.ecsum", base0)).exists()); + assert!(std::path::Path::new(&format!("{}.ecsum", base1)).exists()); + + // Disk 1's last shard goes: its sidecar is orphaned and removed, but + // disk 0 was never touched by either delete and keeps its sidecar. + store.delete_ec_shards(vid, collection, &[1]); + assert!(!std::path::Path::new(&format!("{}.ec01", base1)).exists()); + assert!( + !std::path::Path::new(&format!("{}.ecsum", base1)).exists(), + "orphaned sidecar should be removed with the disk's last shard" + ); + assert!( + std::path::Path::new(&format!("{}.ecsum", base0)).exists(), + "a delete that removed nothing on this disk must not strip its sidecar" + ); + } + + /// With one -dir.idx shared by every location, emptying disk 0 must not + /// remove the idx-base sidecar while disk 1 still holds shards; it goes + /// only when the last location sharing the idx dir is emptied too. + #[test] + fn test_delete_ec_shards_preserves_shared_idx_sidecar() { + let tmp = TempDir::new().unwrap(); + let idx = tmp.path().join("idx"); + std::fs::create_dir_all(&idx).unwrap(); + let mut store = Store::new(NeedleMapKind::InMemory); + for i in 0..2 { + let path = tmp.path().join(format!("data{}", i)); + std::fs::create_dir_all(&path).unwrap(); + store + .add_location( + path.to_str().unwrap(), + idx.to_str().unwrap(), + 100, + DiskType::HardDrive, + MinFreeSpace::Percent(0.0), + Vec::new(), + ) + .unwrap(); + } + let collection = "c"; + let vid = VolumeId(7); + + let base0 = volume_file_name(&store.locations[0].directory, collection, vid); + let base1 = volume_file_name(&store.locations[1].directory, collection, vid); + let idx_base = volume_file_name(&store.locations[0].idx_directory, collection, vid); + std::fs::write(format!("{}.ec00", base0), b"x").unwrap(); + std::fs::write(format!("{}.ec01", base1), b"x").unwrap(); + std::fs::write(format!("{}.ecsum", idx_base), b"x").unwrap(); + + store.delete_ec_shards(vid, collection, &[0]); + assert!( + std::path::Path::new(&format!("{}.ecsum", idx_base)).exists(), + "shared idx sidecar must survive while a sibling disk still has shards" + ); + + store.delete_ec_shards(vid, collection, &[1]); + assert!( + !std::path::Path::new(&format!("{}.ecsum", idx_base)).exists(), + "shared idx sidecar should go with the last location's last shard" + ); + } + + /// When the node's last shard goes, a shard-only disk also drops its stale + /// .vif (Go removeEcSharedIndexFiles); a disk with a live .idx still hosts + /// the source volume and keeps its .vif. + #[test] + fn test_delete_ec_shards_vif_removal_gated_on_idx() { + let (mut store, _tmp) = make_ec_target_test_store(2); + let collection = "c"; + let vid = VolumeId(7); + let base0 = volume_file_name(&store.locations[0].directory, collection, vid); + let base1 = volume_file_name(&store.locations[1].directory, collection, vid); + std::fs::write(format!("{}.ec00", base0), b"x").unwrap(); + std::fs::write(format!("{}.vif", base0), b"x").unwrap(); + std::fs::write(format!("{}.ec01", base1), b"x").unwrap(); + std::fs::write(format!("{}.vif", base1), b"x").unwrap(); + std::fs::write(format!("{}.idx", base1), b"x").unwrap(); + + store.delete_ec_shards(vid, collection, &[0, 1]); + + assert!( + !std::path::Path::new(&format!("{}.vif", base0)).exists(), + "shard-only disk should drop its stale .vif with the node's last shard" + ); + assert!( + std::path::Path::new(&format!("{}.vif", base1)).exists(), + "a disk with a live .idx keeps its .vif" + ); + } + /// An already-mounted EC volume on disk 1 must win over a stray /// `.ecx` on disk 2. Protects the post-startup steady state from /// being perturbed by leftover index files from a prior failed move. diff --git a/weed/server/volume_grpc_ec_shared_index_test.go b/weed/server/volume_grpc_ec_shared_index_test.go index 4162d44aa..96766016e 100644 --- a/weed/server/volume_grpc_ec_shared_index_test.go +++ b/weed/server/volume_grpc_ec_shared_index_test.go @@ -85,3 +85,67 @@ func TestEcShardDeleteKeepsSharedIndexWhileSiblingHasShards(t *testing.T) { del(7, 12) require.False(t, util.FileExists(base0+".ecx"), "shared .ecx must be removed once no shard remains node-wide") } + +// Same protection for the idx-base bitrot sidecar: with one -dir.idx shared by +// every disk, emptying one disk must not sweep /.ecsum while a +// sibling disk still holds shards; it goes when the last sibling is emptied. +func TestEcShardDeleteKeepsSharedIdxSidecarWhileSiblingHasShards(t *testing.T) { + tempDir := t.TempDir() + dir0 := filepath.Join(tempDir, "disk0") + dir1 := filepath.Join(tempDir, "disk1") + idxDir := filepath.Join(tempDir, "idx") + for _, d := range []string{dir0, dir1, idxDir} { + require.NoError(t, os.MkdirAll(d, 0o755)) + } + const collection = "ec-shared-idx-sidecar" + vid := needle.VolumeId(78) + + store := storage.NewStore(nil, "localhost", 8080, 18080, "http://localhost:8080", "store-id", + []string{dir0, dir1}, []int32{100, 100}, []util.MinFreeSpace{{}, {}}, idxDir, + storage.NeedleMapInMemory, []types.DiskType{types.HardDriveType, types.HardDriveType}, nil, 3, stats.DefaultDiskIOProbeConfig()) + done := make(chan struct{}) + go func() { + for { + select { + case <-store.NewEcShardsChan: + case <-store.NewVolumesChan: + case <-store.DeletedVolumesChan: + case <-store.DeletedEcShardsChan: + case <-store.StateUpdateChan: + case <-done: + return + } + } + }() + t.Cleanup(func() { + store.Close() + close(done) + }) + + plant := func(dir string, ids ...int) { + base := erasure_coding.EcShardFileName(collection, dir, int(vid)) + for _, id := range ids { + require.NoError(t, os.WriteFile(base+erasure_coding.ToExt(id), []byte("s"), 0o644)) + } + } + plant(dir0, 0) + plant(dir1, 7) + idxSidecar := erasure_coding.EcShardFileName(collection, idxDir, int(vid)) + erasure_coding.BitrotSidecarExt + require.NoError(t, os.WriteFile(idxSidecar, []byte("x"), 0o644)) + + vs := &VolumeServer{store: store} + del := func(ids ...uint32) { + _, err := vs.VolumeEcShardsDelete(context.Background(), &volume_server_pb.VolumeEcShardsDeleteRequest{ + VolumeId: uint32(vid), + Collection: collection, + ShardIds: ids, + }) + require.NoError(t, err) + } + + del(0) + require.True(t, util.FileExists(idxSidecar), "shared idx sidecar must survive while a sibling disk holds shards") + + del(7) + require.False(t, util.FileExists(idxSidecar), "shared idx sidecar must be removed once no shard remains node-wide") +} diff --git a/weed/server/volume_grpc_erasure_coding.go b/weed/server/volume_grpc_erasure_coding.go index edce72d06..443632287 100644 --- a/weed/server/volume_grpc_erasure_coding.go +++ b/weed/server/volume_grpc_erasure_coding.go @@ -471,7 +471,7 @@ func (vs *VolumeServer) VolumeEcShardsDelete(ctx context.Context, req *volume_se // Pass 1: delete the requested shard files (and any now-orphaned per-disk bitrot // sidecars) on every disk. for diskId, location := range vs.store.Locations { - if err := deleteEcShardIdsForEachLocation(bName, location, req.ShardIds); err != nil { + if err := deleteEcShardIdsForEachLocation(bName, location, vs.store.Locations, req.ShardIds); err != nil { glog.Errorf("deleteEcShards from disk_id:%d %s %s.%v: %v", diskId, location.Directory, bName, req.ShardIds, err) return nil, err } @@ -508,7 +508,7 @@ func (vs *VolumeServer) VolumeEcShardsDelete(ctx context.Context, req *volume_se return &volume_server_pb.VolumeEcShardsDeleteResponse{}, nil } -func deleteEcShardIdsForEachLocation(bName string, location *storage.DiskLocation, shardIds []uint32) error { +func deleteEcShardIdsForEachLocation(bName string, location *storage.DiskLocation, locations []*storage.DiskLocation, shardIds []uint32) error { found := false @@ -546,7 +546,7 @@ func deleteEcShardIdsForEachLocation(bName string, location *storage.DiskLocatio if err := removeBitrotSidecars(dataBaseFilename); err != nil { return err } - if location.IdxDirectory != location.Directory { + if location.IdxDirectory != location.Directory && !idxSidecarInUse(bName, location.IdxDirectory, locations) { if err := removeBitrotSidecars(indexBaseFilename); err != nil { return err } @@ -556,6 +556,21 @@ func deleteEcShardIdsForEachLocation(bName string, location *storage.DiskLocatio return nil } +// One -dir.idx serves every disk, so the idx-base sidecar is shared: it stays +// while any disk using that idx directory still holds shards of this volume. +// A status error counts as in-use so a transient failure never strips it early. +func idxSidecarInUse(bName string, idxDirectory string, locations []*storage.DiskLocation) bool { + for _, other := range locations { + if other.IdxDirectory != idxDirectory { + continue + } + if _, _, count, err := checkEcVolumeStatus(bName, other); err != nil || count > 0 { + return true + } + } + return false +} + // removeEcSharedIndexFiles removes the shared .ecx/.ecj index (and the .vif when no // .idx is present) for an EC volume on one disk. The caller invokes it only after // the whole node's shards for the volume are gone, so a sibling disk's shards are diff --git a/weed/server/volume_grpc_erasure_coding_test.go b/weed/server/volume_grpc_erasure_coding_test.go index 49ad22577..da4e13b1c 100644 --- a/weed/server/volume_grpc_erasure_coding_test.go +++ b/weed/server/volume_grpc_erasure_coding_test.go @@ -143,7 +143,7 @@ func TestDeleteEcShardsWithoutLocalEcx(t *testing.T) { } location := &storage.DiskLocation{Directory: dataDir, IdxDirectory: idxDir} - if err := deleteEcShardIdsForEachLocation(baseName, location, []uint32{3, 11}); err != nil { + if err := deleteEcShardIdsForEachLocation(baseName, location, []*storage.DiskLocation{location}, []uint32{3, 11}); err != nil { t.Fatalf("deleteEcShardIdsForEachLocation: %v", err) } for _, f := range orphans {