From 98f9e67b4d572a94f9fd088c54958b32ff2c9b6f Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 10 Aug 2026 00:41:33 -0700 Subject: [PATCH] topology: provisional volume update must not erase the reported disk id (#10687) Volume growth registers a provisional record before it can know which directory the server chose, while the server's own report -- pushed during the AllocateVolume RPC -- carries the real disk id. The merge is last-writer-wins, so whichever lands second sticks, and fresh volumes nondeterministically show disk 0 on multi-dir servers. Keep the reported disk id when the provisional update carries none, before the report digest is computed so the stored record stays consistent with what the server keeps reporting. Claude-Session: https://claude.ai/code/session_01QdTEEPbg4MtcoEGwqbgtZC --- weed/topology/disk.go | 8 +++++ .../topology/disk_provisional_disk_id_test.go | 33 +++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 weed/topology/disk_provisional_disk_id_test.go diff --git a/weed/topology/disk.go b/weed/topology/disk.go index 1a952b481..7bffdfcf1 100644 --- a/weed/topology/disk.go +++ b/weed/topology/disk.go @@ -230,6 +230,14 @@ func (d *Disk) doAddOrUpdateVolume(v storage.VolumeInfo, fromReport bool) (isNew d.UpAdjustDiskUsageDelta(types.ToDiskType(v.DiskType), deltaDiskUsage) isNew = true } else { + if !fromReport && v.DiskId == 0 && oldV.DiskId != 0 { + // A provisional (grow-time) record carries no disk id -- the + // master cannot know which directory the server chose. Keep the + // one the server's report already named, before the digest below + // is computed, or the stored record would drift from what the + // server keeps reporting. + v.DiskId = oldV.DiskId + } if oldV.IsRemote() != v.IsRemote() { if v.IsRemote() { deltaDiskUsage.remoteVolumeCount = 1 diff --git a/weed/topology/disk_provisional_disk_id_test.go b/weed/topology/disk_provisional_disk_id_test.go new file mode 100644 index 000000000..b06fd09fd --- /dev/null +++ b/weed/topology/disk_provisional_disk_id_test.go @@ -0,0 +1,33 @@ +package topology + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/storage" + "github.com/seaweedfs/seaweedfs/weed/storage/types" +) + +// A provisional (grow-time) registration carries no disk id; when it lands +// after the server's own report -- the server pushes its report during the +// AllocateVolume RPC, so either order happens -- it must not erase the disk id +// the report recorded. +func TestProvisionalUpdateKeepsReportedDiskId(t *testing.T) { + disk := NewDisk(types.HardDriveType.String()) + + disk.AddOrUpdateVolume(storage.VolumeInfo{Id: 1, DiskId: 2}) + disk.AddProvisionalVolume(storage.VolumeInfo{Id: 1}) + + v, err := disk.GetVolumesById(1) + if err != nil { + t.Fatal(err) + } + if v.DiskId != 2 { + t.Fatalf("DiskId = %d, want 2 (provisional update clobbered the reported value)", v.DiskId) + } + + // A later server report naming a different disk still wins. + disk.AddOrUpdateVolume(storage.VolumeInfo{Id: 1, DiskId: 1}) + if v, _ = disk.GetVolumesById(1); v.DiskId != 1 { + t.Fatalf("DiskId = %d, want 1 (a real report must override)", v.DiskId) + } +}