fix(volume_server): track actual .ecx dir in cross-disk reconcile

indexEcxOwners scans both IdxDirectory and Directory to find each
volume's .ecx — the second scan covers the legacy case where index
files were written into the data dir before -dir.idx was configured
(removeEcVolumeFiles already accounts for this in disk_location_ec.go).
But the returned map dropped which directory matched, and reconcile
unconditionally passed owner.IdxDirectory to loadEcShardsWithIdxDir.

When the owner's .ecx is in Directory and IdxDirectory != Directory
(server later re-configured with -dir.idx pointing at a fresh path),
NewEcVolume opens IdxDirectory/.ecx → ENOENT, retries the same-disk
fallback at dataBaseFileName+.ecx — but dataBaseFileName uses the
*orphan* disk's data dir, not the owner's, so it ENOENTs again and the
orphan shards stay unloaded.

Track which scan dir matched in indexEcxOwners and pass it through.
Adds TestLoadEcShardsWhenOwnerEcxIsInDataDir as the regression.

Reported in PR #9244 review by @gemini-code-assist and @coderabbitai.
This commit is contained in:
Chris Lu
2026-04-27 12:08:52 -07:00
parent d54e49b14a
commit af57cc6523
2 changed files with 133 additions and 11 deletions

View File

@@ -349,3 +349,109 @@ func TestReconcileNoOpWhenEachDiskIsSelfContained(t *testing.T) {
}
}
// TestLoadEcShardsWhenOwnerEcxIsInDataDir covers the legacy layout flagged
// in PR #9244 review: -dir.idx is configured (so every DiskLocation has
// IdxDirectory != Directory), but the owner's .ecx / .ecj / .vif were
// written into the owner's data dir before -dir.idx was set. indexEcxOwners
// must record the directory the .ecx was actually found in (Directory),
// not just the owner's IdxDirectory — otherwise NewEcVolume's same-disk
// fallback retries the orphan disk's data dir and ENOENTs there too.
func TestLoadEcShardsWhenOwnerEcxIsInDataDir(t *testing.T) {
tempDir := t.TempDir()
dir0 := filepath.Join(tempDir, "data1") // orphan: shards only
dir1 := filepath.Join(tempDir, "data2") // owner: .ecx in data dir
idxDir := filepath.Join(tempDir, "idx") // shared idx folder, intentionally empty
for _, d := range []string{dir0, dir1, idxDir} {
if err := os.MkdirAll(d, 0o755); err != nil {
t.Fatalf("mkdir %s: %v", d, err)
}
}
collection := "grafana-loki"
vid := needle.VolumeId(4242)
const dataShards, parityShards = 10, 4
const datSize int64 = 10 * 1024 * 1024
expectedShardSize := calculateExpectedShardSize(datSize)
writeShard := func(dir string, shardId int) {
t.Helper()
base := erasure_coding.EcShardFileName(collection, dir, int(vid))
f, err := os.Create(base + erasure_coding.ToExt(shardId))
if err != nil {
t.Fatalf("create shard %d in %s: %v", shardId, dir, err)
}
if err := f.Truncate(expectedShardSize); err != nil {
f.Close()
t.Fatalf("truncate shard %d in %s: %v", shardId, dir, err)
}
f.Close()
}
writeShard(dir0, 0)
writeShard(dir0, 12)
writeShard(dir1, 1)
// Owner's .ecx / .ecj / .vif live in dir1 (data dir), NOT idxDir.
// This mirrors a server that ran without -dir.idx, then later got it
// configured — the index files stay in their original on-disk home.
base1Data := erasure_coding.EcShardFileName(collection, dir1, int(vid))
if err := os.WriteFile(base1Data+".ecx", make([]byte, 20), 0o644); err != nil {
t.Fatalf("write .ecx in data dir: %v", err)
}
if err := os.WriteFile(base1Data+".ecj", nil, 0o644); err != nil {
t.Fatalf("write .ecj in data dir: %v", err)
}
if err := volume_info.SaveVolumeInfo(base1Data+".vif", &volume_server_pb.VolumeInfo{
Version: uint32(needle.Version3),
DatFileSize: datSize,
EcShardConfig: &volume_server_pb.EcShardConfig{
DataShards: dataShards,
ParityShards: parityShards,
},
}); err != nil {
t.Fatalf("save .vif in data dir: %v", err)
}
// idxDir is configured but intentionally empty for this volume — we
// want IdxDirectory != Directory while the .ecx lives in Directory.
store := NewStore(nil, "localhost", 8080, 18080, "http://localhost:8080", "store-id",
[]string{dir0, dir1},
[]int32{100, 100},
[]util.MinFreeSpace{{}, {}},
idxDir,
NeedleMapInMemory,
[]types.DiskType{types.HardDriveType, types.HardDriveType},
nil,
3,
)
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)
})
loc1 := store.Locations[1]
if _, found := loc1.FindEcShard(vid, 1); !found {
t.Fatalf("baseline broken: shard 1 on dir1 (which has .ecx in its data dir) was not loaded")
}
loc0 := store.Locations[0]
for _, sid := range []erasure_coding.ShardId{0, 12} {
if _, found := loc0.FindEcShard(vid, sid); !found {
t.Errorf("issue #9212 (PR #9244 review): orphan shard %d on dir0 not loaded; reconcile pointed loader at IdxDirectory but .ecx was actually in owner.Directory", sid)
}
}
}

