ec: remove .ecsum sidecars on destroy / shard delete; align Go and Rust cleanup (#10307)

* fix(rust-volume): remove .ecsum sidecars on EC destroy / shard delete

Rust EcVolume::destroy removed shards and .ecx/.ecj/.vif but left bitrot
checksum sidecars (.ecsum / .ecsum.v*). On clusters that run weed-volume
(not Go weed volume), collection.delete therefore orphans every sidecar
while correctly wiping shards — observed live on 4.39 (14/14 .ecsum
survived after collection.delete on a freshly encoded EC volume).

Go Destroy already calls RemoveBitrotSidecars; this brings Rust to parity:
- hoist remove_bitrot_sidecars into ec_bitrot (shared helper)
- call it from EcVolume::destroy for dir / dir_idx / ecx_actual_dir
- call it from Store::delete_ec_shards when a disk has no remaining shards
- unit test: test_destroy_removes_bitrot_sidecar

* rust volume: gate the shard-delete sidecar sweep on a local shard removal

Only sweep a disk's .ecsum when this delete actually removed a shard file
there, matching Go's found gate: a delete that never touched a disk must not
strip 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. The shard-presence probe now treats unexpected stat errors as
"exists" so a transient failure cannot orphan-classify live shards, and
check_all_ec_shards_deleted reuses it.

* rust volume: destroy() sidecar sweep needs only the data and idx bases

ecx_actual_dir is always one of the two, so the third branch could never
run; this is now exactly Go Destroy()'s two-base sweep.

* rust volume: call the shared sidecar removal helper directly

* rust volume: unit-test remove_bitrot_sidecars

Mirrors Go's TestRemoveBitrotSidecars: legacy and versioned sidecars are
removed, a shard file and a longer-vid sidecar survive, absent is success.

* rust volume: keep the shared idx-base sidecar while a sibling disk has shards

One -dir.idx serves every location, so emptying one disk must not sweep
<idx>/<vol>.ecsum out from under a sibling that still holds shards. Nothing
reads the idx-base sidecar today, but .ecx shows index-dir files are real;
this keeps the defensive sweep safe if a writer ever lands one there.

* ec shard delete: keep the shared idx-base sidecar while a sibling disk has shards

One -dir.idx serves every disk, so emptying one disk must not sweep
<idx>/<vol>.ecsum out from under a sibling that still holds shards of the
volume — the same gate the Rust volume server applies. A status error counts
as in-use so a transient failure never strips it early.

* rust volume: drop a shard-only disk's stale .vif with the node's last shard

Go's removeEcSharedIndexFiles also clears the data-base .vif in the
all-shards-gone pass, gated on .idx absence so a disk still hosting the
source volume keeps its live .vif; the Rust delete path left it behind.
Unexpected stat errors count as .idx-present so a transient failure never
strips a live volume's .vif.

---------

Co-authored-by: Chris Lu <chris.lu@gmail.com>
This commit is contained in:
Eliah Rusin
2026-07-11 00:06:36 -05:00
committed by GitHub
parent ce82e3a057
commit c006dc563e
7 changed files with 438 additions and 58 deletions
+1 -43
View File
@@ -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
/// `<base>.ecsum` (generation 0) and any versioned `<base>.ecsum.v<N>`.
/// 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<io::Error> = 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));
@@ -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
/// `<base>.ecsum` (generation 0) and any versioned `<base>.ecsum.v<N>`.
///
/// 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<io::Error> = 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<u8> {
@@ -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
@@ -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 <base>.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]
+183 -11
View File
@@ -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.
@@ -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 <idx>/<volume>.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")
}
+18 -3
View File
@@ -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
@@ -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 {