shell: say which read-only volumes volume.vacuum leaves alone (#11525)

* shell: say which read-only volumes volume.vacuum leaves alone

volume.vacuum without -volumeId runs the same sweep as the automatic
vacuum, which skips read-only volumes, and the master's response carries
no result. An operator whose disk filled up runs the command, sees it
return, and watches nothing change (issue #11516).

Before issuing the request, list the read-only volumes whose garbage is at
or above the threshold and point at -volumeId, which is the explicit path
PR #9861 opened for them. The help text says the same.

* shell: volume.vacuum hint survives a failed listing and looks at every replica

Review follow-ups: a failed topology listing no longer stops a sweep
without -volumeId, it only drops the hint; a volume counts as read-only
when any replica is, with the garbage ratio taken from the replica that
reports the most, which is what the sweep itself does; a converted index
that reports deletes without sizes is listed rather than hidden; and the
threshold is printed as given instead of rounded to two decimals.

* shell: do not guess a garbage ratio for a converted index

The master cannot compute one for a volume that reports deletes without
their sizes, and a guess of 1 would send the operator to -volumeId for a
volume the server may decline at that threshold. Leave it out and say so.

weed/shell/command_volume_vacuum.go

@@ -28,6 +28,9 @@ func (c *commandVacuum) Help() string {
 
 	volume.vacuum [-garbageThreshold=0.3] [-collection=<collection name>] [-volumeId=<volume id>]
 
+	Without -volumeId this runs the same sweep as the automatic vacuum, which skips
+	read-only volumes. Name a read-only volume with -volumeId to vacuum it anyway.
+
 `
 }
 
@@ -63,15 +66,20 @@ func (c *commandVacuum) Do(args []string, commandEnv *CommandEnv, writer io.Writ
 		volumeIdInts = append(volumeIdInts, 0)
 	}
 
+	topo, _, err := collectTopologyInfo(commandEnv, 0)
+	if err != nil {
+		if *volumeIds != "" {
+			return fmt.Errorf("collect topology: %w", err)
+		}
+		// The hint below is a courtesy; a sweep must not depend on it.
+		fmt.Fprintf(writer, "could not list volumes to check for read-only ones: %v\n", err)
+	}
+
 	// Reject unknown ids up front. The master's VacuumVolume RPC silently
 	// iterates matching volumes, so a typo or an already-deleted volume just
 	// returns success — making this command look like it worked when nothing
 	// happened.
 	if *volumeIds != "" {
-		topo, _, err := collectTopologyInfo(commandEnv, 0)
-		if err != nil {
-			return fmt.Errorf("collect topology: %w", err)
-		}
 		known := make(map[uint32]bool)
 		eachDataNode(topo, func(_ DataCenterId, _ RackId, dn *master_pb.DataNodeInfo) {
 			for _, disk := range dn.DiskInfos {
@@ -96,6 +104,13 @@ func (c *commandVacuum) Do(args []string, commandEnv *CommandEnv, writer io.Writ
 			sort.Slice(missing, func(i, j int) bool { return missing[i] < missing[j] })
 			return fmt.Errorf("volume(s) not found on master: %v", missing)
 		}
+	} else if topo != nil {
+		// The sweep says nothing about the volumes it leaves alone, so an
+		// operator on a full disk sees the command return and nothing change.
+		if skipped := readOnlyVolumesAboveThreshold(topo, *collection, *garbageThreshold); len(skipped) > 0 {
+			fmt.Fprintf(writer, "%d read-only volume(s) hold garbage above %g and are skipped by the sweep: %v\n", len(skipped), *garbageThreshold, skipped)
+			fmt.Fprintf(writer, "vacuum them explicitly with -volumeId\n")
+		}
 	}
 
 	for _, volumeId := range volumeIdInts {
@@ -114,3 +129,42 @@ func (c *commandVacuum) Do(args []string, commandEnv *CommandEnv, writer io.Writ
 
 	return nil
 }
+
+// readOnlyVolumesAboveThreshold lists the volumes a sweep leaves alone: any
+// replica read-only (that is what the sweep checks), in the collection when one
+// is given, and some replica with a garbage ratio at or above the threshold.
+// The ratio uses the sizes the master reports, which is deleted bytes over the
+// .dat size rather than over the content size the volume server divides by, so
+// it can only understate, and a converted index that reports deletes without
+// their sizes is left out because its ratio is not knowable here. This is a
+// hint; the volume server's own check decides.
+func readOnlyVolumesAboveThreshold(topo *master_pb.TopologyInfo, collection string, garbageThreshold float64) []uint32 {
+	readOnly := make(map[uint32]bool)
+	garbage := make(map[uint32]float64) // the highest ratio any replica reports
+	eachDataNode(topo, func(_ DataCenterId, _ RackId, dn *master_pb.DataNodeInfo) {
+		for _, disk := range dn.DiskInfos {
+			for _, v := range disk.VolumeInfos {
+				if collection != "" && v.Collection != collection {
+					continue
+				}
+				if v.ReadOnly {
+					readOnly[v.Id] = true
+				}
+				if v.Size == 0 {
+					continue
+				}
+				if ratio := float64(v.DeletedByteCount) / float64(v.Size); ratio > garbage[v.Id] {
+					garbage[v.Id] = ratio
+				}
+			}
+		}
+	})
+	vids := make([]uint32, 0, len(readOnly))
+	for vid := range readOnly {
+		if garbage[vid] >= garbageThreshold {
+			vids = append(vids, vid)
+		}
+	}
+	sort.Slice(vids, func(i, j int) bool { return vids[i] < vids[j] })
+	return vids
+}

weed/shell/command_volume_vacuum_test.go

@@ -0,0 +1,58 @@
+package shell
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
+)
+
+func vacuumTestTopology(nodes ...*master_pb.DataNodeInfo) *master_pb.TopologyInfo {
+	return &master_pb.TopologyInfo{
+		DataCenterInfos: []*master_pb.DataCenterInfo{{
+			RackInfos: []*master_pb.RackInfo{{DataNodeInfos: nodes}},
+		}},
+	}
+}
+
+func vacuumTestNode(volumes ...*master_pb.VolumeInformationMessage) *master_pb.DataNodeInfo {
+	return &master_pb.DataNodeInfo{
+		DiskInfos: map[string]*master_pb.DiskInfo{"": {VolumeInfos: volumes}},
+	}
+}
+
+func TestReadOnlyVolumesAboveThreshold(t *testing.T) {
+	readOnlyHalfGarbage := &master_pb.VolumeInformationMessage{Id: 1, Collection: "a", Size: 100, DeletedByteCount: 50, ReadOnly: true}
+	writableMostlyGarbage := &master_pb.VolumeInformationMessage{Id: 2, Collection: "a", Size: 100, DeletedByteCount: 90}
+	readOnlyLittleGarbage := &master_pb.VolumeInformationMessage{Id: 3, Collection: "a", Size: 100, DeletedByteCount: 10, ReadOnly: true}
+	readOnlyOtherCollection := &master_pb.VolumeInformationMessage{Id: 4, Collection: "b", Size: 100, DeletedByteCount: 100, ReadOnly: true}
+	readOnlyEmpty := &master_pb.VolumeInformationMessage{Id: 5, Collection: "a", Size: 0, DeletedByteCount: 0, ReadOnly: true}
+	readOnlyAtThreshold := &master_pb.VolumeInformationMessage{Id: 6, Collection: "a", Size: 100, DeletedByteCount: 30, ReadOnly: true}
+	// the sweep skips a volume when any replica is read-only, and the garbage
+	// may sit on the other replica
+	mixedReadOnlyReplica := &master_pb.VolumeInformationMessage{Id: 7, Collection: "a", Size: 100, DeletedByteCount: 5, ReadOnly: true}
+	mixedWritableReplica := &master_pb.VolumeInformationMessage{Id: 7, Collection: "a", Size: 100, DeletedByteCount: 60}
+	// a converted index reports deletes without their sizes; the ratio is not
+	// knowable from here, so it is not listed
+	readOnlyConvertedIndex := &master_pb.VolumeInformationMessage{Id: 8, Collection: "a", Size: 100, DeleteCount: 2, DeletedByteCount: 0, ReadOnly: true}
+
+	topo := vacuumTestTopology(
+		vacuumTestNode(readOnlyHalfGarbage, writableMostlyGarbage, readOnlyLittleGarbage, readOnlyEmpty, readOnlyAtThreshold, mixedReadOnlyReplica, readOnlyConvertedIndex),
+		// the second replica of volume 1 must not list it twice
+		vacuumTestNode(readOnlyHalfGarbage, readOnlyOtherCollection, mixedWritableReplica),
+	)
+
+	got := readOnlyVolumesAboveThreshold(topo, "a", 0.3)
+	if want := []uint32{1, 6, 7}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("collection a, threshold 0.3: got %v, want %v", got, want)
+	}
+
+	got = readOnlyVolumesAboveThreshold(topo, "", 0.3)
+	if want := []uint32{1, 4, 6, 7}; !reflect.DeepEqual(got, want) {
+		t.Fatalf("all collections, threshold 0.3: got %v, want %v", got, want)
+	}
+
+	if got := readOnlyVolumesAboveThreshold(topo, "a", 0.95); len(got) != 0 {
+		t.Fatalf("threshold 0.95: got %v, want none", got)
+	}
+}