View File

@@ -19,6 +19,20 @@ type ecKeyForReconcile struct {
vid needle.VolumeId
}
// ecxOwnerInfo records both the disk that owns the .ecx and the actual
// directory it lives in (IdxDirectory or Directory). The directory matters
// because indexEcxOwners scans both — when .ecx lives in Directory (the
// legacy "written before -dir.idx was set" layout that removeEcVolumeFiles
// in disk_location_ec.go also keeps cleaning up), passing the owner's
// IdxDirectory to NewEcVolume would ENOENT both the primary and the
// same-disk fallback path, which uses the orphan disk's data dir, not the
// owner's. Tracking the actual scan dir lets reconcile point loaders at
// the directory the .ecx is really in.
type ecxOwnerInfo struct {
location *DiskLocation
idxDir string
}
// reconcileEcShardsAcrossDisks loads EC shards that the per-disk scan in
// loadAllEcShards skipped because the disk holding the .ec?? files does not
// also hold the matching .ecx / .ecj / .vif index files. The index files
@@ -53,28 +67,30 @@ func (s *Store) reconcileEcShardsAcrossDisks() {
key.vid, key.collection, loc.Directory, shards)
continue
}
if owner == loc {
if owner.location == loc {
// .ecx is on this same disk, but loadAllEcShards still
// did not load these shards — handleFoundEcxFile already
// logged the underlying failure. Don't try again here.
continue
}
glog.V(0).Infof("ec volume %d (collection=%q): loading orphan shards %v on %s using index files from %s (issue #9212)",
key.vid, key.collection, shards, loc.Directory, owner.IdxDirectory)
if err := loc.loadEcShardsWithIdxDir(shards, key.collection, key.vid, owner.IdxDirectory, loc.ecShardNotifyHandler); err != nil {
key.vid, key.collection, shards, loc.Directory, owner.idxDir)
if err := loc.loadEcShardsWithIdxDir(shards, key.collection, key.vid, owner.idxDir, loc.ecShardNotifyHandler); err != nil {
glog.Errorf("ec volume %d on %s: cross-disk shard load failed: %v", key.vid, loc.Directory, err)
}
}
}
}
// indexEcxOwners returns the disk that owns the .ecx file for each
// (collection, vid) on this store. .ecx normally lives in IdxDirectory but
// may have been written into the data directory before -dir.idx was set,
// so we check both. The first owner found wins; duplicates across disks
// are unusual but tolerated.
func (s *Store) indexEcxOwners() map[ecKeyForReconcile]*DiskLocation {
owners := make(map[ecKeyForReconcile]*DiskLocation)
// indexEcxOwners returns the disk and the actual directory that owns the
// .ecx file for each (collection, vid) on this store. .ecx normally lives
// in IdxDirectory but may have been written into the data directory before
// -dir.idx was set, so we check both — and we record which one matched so
// downstream loaders point NewEcVolume at the directory that really has
// the file. The first owner found wins; duplicates across disks are
// unusual but tolerated.
func (s *Store) indexEcxOwners() map[ecKeyForReconcile]ecxOwnerInfo {
owners := make(map[ecKeyForReconcile]ecxOwnerInfo)
for _, loc := range s.Locations {
seen := make(map[string]bool, 2)
for _, scan := range []string{loc.IdxDirectory, loc.Directory} {
@@ -101,7 +117,7 @@ func (s *Store) indexEcxOwners() map[ecKeyForReconcile]*DiskLocation {
}
key := ecKeyForReconcile{collection: collection, vid: vid}
if _, exists := owners[key]; !exists {
owners[key] = loc
owners[key] = ecxOwnerInfo{location: loc, idxDir: scan}
}
}
}