fix(storage): keep EC .vif when deleting a coexisting regular volume (#9723)

* fix(storage): keep EC .vif when deleting a coexisting regular volume

A regular volume and an EC volume for the same id share <base>.vif. When
EC shards are distributed onto a server that still holds the regular
volume — the encode source, or any replica the planner targets — the
post-encode VolumeDelete ran removeVolumeFiles and stripped the shared
.vif, leaving the freshly built EC volume without its info file.

Skip the .vif in removeVolumeFiles when an EC volume for the same id
exists on the disk (mounted, or a sealed .ecx on disk). The regular
volume's .dat/.idx still go; the EC sidecars survive.

A two-server end-to-end test encodes a volume whose source and a stub
replica both also receive shards, and asserts the final on-disk layout:
both .dat/.idx gone, each server holding only its assigned shards plus
.ecx/.vif. Storage unit tests cover the with-EC and no-EC cases, and the
Rust seaweed-volume port carries the same guard and tests.

* test(storage): assert .idx is removed in the no-EC destroy case

Strengthen TestDestroyRemovesVifWhenNoEc to confirm the full regular
volume cleanup (.dat, .idx, .vif) when no EC volume coexists.
This commit is contained in:
Chris Lu
2026-05-28 15:39:31 -07:00
committed by GitHub
parent dfd05d14cb
commit 3674f9d04d
6 changed files with 329 additions and 11 deletions
+2 -2
View File
@@ -131,8 +131,8 @@ impl DiskLocation {
volume_id = vid.0,
"volume was not completed: {}, removing files", note
);
remove_volume_files(&volume_name);
remove_volume_files(&idx_name);
remove_volume_files(&volume_name, false);
remove_volume_files(&idx_name, false);
continue;
}
+67 -3
View File
@@ -3157,12 +3157,34 @@ impl Volume {
}
}
// A regular volume and an EC volume for the same id share <base>.vif.
// When EC artefacts coexist on this disk (e.g. shards distributed onto
// a source replica before it is deleted), keep the .vif so removing the
// regular volume does not strip the EC volume's info file.
let keep_vif = self.shares_vif_with_ec_volume();
self.close();
remove_volume_files(&self.data_file_name());
remove_volume_files(&self.index_file_name());
remove_volume_files(&self.data_file_name(), keep_vif);
remove_volume_files(&self.index_file_name(), keep_vif);
Ok(())
}
/// Reports whether an EC volume for this id has a sealed .ecx on the same
/// disk, in which case its .vif is the same file as the regular volume's
/// and must outlive the regular volume's deletion. Mirrors the on-disk
/// portion of Go's Volume.sharesVifWithEcVolume / HasEcxFileOnDisk.
fn shares_vif_with_ec_volume(&self) -> bool {
let has_ecx = |base: &str| -> bool {
fs::metadata(format!("{}.ecx", base))
.map(|m| !m.is_dir() && m.len() > 0)
.unwrap_or(false)
};
if has_ecx(&volume_file_name(&self.dir_idx, &self.collection, self.id)) {
return true;
}
self.dir != self.dir_idx
&& has_ecx(&volume_file_name(&self.dir, &self.collection, self.id))
}
/// Check if an I/O error is EIO (errno 5) and record it for health monitoring.
/// On success (None), clears any previously recorded EIO error.
/// Matches Go's `checkReadWriteError` in volume_write.go.
@@ -3231,10 +3253,13 @@ fn get_append_at_ns(last: u64) -> u64 {
/// Remove all files associated with a volume.
/// .dat/.idx removals log at info level so destructive calls are traceable.
pub(crate) fn remove_volume_files(base: &str) {
pub(crate) fn remove_volume_files(base: &str, keep_vif: bool) {
for ext in &[
".dat", ".idx", ".vif", ".sdx", ".cpd", ".cpx", ".note", ".rdb",
] {
if *ext == ".vif" && keep_vif {
continue;
}
let path = format!("{}{}", base, ext);
let size = fs::metadata(&path).map(|m| m.len()).unwrap_or(0);
let existed = fs::remove_file(&path).is_ok();
@@ -4740,4 +4765,43 @@ mod tests {
".vif removed from data dir"
);
}
/// When an EC volume for the same id has a sealed .ecx on the same disk, the
/// .vif is shared with it and must survive the regular volume's deletion.
#[test]
fn test_destroy_keeps_vif_when_ec_coexists() {
let tmp = TempDir::new().unwrap();
let dir = tmp.path().to_str().unwrap();
let mut v = make_test_volume(dir);
let mut n = Needle {
id: NeedleId(1),
cookie: Cookie(1),
data: b"test".to_vec(),
data_size: 4,
..Needle::default()
};
v.write_needle(&mut n, true).unwrap();
let vif_path = format!("{}/1.vif", dir);
std::fs::write(&vif_path, r#"{"version":3}"#).unwrap();
// A sealed .ecx marks a coexisting EC volume for the same id.
let ecx_path = format!("{}/1.ecx", dir);
std::fs::write(&ecx_path, b"ec-index").unwrap();
v.destroy(false, false).unwrap();
let dat_path = format!("{}/1.dat", dir);
let idx_path = format!("{}/1.idx", dir);
assert!(!std::path::Path::new(&dat_path).exists(), ".dat removed");
assert!(!std::path::Path::new(&idx_path).exists(), ".idx removed");
assert!(
std::path::Path::new(&vif_path).exists(),
".vif kept: shared with the coexisting EC volume"
);
assert!(
std::path::Path::new(&ecx_path).exists(),
".ecx is an EC sidecar, never touched here"
);
}
}
+2 -2
View File
@@ -209,8 +209,8 @@ func (l *DiskLocation) loadExistingVolume(dirEntry os.DirEntry, needleMapKind Ne
if util.FileExists(noteFile) {
note, _ := os.ReadFile(noteFile)
glog.Warningf("volume %s was not completed: %s", volumeName, string(note))
removeVolumeFiles(l.Directory + "/" + volumeName)
removeVolumeFiles(l.IdxDirectory + "/" + volumeName)
removeVolumeFiles(l.Directory+"/"+volumeName, false)
removeVolumeFiles(l.IdxDirectory+"/"+volumeName, false)
return false
}
@@ -0,0 +1,75 @@
package storage
import (
"os"
"testing"
"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/util"
"github.com/stretchr/testify/require"
)
// A regular volume and an EC volume for the same id share <base>.vif. Deleting
// the regular volume must drop its .dat/.idx but keep the .vif so the
// coexisting EC volume's info file survives. This is the same-disk case that
// arises when EC shards are distributed onto a source/replica server before
// the original volume is deleted.
func TestDestroyKeepsVifWhenEcCoexists(t *testing.T) {
dir := t.TempDir()
v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
require.NoError(t, err)
v.location = newTestDiskLocation(dir)
_, _, _, err = v.writeNeedle2(newRandomNeedle(1), true, false)
require.NoError(t, err)
base := VolumeFileName(dir, "", 1)
vifPath := base + ".vif"
require.NoError(t, os.WriteFile(vifPath, []byte("ec-volume-info"), 0o644))
// An on-disk .ecx marks a coexisting EC volume for the same id.
ecxPath := erasure_coding.EcShardFileName("", dir, 1) + ".ecx"
require.NoError(t, os.WriteFile(ecxPath, []byte("ec-index"), 0o644))
require.NoError(t, v.Destroy(false, false))
assertFileExist(t, false, base+".dat")
assertFileExist(t, false, base+".idx")
assertFileExist(t, true, vifPath) // shared with the EC volume, must survive
assertFileExist(t, true, ecxPath) // EC sidecars are never touched here
}
// With no coexisting EC volume the .vif is a plain regular-volume file and is
// removed with the rest.
func TestDestroyRemovesVifWhenNoEc(t *testing.T) {
dir := t.TempDir()
v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
require.NoError(t, err)
v.location = newTestDiskLocation(dir)
_, _, _, err = v.writeNeedle2(newRandomNeedle(1), true, false)
require.NoError(t, err)
base := VolumeFileName(dir, "", 1)
vifPath := base + ".vif"
require.NoError(t, os.WriteFile(vifPath, []byte("regular-volume-info"), 0o644))
require.NoError(t, v.Destroy(false, false))
assertFileExist(t, false, base+".dat")
assertFileExist(t, false, base+".idx")
assertFileExist(t, false, vifPath) // removed along with the regular volume
}
func newTestDiskLocation(dir string) *DiskLocation {
loc := &DiskLocation{
Directory: dir,
IdxDirectory: dir,
DiskType: types.HddType,
MaxVolumeCount: 100,
MinFreeSpace: util.MinFreeSpace{Type: util.AsPercent, Percent: 1, Raw: "1"},
}
loc.volumes = make(map[needle.VolumeId]*Volume)
loc.ecVolumes = make(map[needle.VolumeId]*erasure_coding.EcVolume)
return loc
}
+24 -4
View File
@@ -94,13 +94,31 @@ func (v *Volume) Destroy(onlyEmpty bool, keepRemoteData bool) (err error) {
}
}
}
// A regular volume and an EC volume for the same id share <base>.vif. When
// EC artefacts coexist on this disk (e.g. shards distributed onto a source
// replica before it is deleted), keep the .vif so removing the regular
// volume does not strip the EC volume's info file.
keepVif := v.sharesVifWithEcVolume()
v.doClose()
removeVolumeFiles(v.DataFileName())
removeVolumeFiles(v.IndexFileName())
removeVolumeFiles(v.DataFileName(), keepVif)
removeVolumeFiles(v.IndexFileName(), keepVif)
return
}
func removeVolumeFiles(filename string) {
// sharesVifWithEcVolume reports whether an EC volume for this volume id lives
// on the same disk, in which case its .vif is the same file as the regular
// volume's and must outlive the regular volume's deletion.
func (v *Volume) sharesVifWithEcVolume() bool {
if v.location == nil {
return false
}
if _, found := v.location.FindEcVolume(v.Id); found {
return true
}
return v.location.HasEcxFileOnDisk(v.Collection, v.Id)
}
func removeVolumeFiles(filename string, keepVif bool) {
// .dat/.idx removals log at V(0) so destructive calls are traceable.
deleteAndLog := func(ext string) {
fullFilename := filename + "." + ext
@@ -116,7 +134,9 @@ func removeVolumeFiles(filename string) {
}
deleteAndLog("dat")
deleteAndLog("idx")
deleteAndLog("vif")
if !keepVif {
deleteAndLog("vif")
}
// sorted index file
deleteAndLog("sdx")
// compaction
@@ -0,0 +1,159 @@
package erasure_coding
import (
"context"
"fmt"
"net"
"net/http"
"os"
"path/filepath"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/test/volume_server/framework"
"github.com/seaweedfs/seaweedfs/test/volume_server/matrix"
"github.com/seaweedfs/seaweedfs/weed/pb/worker_pb"
"github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
)
// End-to-end on two servers: a volume with a real replica on server A and a
// 0-byte stub replica of the same id on server B (an interrupted-encode
// leftover). After a full EC encode the cluster must end in exactly one valid
// layout — the complete shard set split across A and B, each with .ecx/.vif —
// and every wrong file must be gone: both regular .dat files (source deleted
// after verify, stub swept before distribute), no shard on the wrong server,
// and B's shared <collection>_<vid>.vif intact rather than clobbered by the
// stub's delete.
func TestEcEncodeLeavesRightFilesAndRemovesStubAndSource(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
cluster := framework.StartMultiVolumeCluster(t, matrix.P1(), 2)
dialOption := grpc.WithTransportCredentials(insecure.NewCredentials())
const (
volumeID = uint32(9490)
collection = "ec-e2e"
)
addrA := serverAddress(cluster, 0)
addrB := serverAddress(cluster, 1)
connA, clientA := framework.DialVolumeServer(t, cluster.VolumeGRPCAddress(0))
defer connA.Close()
connB, clientB := framework.DialVolumeServer(t, cluster.VolumeGRPCAddress(1))
defer connB.Close()
// Server A: real source replica with data.
framework.AllocateVolume(t, clientA, volumeID, collection)
httpClient := framework.NewHTTPClient()
for i := 0; i < 8; i++ {
fid := framework.NewFileID(volumeID, uint64(948000+i), uint32(0x9490CA00+i))
payload := make([]byte, 4096)
for j := range payload {
payload[j] = byte(i + 1)
}
resp := framework.UploadBytes(t, httpClient, cluster.VolumeAdminURL(0), fid, payload)
_ = framework.ReadAllAndClose(t, resp)
require.Equal(t, http.StatusCreated, resp.StatusCode)
}
// Server B: empty stub replica of the same volume id.
framework.AllocateVolume(t, clientB, volumeID, collection)
dataShards := int(erasure_coding.DataShardsCount)
totalShards := int(erasure_coding.DataShardsCount + erasure_coding.ParityShardsCount)
aShards := shardRange(0, dataShards) // 0..DataShardsCount-1 on A
bShards := shardRange(dataShards, totalShards) // parity range on B
task := NewErasureCodingTask("ec-e2e", addrA, volumeID, collection, dialOption)
params := &worker_pb.TaskParams{
VolumeId: volumeID,
Collection: collection,
Sources: []*worker_pb.TaskSource{
{Node: addrA, VolumeId: volumeID},
{Node: addrB, VolumeId: volumeID},
},
Targets: []*worker_pb.TaskTarget{
{Node: addrA, ShardIds: aShards},
{Node: addrB, ShardIds: bShards},
},
TaskParams: &worker_pb.TaskParams_ErasureCodingParams{
ErasureCodingParams: &worker_pb.ErasureCodingTaskParams{
DataShards: erasure_coding.DataShardsCount,
ParityShards: erasure_coding.ParityShardsCount,
WorkingDir: t.TempDir(),
},
},
}
ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
defer cancel()
require.NoError(t, task.Execute(ctx, params))
dirA := filepath.Join(cluster.BaseDir(), "volume0")
dirB := filepath.Join(cluster.BaseDir(), "volume1")
base := fmt.Sprintf("%s_%d", collection, volumeID)
// Both original regular volumes are gone: A's source deleted after verify,
// B's stub swept before distribute.
requireAbsent(t, dirA, base+".dat")
requireAbsent(t, dirA, base+".idx")
requireAbsent(t, dirB, base+".dat")
requireAbsent(t, dirB, base+".idx")
// Each server holds exactly its assigned shards, plus index/info sidecars.
for _, id := range aShards {
requirePresent(t, dirA, fmt.Sprintf("%s.ec%02d", base, id))
}
for _, id := range bShards {
requireAbsent(t, dirA, fmt.Sprintf("%s.ec%02d", base, id))
}
requirePresent(t, dirA, base+".ecx")
requirePresent(t, dirA, base+".vif")
for _, id := range bShards {
requirePresent(t, dirB, fmt.Sprintf("%s.ec%02d", base, id))
}
for _, id := range aShards {
requireAbsent(t, dirB, fmt.Sprintf("%s.ec%02d", base, id))
}
requirePresent(t, dirB, base+".ecx")
// The shared .vif must survive on B: the stub was deleted before the EC
// files landed, so deleteOriginalVolume never ran removeVolumeFiles there.
requirePresent(t, dirB, base+".vif")
}
// serverAddress builds the SeaweedFS ip:httpPort.grpcPort address the worker's
// gRPC client decodes, from the multi-cluster's separate admin and grpc ports.
func serverAddress(c *framework.MultiVolumeCluster, index int) string {
_, grpcPort, err := net.SplitHostPort(c.VolumeGRPCAddress(index))
if err != nil {
panic(err)
}
return c.VolumeAdminAddress(index) + "." + grpcPort
}
func shardRange(start, end int) []uint32 {
ids := make([]uint32, 0, end-start)
for i := start; i < end; i++ {
ids = append(ids, uint32(i))
}
return ids
}
func requirePresent(t *testing.T, dir, name string) {
t.Helper()
_, err := os.Stat(filepath.Join(dir, name))
require.NoError(t, err, "expected %s to be present in %s", name, dir)
}
func requireAbsent(t *testing.T, dir, name string) {
t.Helper()
_, err := os.Stat(filepath.Join(dir, name))
require.True(t, os.IsNotExist(err), "expected %s to be absent in %s, stat err=%v", name, dir, err)
}