fix(storage): refuse to load .vif-only entry as regular volume when .ecx exists (#9448) (#9461)

fix(storage): refuse to load .vif-only entry as regular volume when .ecx exists

Defensive root-cause fix for issue #9448. The detection-side guard in
the EC plugin worker already breaks the infinite re-encode loop, but
the underlying volume-server state — `.vif` preserved next to EC shards
when the source replica is destroyed — can still re-arm a phantom
regular volume via the MountVolume / LoadVolume path.

In loadExistingVolume, the existing `.ecx`-present check is gated on
the caller passing `skipIfEcVolumesExists=true`. The startup-scan path
(concurrentLoadingVolumes) does pass that flag and correctly skips
the `.vif`. The LoadVolume → loadExistingVolume(…, false, …) path
used by VolumeMount does NOT, so it falls through to NewVolume, which
calls v.load with createDatIfMissing=true and creates a phantom
empty `.dat`. The master then reports the volume as regular and EC
detection re-proposes it.

Hoist the `.ecx`-present check so it runs unconditionally for `.vif`
entries: if the EC index is on the disk, the `.vif` belongs to those
EC shards, never to a regular volume that should be resurrected.
Pure OSS clusters never reach this exact state today (OSS deletes
`.vif` unconditionally during Destroy), but the guard hardens the
load path against any future path that leaves the same state.

Test:
- TestLoadExistingVolumeSkipsVifWhenEcxPresent builds the exact
  post-#9448 disk layout (`.vif` + `.ecx`, no `.dat`) and asserts
  loadExistingVolume(skipIfEcVolumesExists=false) returns false,
  does not create a placeholder `.dat`, and does not register a
  phantom volume in l.volumes.
This commit is contained in:
Chris Lu
2026-05-12 09:30:42 -07:00
committed by GitHub
parent d221a64262
commit 18677a8430
2 changed files with 96 additions and 6 deletions
+21 -6
View File
@@ -152,6 +152,19 @@ func getValidVolumeName(basename string) string {
return ""
}
// hasEcxFile reports whether an .ecx for volumeName exists on this disk.
// Checks IdxDirectory first, then falls back to Directory (the .ecx may
// have been created before -dir.idx was configured).
func (l *DiskLocation) hasEcxFile(volumeName string) bool {
if util.FileExists(filepath.Join(l.IdxDirectory, volumeName+".ecx")) {
return true
}
if l.IdxDirectory != l.Directory {
return util.FileExists(filepath.Join(l.Directory, volumeName+".ecx"))
}
return false
}
func (l *DiskLocation) loadExistingVolume(dirEntry os.DirEntry, needleMapKind NeedleMapKind, skipIfEcVolumesExists bool, ldbTimeout int64, diskId uint32) bool {
basename := dirEntry.Name()
if dirEntry.IsDir() {
@@ -169,14 +182,16 @@ func (l *DiskLocation) loadExistingVolume(dirEntry os.DirEntry, needleMapKind Ne
return false
}
// .vif next to .ecx is EC shard metadata, not a regular volume.
// Without this guard NewVolume below would create a phantom empty .dat.
if strings.HasSuffix(basename, ".vif") && l.hasEcxFile(volumeName) {
glog.V(1).Infof("loadExistingVolume: skipping .vif-only entry for volume %d (collection=%q); .ecx present", vid, collection)
return false
}
// skip if ec volumes exists, but validate EC files first
if skipIfEcVolumesExists {
ecxFilePath := filepath.Join(l.IdxDirectory, volumeName+".ecx")
if !util.FileExists(ecxFilePath) && l.IdxDirectory != l.Directory {
// .ecx may have been created before -dir.idx was configured
ecxFilePath = filepath.Join(l.Directory, volumeName+".ecx")
}
if util.FileExists(ecxFilePath) {
if l.hasEcxFile(volumeName) {
// Validate EC volume: shard count, size consistency, and expected size vs .dat file
if !l.validateEcVolume(collection, vid) {
glog.Warningf("EC volume %d validation failed, removing incomplete EC files to allow .dat file loading", vid)
+75
View File
@@ -661,3 +661,78 @@ func TestDistributedEcVolumeNoFileDeletion(t *testing.T) {
t.Logf("SUCCESS: Distributed EC volume files preserved (not deleted)")
}
// TestLoadExistingVolumeSkipsVifWhenEcxPresent pins the skip behavior on
// the LoadVolume / MountVolume path (skipIfEcVolumesExists=false) for the
// .vif + .ecx disk layout without .dat. Two variants cover both
// IdxDirectory==Directory and the split-idx-dir fallback.
func TestLoadExistingVolumeSkipsVifWhenEcxPresent(t *testing.T) {
const vid needle.VolumeId = 42
cases := []struct {
name string
splitDirs bool
}{
{name: "same-idx-dir", splitDirs: false},
{name: "split-idx-dir", splitDirs: true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
dataDir := t.TempDir()
idxDir := dataDir
if tc.splitDirs {
idxDir = t.TempDir()
}
minFreeSpace := util.MinFreeSpace{Type: util.AsPercent, Percent: 1, Raw: "1"}
diskLocation := &DiskLocation{
Directory: dataDir,
DirectoryUuid: "test-uuid",
IdxDirectory: idxDir,
DiskType: types.HddType,
MaxVolumeCount: 100,
OriginalMaxVolumeCount: 100,
MinFreeSpace: minFreeSpace,
}
diskLocation.volumes = make(map[needle.VolumeId]*Volume)
diskLocation.ecVolumes = make(map[needle.VolumeId]*erasure_coding.EcVolume)
vifPath := erasure_coding.EcShardFileName("", dataDir, int(vid)) + ".vif"
ecxPath := erasure_coding.EcShardFileName("", idxDir, int(vid)) + ".ecx"
if err := os.WriteFile(vifPath, []byte{}, 0644); err != nil {
t.Fatalf("write .vif: %v", err)
}
if err := os.WriteFile(ecxPath, []byte{}, 0644); err != nil {
t.Fatalf("write .ecx: %v", err)
}
entries, err := os.ReadDir(dataDir)
if err != nil {
t.Fatalf("read dir: %v", err)
}
var vifEntry os.DirEntry
for _, e := range entries {
if filepath.Ext(e.Name()) == ".vif" {
vifEntry = e
break
}
}
if vifEntry == nil {
t.Fatalf(".vif entry missing from dir listing")
}
loaded := diskLocation.loadExistingVolume(vifEntry, NeedleMapInMemory, false, 0, 0)
if loaded {
t.Fatalf("loadExistingVolume should refuse to load a .vif-only entry when .ecx is present (volume %d)", vid)
}
if _, exists := diskLocation.volumes[vid]; exists {
t.Fatalf("volume %d should not be registered in l.volumes (would create phantom regular volume)", vid)
}
datPath := erasure_coding.EcShardFileName("", dataDir, int(vid)) + ".dat"
if util.FileExists(datPath) {
t.Fatalf("guard must not create a placeholder .dat for volume %d", vid)
}
})
}
}