From 4f8af455bf954a5d5f07b8f74d1a1b7d8f60fa53 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 11 Jun 2026 12:26:21 -0700 Subject: [PATCH] feat(storage): sweep leftover empty EC .dat stubs on volume server startup (#9927) * feat(storage): sweep leftover empty EC .dat stubs on volume server startup An EC volume keeps no local .dat. The pre-fix loader left empty 8-byte superblock .dat stubs next to EC metadata (one per lone .vif). Left in place each loads as a phantom empty volume, and the same vid's stub on two disks of one server blocks Rust startup via the duplicate-vid check in Store::add_location -- the prior fix stops creating new stubs but does not clean up existing ones. On startup, when a .dat is empty (<= a superblock, i.e. zero needles) and its .vif marks the volume erasure-coded, remove the stub (+ empty .idx) instead of loading it. The real data is in the EC shards, so the empty stub holds nothing to lose. Non-EC empty .dat files (e.g. freshly allocated volumes) are left alone. Done in both Rust (load_existing_volumes) and Go (loadExistingVolume), with regression tests that fail without the sweep. * refactor(storage): extract empty EC .dat stub sweep into its own function Move the startup stub-sweep into remove_empty_ec_dat_stub (Rust) and removeEmptyEcDatStub + vifIsEcVolume (Go) for clearer logic, and look up the .vif in both the data and idx directories (each read at most once) so a stub is still found when -dir.idx is configured. Adds direct tests for the idx-directory lookup on both engines. --- seaweed-volume/src/storage/disk_location.rs | 80 +++++++++++- .../src/storage/store_ec_reconcile.rs | 99 ++++++++++++++ weed/storage/disk_location.go | 37 ++++++ weed/storage/store_ec_phantom_dat_test.go | 122 ++++++++++++++++++ 4 files changed, 337 insertions(+), 1 deletion(-) diff --git a/seaweed-volume/src/storage/disk_location.rs b/seaweed-volume/src/storage/disk_location.rs index 1503a821a..18c702483 100644 --- a/seaweed-volume/src/storage/disk_location.rs +++ b/seaweed-volume/src/storage/disk_location.rs @@ -19,7 +19,7 @@ use crate::storage::erasure_coding::ec_shard::{ }; use crate::storage::erasure_coding::ec_volume::EcVolume; use crate::storage::needle_map::NeedleMapKind; -use crate::storage::super_block::ReplicaPlacement; +use crate::storage::super_block::{ReplicaPlacement, SUPER_BLOCK_SIZE}; use crate::storage::types::*; use crate::storage::volume::{ remove_volume_files, volume_file_name, VifVolumeInfo, Volume, VolumeError, @@ -138,6 +138,12 @@ impl DiskLocation { continue; } + // Sweep a leftover empty `.dat` stub (a phantom from the pre-fix + // loader) before it loads as a phantom volume or blocks startup. + if remove_empty_ec_dat_stub(&volume_name, &idx_name, vid) { + continue; + } + // If valid EC shards exist (.ecx file present), skip loading .dat let ecx_path = format!("{}.ecx", idx_name); let ecx_exists = if std::path::Path::new(&ecx_path).exists() { @@ -1076,6 +1082,47 @@ fn vif_references_remote_file(vif_path: &str) -> bool { .unwrap_or(false) } +/// True when a `.vif` records an EC shard config, i.e. the volume was +/// erasure-coded. Such a volume keeps no local `.dat`, so an empty `.dat` +/// alongside it is a leftover stub safe to remove. +fn vif_is_ec_volume(vif_path: &str) -> bool { + fs::read_to_string(vif_path) + .ok() + .and_then(|s| serde_json::from_str::(&s).ok()) + .map(|vif| vif.ec_shard_config.is_some()) + .unwrap_or(false) +} + +/// Remove a leftover empty EC `.dat` stub and return whether one was swept. +/// +/// A stub is an empty `.dat` (<= a superblock, i.e. zero needles) whose `.vif` +/// records an EC shard config. An EC volume keeps no local `.dat`, so the stub +/// holds no data — its shards live on other servers. Such stubs (phantoms from +/// the pre-fix loader) otherwise load as phantom empty volumes, and a same-vid +/// stub on two disks blocks startup via the duplicate-vid check. The `.dat` and +/// its empty `.idx` are removed; non-EC empty `.dat` files are left alone. The +/// `.vif` is looked up in both the data and idx directories (which differ only +/// when `-dir.idx` is configured), each read at most once. +fn remove_empty_ec_dat_stub(volume_name: &str, idx_name: &str, vid: VolumeId) -> bool { + let dat_path = format!("{}.dat", volume_name); + match fs::metadata(&dat_path) { + Ok(meta) if meta.len() <= SUPER_BLOCK_SIZE as u64 => {} + _ => return false, + } + + let data_vif = format!("{}.vif", volume_name); + let idx_vif = format!("{}.vif", idx_name); + let is_ec = vif_is_ec_volume(&data_vif) || (idx_vif != data_vif && vif_is_ec_volume(&idx_vif)); + if !is_ec { + return false; + } + + warn!(volume_id = vid.0, "removing leftover empty .dat stub for EC volume"); + let _ = fs::remove_file(&dat_path); + let _ = fs::remove_file(format!("{}.idx", idx_name)); + true +} + fn parse_volume_filename(filename: &str) -> Option<(String, VolumeId)> { let stem = filename .strip_suffix(".dat") @@ -1101,6 +1148,37 @@ mod tests { use super::*; use tempfile::TempDir; + /// When `-dir.idx` is configured the EC `.vif` may live in the idx + /// directory; the sweep must look there too, not only the data dir. + #[test] + fn test_remove_empty_ec_dat_stub_finds_vif_in_idx_dir() { + let tmp = TempDir::new().unwrap(); + let data = tmp.path().join("data"); + let idx = tmp.path().join("idx"); + std::fs::create_dir_all(&data).unwrap(); + std::fs::create_dir_all(&idx).unwrap(); + let vbase = format!("{}/warp-cal_42", data.to_str().unwrap()); + let ibase = format!("{}/warp-cal_42", idx.to_str().unwrap()); + std::fs::write(format!("{}.dat", vbase), vec![0u8; 8]).unwrap(); + + let vif = VifVolumeInfo { + version: 3, + ec_shard_config: Some(crate::storage::volume::VifEcShardConfig { + data_shards: 10, + parity_shards: 4, + ..Default::default() + }), + ..Default::default() + }; + std::fs::write(format!("{}.vif", ibase), serde_json::to_string(&vif).unwrap()).unwrap(); + + assert!( + remove_empty_ec_dat_stub(&vbase, &ibase, VolumeId(42)), + "EC .vif in the idx dir should be found and the stub removed", + ); + assert!(!std::path::Path::new(&format!("{}.dat", vbase)).exists()); + } + #[test] fn test_parse_volume_filename() { assert_eq!( diff --git a/seaweed-volume/src/storage/store_ec_reconcile.rs b/seaweed-volume/src/storage/store_ec_reconcile.rs index e0171e657..53d9a7589 100644 --- a/seaweed-volume/src/storage/store_ec_reconcile.rs +++ b/seaweed-volume/src/storage/store_ec_reconcile.rs @@ -466,6 +466,105 @@ mod tests { .unwrap(); } + fn write_ec_vif(dir: &str, collection: &str, vid: u32) { + let vif = VifVolumeInfo { + version: 3, + ec_shard_config: Some(VifEcShardConfig { + data_shards: 10, + parity_shards: 4, + ..Default::default() + }), + ..Default::default() + }; + std::fs::write( + format!("{}/{}_{}.vif", dir, collection, vid), + serde_json::to_string(&vif).unwrap(), + ) + .unwrap(); + } + + /// An empty `.dat` (<= a superblock, i.e. zero needles) for an EC volume + /// is a leftover stub from the pre-fix loader. It must be swept on startup, + /// not loaded as a phantom empty volume. With the same vid's stub on two + /// disks this also unblocks startup, which previously failed the + /// duplicate-vid check (the volume6 incident). + #[test] + fn test_empty_ec_dat_stub_removed_and_unblocks_startup() { + let tmp = TempDir::new().unwrap(); + let d0 = tmp.path().join("data0"); + let d1 = tmp.path().join("data1"); + std::fs::create_dir_all(&d0).unwrap(); + std::fs::create_dir_all(&d1).unwrap(); + let coll = "warp-cal"; + let vid = 41u32; + + // A real (loadable) but empty superblock: without the sweep both disks + // load it as vid 41 and add_location fails the duplicate-vid check. + let stub = crate::storage::super_block::SuperBlock { + version: crate::storage::types::Version::current(), + ..Default::default() + } + .to_bytes(); + for d in [&d0, &d1] { + let dir = d.to_str().unwrap(); + std::fs::write(format!("{}/{}_{}.dat", dir, coll, vid), &stub).unwrap(); + write_ec_vif(dir, coll, vid); + } + + let mut store = Store::new(NeedleMapKind::InMemory); + for d in [&d0, &d1] { + store + .add_location( + d.to_str().unwrap(), + d.to_str().unwrap(), + 100, + DiskType::HardDrive, + MinFreeSpace::Percent(0.0), + Vec::new(), + ) + .expect("a same-vid empty stub on two disks must not block startup"); + } + + let loaded: usize = store.locations.iter().map(|l| l.volume_ids().len()).sum(); + assert_eq!(loaded, 0, "empty EC stub was loaded as a phantom volume"); + for d in [&d0, &d1] { + assert!( + !std::path::Path::new(&format!("{}/{}_{}.dat", d.to_str().unwrap(), coll, vid)) + .exists(), + "empty .dat stub was not removed", + ); + } + } + + /// Safety: an empty `.dat` for a NON-EC volume (no EC `.vif`) is left + /// alone — only EC stubs are swept, so freshly-allocated empty volumes + /// survive. + #[test] + fn test_keeps_empty_dat_for_non_ec_volume() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join("data0"); + std::fs::create_dir_all(&dir).unwrap(); + let dat = format!("{}/7.dat", dir.to_str().unwrap()); + std::fs::write(&dat, vec![0u8; 8]).unwrap(); + + let mut store = Store::new(NeedleMapKind::InMemory); + store + .add_location( + dir.to_str().unwrap(), + dir.to_str().unwrap(), + 100, + DiskType::HardDrive, + MinFreeSpace::Percent(0.0), + Vec::new(), + ) + .unwrap(); + + assert!( + std::path::Path::new(&dat).exists(), + "a non-EC empty .dat must not be swept", + ); + } + /// Regression: a lone `.vif` whose `.ecx` is on a sibling disk must not /// make the loader create a phantom `.dat`, nor the sibling-.dat prune /// delete the real shards on the sibling. diff --git a/weed/storage/disk_location.go b/weed/storage/disk_location.go index ee32c87c7..eb47e1fcc 100644 --- a/weed/storage/disk_location.go +++ b/weed/storage/disk_location.go @@ -17,6 +17,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/stats" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/super_block" "github.com/seaweedfs/seaweedfs/weed/storage/types" "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" "github.com/seaweedfs/seaweedfs/weed/util" @@ -167,6 +168,36 @@ func (l *DiskLocation) hasEcxFile(volumeName string) bool { return false } +// removeEmptyEcDatStub removes a leftover empty EC .dat stub and returns +// whether one was swept. A stub is an empty .dat (<= a superblock, i.e. zero +// needles) whose .vif records an EC shard config. An EC volume keeps no local +// .dat, so the stub holds no data -- its shards live on other servers. Such +// stubs (phantoms from the pre-fix loader) otherwise load as phantom empty +// volumes, and a same-vid stub on two disks can shadow a real replica. The +// .dat and its empty .idx are removed; non-EC empty .dat files are left alone. +// The .vif is looked up in both the data and idx directories (which differ +// only when -dir.idx is configured). +func (l *DiskLocation) removeEmptyEcDatStub(volumeName string, vid needle.VolumeId, collection string) bool { + datPath := l.Directory + "/" + volumeName + ".dat" + if fi, err := os.Stat(datPath); err != nil || fi.Size() > int64(super_block.SuperBlockSize) { + return false + } + if !vifIsEcVolume(l.Directory+"/"+volumeName+".vif") && + !(l.IdxDirectory != l.Directory && vifIsEcVolume(l.IdxDirectory+"/"+volumeName+".vif")) { + return false + } + glog.Warningf("removing leftover empty .dat stub for EC volume %d (collection=%q)", vid, collection) + os.Remove(datPath) + os.Remove(l.IdxDirectory + "/" + volumeName + ".idx") + return true +} + +// vifIsEcVolume reports whether the .vif at vifPath records an EC shard config. +func vifIsEcVolume(vifPath string) bool { + vi, _, _, err := volume_info.MaybeLoadVolumeInfo(vifPath) + return err == nil && vi.GetEcShardConfig() != nil +} + func (l *DiskLocation) loadExistingVolume(dirEntry os.DirEntry, needleMapKind NeedleMapKind, skipIfEcVolumesExists bool, ldbTimeout int64, diskId uint32) bool { basename := dirEntry.Name() if dirEntry.IsDir() { @@ -225,6 +256,12 @@ func (l *DiskLocation) loadExistingVolume(dirEntry os.DirEntry, needleMapKind Ne return true } + // Sweep a leftover empty .dat stub (a phantom from the pre-fix loader) + // before it loads as a phantom volume. + if l.removeEmptyEcDatStub(volumeName, vid, collection) { + return false + } + // Load existing data only; never let NewVolume create a phantom .dat. A // lone .vif/.idx (e.g. an EC sidecar whose .ecx is on a sibling disk, // which the same-disk hasEcxFile() guard misses) would otherwise get an diff --git a/weed/storage/store_ec_phantom_dat_test.go b/weed/storage/store_ec_phantom_dat_test.go index fd47e5a64..e653549f1 100644 --- a/weed/storage/store_ec_phantom_dat_test.go +++ b/weed/storage/store_ec_phantom_dat_test.go @@ -9,6 +9,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/stats" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/super_block" "github.com/seaweedfs/seaweedfs/weed/storage/types" "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" "github.com/seaweedfs/seaweedfs/weed/util" @@ -122,3 +123,124 @@ func TestLoneVifDoesNotCreatePhantomDat(t *testing.T) { } } } + +// TestEmptyEcDatStubIsSwept: an empty .dat (<= a superblock, i.e. zero needles) +// for an EC volume is a leftover stub from the pre-fix loader; the loader must +// sweep it on startup, not load it as a phantom empty volume. (On Rust this +// also unblocks the duplicate-vid startup check; Go loads each disk +// independently and does not crash, but the phantom must still go.) +func TestEmptyEcDatStubIsSwept(t *testing.T) { + tempDir := t.TempDir() + dir0 := filepath.Join(tempDir, "data1") + dir1 := filepath.Join(tempDir, "data2") + for _, d := range []string{dir0, dir1} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", d, err) + } + } + + collection := "warp-cal" + vid := needle.VolumeId(41) + // A real (loadable) but empty superblock: without the sweep it loads as a + // phantom empty volume. + stub := (&super_block.SuperBlock{ + Version: needle.Version3, + ReplicaPlacement: &super_block.ReplicaPlacement{}, + Ttl: &needle.TTL{}, + }).Bytes() + for _, d := range []string{dir0, dir1} { + base := erasure_coding.EcShardFileName(collection, d, int(vid)) + if err := os.WriteFile(base+".dat", stub, 0o644); err != nil { + t.Fatalf("write stub .dat: %v", err) + } + if err := volume_info.SaveVolumeInfo(base+".vif", &volume_server_pb.VolumeInfo{ + Version: uint32(needle.Version3), + EcShardConfig: &volume_server_pb.EcShardConfig{DataShards: 10, ParityShards: 4}, + }); err != nil { + t.Fatalf("save .vif: %v", err) + } + } + + diskIOProbeConfig := stats.DefaultDiskIOProbeConfig() + store := NewStore(nil, "localhost", 8080, 18080, "http://localhost:8080", "store-id", + []string{dir0, dir1}, + []int32{100, 100}, + []util.MinFreeSpace{{}, {}}, + "", + NeedleMapInMemory, + []types.DiskType{types.HardDriveType, types.HardDriveType}, + nil, + 3, + diskIOProbeConfig, + ) + done := make(chan struct{}) + go func() { + for { + select { + case <-store.NewVolumesChan: + case <-store.NewEcShardsChan: + case <-store.DeletedVolumesChan: + case <-store.DeletedEcShardsChan: + case <-store.StateUpdateChan: + case <-done: + return + } + } + }() + t.Cleanup(func() { + store.Close() + close(done) + }) + + if store.findVolume(vid) != nil { + t.Errorf("empty EC .dat stub was loaded as a phantom volume %d", vid) + } + for _, d := range []string{dir0, dir1} { + if util.FileExists(erasure_coding.EcShardFileName(collection, d, int(vid)) + ".dat") { + t.Errorf("empty .dat stub on %s was not removed", d) + } + } +} + +// TestRemoveEmptyEcDatStubFindsVifInIdxDir: when -dir.idx is configured the EC +// .vif may live in the idx directory; removeEmptyEcDatStub must look there too, +// not only in the data dir. Calls the helper directly (the dir scan only +// discovers volumes via a .idx/.vif in the data dir, which a separate idx dir +// sidesteps). +func TestRemoveEmptyEcDatStubFindsVifInIdxDir(t *testing.T) { + dataDir := t.TempDir() + idxDir := t.TempDir() + loc := &DiskLocation{ + Directory: dataDir, + DirectoryUuid: "test-uuid", + IdxDirectory: idxDir, + DiskType: types.HddType, + MaxVolumeCount: 100, + OriginalMaxVolumeCount: 100, + MinFreeSpace: util.MinFreeSpace{Type: util.AsPercent, Percent: 1, Raw: "1"}, + } + + const volumeName = "warp-cal_42" + stub := (&super_block.SuperBlock{ + Version: needle.Version3, + ReplicaPlacement: &super_block.ReplicaPlacement{}, + Ttl: &needle.TTL{}, + }).Bytes() + if err := os.WriteFile(dataDir+"/"+volumeName+".dat", stub, 0o644); err != nil { + t.Fatalf("write stub .dat: %v", err) + } + // EC .vif lives in the idx dir, not next to the .dat. + if err := volume_info.SaveVolumeInfo(idxDir+"/"+volumeName+".vif", &volume_server_pb.VolumeInfo{ + Version: uint32(needle.Version3), + EcShardConfig: &volume_server_pb.EcShardConfig{DataShards: 10, ParityShards: 4}, + }); err != nil { + t.Fatalf("save .vif: %v", err) + } + + if !loc.removeEmptyEcDatStub(volumeName, needle.VolumeId(42), "warp-cal") { + t.Fatal("stub with EC .vif in the idx dir should be removed") + } + if util.FileExists(dataDir + "/" + volumeName + ".dat") { + t.Error(".dat stub was not removed") + } +}