From 05c6500453a772487c3d0b8e9068258cca31ed4f Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sat, 30 May 2026 23:45:17 -0700 Subject: [PATCH] volume: fix maxVolumeCount dead zone that stalled writes on auto-sized disks (#9755) * volume: don't drop the last writable slot on auto-sized disks MaybeAdjustVolumeMax subtracted 1 from the per-disk slot count, so a disk with room for exactly one volume (free between 1x and 2x the size limit) reported 0 slots. The master then never grew a writable volume and every assign drained its retry budget, so writes failed with context deadline exceeded. Count the full volumes that actually fit, floored at one for an auto-sized disk that has free space. * mini: show disk and volume capacity in the startup banner Print free space, volume size, total volume count and free volume count under the data directory line, so a volume size limit that outstrips the disk is visible at startup instead of surfacing later as failed writes. --- seaweed-volume/src/storage/store.rs | 53 +++++++++++++++- weed/command/mini.go | 43 ++++++++++++- weed/storage/store.go | 12 +++- weed/storage/store_maxvolume_deadzone_test.go | 61 +++++++++++++++++++ 4 files changed, 163 insertions(+), 6 deletions(-) create mode 100644 weed/storage/store_maxvolume_deadzone_test.go diff --git a/seaweed-volume/src/storage/store.rs b/seaweed-volume/src/storage/store.rs index 32433507f..bb815be4c 100644 --- a/seaweed-volume/src/storage/store.rs +++ b/seaweed-volume/src/storage/store.rs @@ -598,8 +598,16 @@ impl Store { as i32; let mut max_count = vol_count + ec_equivalent; - if unclaimed > volume_size_limit as i64 { - max_count += (unclaimed as u64 / volume_size_limit) as i32 - 1; + // One slot per full volume that fits in the unclaimed space. + // A "- 1" here used to zero the count when the disk had room for + // exactly one volume (free between 1x and 2x the limit), stranding + // auto-sized disks at max_volume_count 0 with no writable volume. + if unclaimed > 0 { + max_count += (unclaimed as u64 / volume_size_limit) as i32; + } + // An auto-sized disk with free space always hosts at least one volume. + if max_count < 1 { + max_count = 1; } loc.max_volume_count.store(max_count, Ordering::Relaxed); @@ -1462,6 +1470,47 @@ mod tests { assert!(with_preallocate > without_preallocate); } + #[test] + fn test_maybe_adjust_volume_max_no_dead_zone() { + // With auto max (original_max_volume_count == 0) and a large volume size + // limit, a disk with room for at least one volume must not report 0 + // slots. It once did so for disks sized between 1x and 2x the limit, + // leaving no writable volume and timing out every assign. + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + + let (_, free) = crate::storage::disk_location::get_disk_stats(dir); + if free < 4 { + return; // cannot determine free disk space + } + + let mut store = Store::new(NeedleMapKind::InMemory); + store + .add_location( + dir, + dir, + 0, // auto + DiskType::HardDrive, + MinFreeSpace::Percent(1.0), + Vec::new(), + ) + .unwrap(); + + // free disk is 1.5x the limit: room for one full volume, less than two. + let volume_size_limit = free * 2 / 3; + store + .volume_size_limit + .store(volume_size_limit, Ordering::Relaxed); + + store.maybe_adjust_volume_max(); + + let max = store.locations[0].max_volume_count.load(Ordering::Relaxed); + assert!( + max >= 1, + "auto-sized disk with room for one volume reported max_volume_count={max}; want >= 1" + ); + } + #[test] fn test_find_free_location_predicate_prefers_more_capacity_and_skips_low_disk() { let tmp1 = TempDir::new().unwrap(); diff --git a/weed/command/mini.go b/weed/command/mini.go index 570e0b554..9eb1f6f58 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "encoding/hex" + "encoding/json" "errors" "fmt" "math/bits" @@ -1787,8 +1788,19 @@ func printWelcomeMessage() { fmt.Fprintf(&sb, " Admin UI: http://%s:%d\n", *miniIp, *miniAdminOptions.port) } - fmt.Fprintf(&sb, "\n Data Directory: %s\n\n", *miniDataFolders) - sb.WriteString(" Press Ctrl+C to stop all components") + fmt.Fprintf(&sb, "\n Data Directory: %s\n", *miniDataFolders) + firstDir := util.StringSplit(*miniDataFolders, ",")[0] + if ds := stats_collect.NewDiskStatus(firstDir); ds != nil && ds.All > 0 { + fmt.Fprintf(&sb, " Free Space: %s\n", util.BytesToHumanReadable(ds.Free)) + } + if miniMasterOptions.volumeSizeLimitMB != nil { + fmt.Fprintf(&sb, " Volume Size: %s\n", util.BytesToHumanReadable(uint64(*miniMasterOptions.volumeSizeLimitMB)*bytesPerMB)) + } + if max, free, ok := miniVolumeCounts(); ok { + fmt.Fprintf(&sb, " Volume Count: %d\n", max) + fmt.Fprintf(&sb, " Free Volumes: %d\n", free) + } + sb.WriteString("\n Press Ctrl+C to stop all components") switch { case s3api.HasAnyIdentity(): @@ -1810,6 +1822,33 @@ func printWelcomeMessage() { fmt.Println("") } +// miniVolumeCounts asks the local master for the total volume slots the data +// directory supports (Topology.Max) and how many are still free (Topology.Free). +// Best-effort: any error returns ok=false so the welcome banner simply omits +// the lines. +func miniVolumeCounts() (max, free int64, ok bool) { + url := getHealthCheckAddr(fmt.Sprintf("http://%s:%d/dir/status", *miniIp, *miniMasterOptions.port)) + client := &http.Client{Timeout: 2 * time.Second} + resp, err := client.Get(url) + if err != nil { + return 0, 0, false + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return 0, 0, false + } + var status struct { + Topology struct { + Max int64 + Free int64 + } + } + if err := json.NewDecoder(resp.Body).Decode(&status); err != nil { + return 0, 0, false + } + return status.Topology.Max, status.Topology.Free, true +} + // ensureMiniBuckets creates each named bucket on the embedded filer if it does // not already exist. bucketSpec is a comma-separated list (whitespace around // each name is trimmed); empty entries and an empty spec are no-ops so callers diff --git a/weed/storage/store.go b/weed/storage/store.go index 9eeeb1493..0ec82b81d 100644 --- a/weed/storage/store.go +++ b/weed/storage/store.go @@ -858,8 +858,16 @@ func (s *Store) MaybeAdjustVolumeMax() (hasChanges bool) { volCount := diskLocation.VolumesLen() ecShardCount := diskLocation.EcShardCount() maxVolumeCount := int32(volCount) + int32((ecShardCount+erasure_coding.DataShardsCount-1)/erasure_coding.DataShardsCount) - if unclaimedSpaces > int64(volumeSizeLimit) { - maxVolumeCount += int32(uint64(unclaimedSpaces)/volumeSizeLimit) - 1 + // One slot per full volume that fits in the unclaimed space. + // A "- 1" here used to zero the count when the disk had room for + // exactly one volume (free between 1x and 2x the limit), stranding + // auto-sized disks at maxVolumeCount 0 with no writable volume. + if unclaimedSpaces > 0 { + maxVolumeCount += int32(uint64(unclaimedSpaces) / volumeSizeLimit) + } + // An auto-sized disk with free space always hosts at least one volume. + if maxVolumeCount < 1 { + maxVolumeCount = 1 } newMaxVolumeCount = newMaxVolumeCount + maxVolumeCount atomic.StoreInt32(&diskLocation.MaxVolumeCount, maxVolumeCount) diff --git a/weed/storage/store_maxvolume_deadzone_test.go b/weed/storage/store_maxvolume_deadzone_test.go new file mode 100644 index 000000000..e81939555 --- /dev/null +++ b/weed/storage/store_maxvolume_deadzone_test.go @@ -0,0 +1,61 @@ +package storage + +import ( + "fmt" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/stats" + "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/util" +) + +// With auto max (OriginalMaxVolumeCount == 0) and a large volumeSizeLimit, +// MaybeAdjustVolumeMax must not compute a maxVolumeCount of 0 when the free +// disk holds room for at least one volume. It once did so for disks sized +// between 1x and 2x the limit, leaving no writable volume and timing out every +// assign. An auto-sized disk with space always hosts at least one volume. +func TestMaybeAdjustVolumeMaxNoDeadZone(t *testing.T) { + dir := t.TempDir() + + free := stats.NewDiskStatus(dir).Free + if free < 4 { + t.Skip("cannot determine free disk space") + } + + // Drive the limit so the disk's free space is a known multiple of it, + // then assert the slot count is the honest "how many full volumes fit", + // floored at one. The 1.5x case is the exact regression from #9753. + for _, ratio := range []float64{0.5, 1.5, 2.5, 3.5} { + t.Run(fmt.Sprintf("ratio=%.1f", ratio), func(t *testing.T) { + free := stats.NewDiskStatus(dir).Free + volumeSizeLimit := uint64(float64(free) / ratio) + if volumeSizeLimit == 0 { + t.Skip("free disk too small for this ratio") + } + + loc := NewDiskLocation(dir, 0 /* auto */, util.MinFreeSpace{}, "", types.HddType, nil) + defer loc.Close() + s := &Store{Locations: []*DiskLocation{loc}} + s.SetVolumeSizeLimit(volumeSizeLimit) + + s.MaybeAdjustVolumeMax() + + // Empty temp dir: no volumes, no EC shards, so the count is purely + // the number of full volumes that fit, never below one. + want := int32(free / volumeSizeLimit) + if want < 1 { + want = 1 + } + t.Logf("free=%.1fGiB limit=%.1fGiB ratio≈%.2f -> MaxVolumeCount=%d (want %d)", + float64(free)/(1<<30), float64(volumeSizeLimit)/(1<<30), + float64(free)/float64(volumeSizeLimit), loc.MaxVolumeCount, want) + + if loc.MaxVolumeCount < 1 { + t.Errorf("auto-sized disk with free space reported MaxVolumeCount=%d; want >= 1", loc.MaxVolumeCount) + } + if loc.MaxVolumeCount != want { + t.Errorf("MaxVolumeCount=%d; want %d", loc.MaxVolumeCount, want) + } + }) + } +}