filter by volume ids (#10983)

* filter by volume ids

* master: carry the volume ids VolumeList asks about in one repeated field

One id and a list of them ask the same question, so field 2 holds the list
rather than standing beside a second field that supersedes it.

Claude-Session: https://claude.ai/code/session_011qAmAdhrYvnzGkw7A9N4mP

---------

Co-authored-by: Chris Lu <chris.lu@gmail.com>

seaweed-volume/proto/master.proto

@@ -412,7 +412,7 @@ message VolumeListRequest {
   // under a disk are selected; the topology and its disk counters are always
   // reported in full.
   string collection = 1;
-  uint32 volume_id = 2;
+  repeated uint32 volume_ids = 2;
   // The one collection the empty string cannot name. A named collection wins.
   bool default_collection_only = 3;
   // Empty and zero take everything. Wildcards are supported.

weed/admin/dash/ec_shard_management.go

@@ -561,7 +561,7 @@ func (s *AdminServer) GetEcVolumeDetails(volumeID uint32, sortBy string, sortOrd
 
 	// Get detailed EC shard information for the specific volume via gRPC
 	err := s.WithMasterClient(func(client master_pb.SeaweedClient) error {
-		resp, err := client.VolumeList(context.Background(), &master_pb.VolumeListRequest{VolumeId: volumeID})
+		resp, err := client.VolumeList(context.Background(), &master_pb.VolumeListRequest{VolumeIds: []uint32{volumeID}})
 		if err != nil {
 			return err
 		}

weed/admin/dash/volume_management.go

@@ -324,7 +324,7 @@ func (s *AdminServer) GetVolumeDetails(volumeID uint32, server string) (*VolumeD
 
 	// Find the volume and all its replicas in the cluster
 	err := s.WithMasterClient(func(client master_pb.SeaweedClient) error {
-		resp, err := client.VolumeList(context.Background(), &master_pb.VolumeListRequest{VolumeId: volumeID})
+		resp, err := client.VolumeList(context.Background(), &master_pb.VolumeListRequest{VolumeIds: []uint32{volumeID}})
 		if err != nil {
 			return err
 		}

weed/pb/master.proto

@@ -412,7 +412,7 @@ message VolumeListRequest {
   // under a disk are selected; the topology and its disk counters are always
   // reported in full.
   string collection = 1;
-  uint32 volume_id = 2;
+  repeated uint32 volume_ids = 2;
   // The one collection the empty string cannot name. A named collection wins.
   bool default_collection_only = 3;
   // Empty and zero take everything. Wildcards are supported.

weed/pb/master_pb/master.pb.go

@@ -2785,7 +2785,7 @@ type VolumeListRequest struct {
 	// under a disk are selected; the topology and its disk counters are always
 	// reported in full.
 	Collection string   `protobuf:"bytes,1,opt,name=collection,proto3" json:"collection,omitempty"`
-	VolumeId   uint32 `protobuf:"varint,2,opt,name=volume_id,json=volumeId,proto3" json:"volume_id,omitempty"`
+	VolumeIds  []uint32 `protobuf:"varint,2,rep,packed,name=volume_ids,json=volumeIds,proto3" json:"volume_ids,omitempty"`
 	// The one collection the empty string cannot name. A named collection wins.
 	DefaultCollectionOnly bool `protobuf:"varint,3,opt,name=default_collection_only,json=defaultCollectionOnly,proto3" json:"default_collection_only,omitempty"`
 	// Empty and zero take everything. Wildcards are supported.
@@ -2832,11 +2832,11 @@ func (x *VolumeListRequest) GetCollection() string {
 	return ""
 }
 
-func (x *VolumeListRequest) GetVolumeId() uint32 {
+func (x *VolumeListRequest) GetVolumeIds() []uint32 {
 	if x != nil {
-		return x.VolumeId
+		return x.VolumeIds
 	}
-	return 0
+	return nil
 }
 
 func (x *VolumeListRequest) GetDefaultCollectionOnly() bool {
@@ -5232,12 +5232,13 @@ const file_master_proto_rawDesc = "" +
 	"\tdiskInfos\x18\x03 \x03(\v2&.master_pb.TopologyInfo.DiskInfosEntryR\tdiskInfos\x1aQ\n" +
 	"\x0eDiskInfosEntry\x12\x10\n" +
 	"\x03key\x18\x01 \x01(\tR\x03key\x12)\n" +
-	"\x05value\x18\x02 \x01(\v2\x13.master_pb.DiskInfoR\x05value:\x028\x01\"\xe4\x01\n" +
+	"\x05value\x18\x02 \x01(\v2\x13.master_pb.DiskInfoR\x05value:\x028\x01\"\xe6\x01\n" +
 	"\x11VolumeListRequest\x12\x1e\n" +
 	"\n" +
 	"collection\x18\x01 \x01(\tR\n" +
-	"collection\x12\x1b\n" +
-	"\tvolume_id\x18\x02 \x01(\rR\bvolumeId\x126\n" +
+	"collection\x12\x1d\n" +
+	"\n" +
+	"volume_ids\x18\x02 \x03(\rR\tvolumeIds\x126\n" +
 	"\x17default_collection_only\x18\x03 \x01(\bR\x15defaultCollectionOnly\x12.\n" +
 	"\x13remote_storage_name\x18\x04 \x01(\tR\x11remoteStorageName\x12*\n" +
 	"\x11local_volume_only\x18\x05 \x01(\bR\x0flocalVolumeOnly\"\x83\x01\n" +

weed/topology/volume_filter.go

@@ -12,7 +12,8 @@ import (
 type VolumeFilter struct {
 	Collection        *string
 	remoteStorageName *string
-	VolumeId          *needle.VolumeId
+	// VolumeIds selects the volumes it holds, and an empty one selects them all.
+	VolumeIds map[needle.VolumeId]struct{}
 	// nothing selects the topology alone, for a listing whose volumes travel
 	// in messages of their own.
 	nothing bool
@@ -44,16 +45,18 @@ func NewVolumeFilter(req *master_pb.VolumeListRequest) VolumeFilter {
 		filter.remoteStorageName = new("")
 	}
 
-	if req.VolumeId != 0 {
-		volumeId := needle.VolumeId(req.VolumeId)
-		filter.VolumeId = &volumeId
+	if len(req.VolumeIds) > 0 {
+		filter.VolumeIds = make(map[needle.VolumeId]struct{}, len(req.VolumeIds))
+		for _, volumeId := range req.VolumeIds {
+			filter.VolumeIds[needle.VolumeId(volumeId)] = struct{}{}
+		}
 	}
 	return filter
 }
 
 // SelectsEverything lets a caller size its result for the whole disk up front.
 func (f VolumeFilter) SelectsEverything() bool {
-	return !f.nothing && f.Collection == nil && f.VolumeId == nil && f.remoteStorageName == nil
+	return !f.nothing && f.Collection == nil && len(f.VolumeIds) == 0 && f.remoteStorageName == nil
 }
 
 type volumeLike interface {
@@ -80,8 +83,10 @@ func (f VolumeFilter) matches(vi volumeLike) bool {
 			return false
 		}
 	}
-	if f.VolumeId != nil && *f.VolumeId != vi.GetVolumeId() {
+	if len(f.VolumeIds) > 0 {
+		if _, ok := f.VolumeIds[vi.GetVolumeId()]; !ok {
 			return false
 		}
+	}
 	return true
 }

weed/topology/volume_filter_test.go

@@ -65,7 +65,13 @@ func equalIds(got, want []uint32) bool {
 func TestVolumeFilterSelects(t *testing.T) {
 	topo := filterTestTopology(t)
 	collection := func(name string) *string { return &name }
-	volume := func(id uint32) *needle.VolumeId { v := needle.VolumeId(id); return &v }
+	volumes := func(ids ...needle.VolumeId) map[needle.VolumeId]struct{} {
+		set := make(map[needle.VolumeId]struct{}, len(ids))
+		for _, id := range ids {
+			set[id] = struct{}{}
+		}
+		return set
+	}
 
 	for _, tc := range []struct {
 		name          string
@@ -78,10 +84,10 @@ func TestVolumeFilterSelects(t *testing.T) {
 		// Asking for it must not read as asking for everything.
 		{"the default collection", VolumeFilter{Collection: collection("")}, []uint32{1}, nil},
 		{"a collection nothing is in", VolumeFilter{Collection: collection("none")}, nil, nil},
-		{"one volume", VolumeFilter{VolumeId: volume(3)}, []uint32{3}, nil},
-		{"one ec volume", VolumeFilter{VolumeId: volume(9)}, nil, []uint32{9}},
-		{"both, agreeing", VolumeFilter{Collection: collection("other"), VolumeId: volume(3)}, []uint32{3}, nil},
-		{"both, disagreeing", VolumeFilter{Collection: collection("c"), VolumeId: volume(3)}, nil, nil},
+		{"one volume", VolumeFilter{VolumeIds: volumes(3)}, []uint32{3}, nil},
+		{"one ec volume", VolumeFilter{VolumeIds: volumes(9)}, nil, []uint32{9}},
+		{"both, agreeing", VolumeFilter{Collection: collection("other"), VolumeIds: volumes(3)}, []uint32{3}, nil},
+		{"both, disagreeing", VolumeFilter{Collection: collection("c"), VolumeIds: volumes(3)}, nil, nil},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			volumes, ecVolumes := listed(topo.ToTopologyInfo(tc.filter))
@@ -95,6 +101,43 @@ func TestVolumeFilterSelects(t *testing.T) {
 	}
 }
 
+// an id nothing answers to narrows the listing rather than widening it.
+func TestVolumeFilterVolumeIds(t *testing.T) {
+	topo := filterTestTopology(t)
+
+	for _, tc := range []struct {
+		name          string
+		ids           []needle.VolumeId
+		wantVolumes   []uint32
+		wantEcVolumes []uint32
+	}{
+		// No id asked of the filter, so every volume is listed.
+		{"no id asked", nil, []uint32{1, 2, 3}, []uint32{8, 9}},
+		{"one regular volume asked", []needle.VolumeId{2}, []uint32{2}, nil},
+		{"one ec volume asked", []needle.VolumeId{8}, nil, []uint32{8}},
+		// Both kinds answer to the same set of ids.
+		{"both kinds asked", []needle.VolumeId{1, 8}, []uint32{1}, []uint32{8}},
+		{"every volume asked", []needle.VolumeId{1, 2, 3, 8, 9}, []uint32{1, 2, 3}, []uint32{8, 9}},
+		// An id nothing answers to must not read as asking for everything.
+		{"a missing id", []needle.VolumeId{7}, nil, nil},
+		{"a found and a missing id", []needle.VolumeId{2, 7}, []uint32{2}, nil},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ids := make(map[needle.VolumeId]struct{}, len(tc.ids))
+			for _, id := range tc.ids {
+				ids[id] = struct{}{}
+			}
+			volumes, ecVolumes := listed(topo.ToTopologyInfo(VolumeFilter{VolumeIds: ids}))
+			if !equalIds(volumes, tc.wantVolumes) {
+				t.Errorf("listed volumes %v, want %v", volumes, tc.wantVolumes)
+			}
+			if !equalIds(ecVolumes, tc.wantEcVolumes) {
+				t.Errorf("listed ec volumes %v, want %v", ecVolumes, tc.wantEcVolumes)
+			}
+		})
+	}
+}
+
 func TestVolumeFilterRemoteStorageNameWildcards(t *testing.T) {
 	topo := NewTopology("filter", nil, 32*1024*1024*1024, 5, false)
 	dn := topo.GetOrCreateDataCenter("dc1").GetOrCreateRack("rack1").
@@ -205,13 +248,69 @@ func TestNewVolumeFilterReadsTheRequest(t *testing.T) {
 			}
 		})
 	}
-
-	if f := NewVolumeFilter(&master_pb.VolumeListRequest{}); f.VolumeId != nil {
-		t.Error("a zero volume id must not filter")
 }
-	f := NewVolumeFilter(&master_pb.VolumeListRequest{VolumeId: 7})
-	if f.VolumeId == nil || uint32(*f.VolumeId) != 7 {
-		t.Errorf("volume id not carried across: %v", f.VolumeId)
+
+func TestNewVolumeFilterReadsTheRemoteStorageRequest(t *testing.T) {
+	remoteStorageOf := func(f VolumeFilter) string {
+		if f.remoteStorageName == nil {
+			return "<every>"
+		}
+		return *f.remoteStorageName
+	}
+
+	for _, tc := range []struct {
+		name    string
+		request *master_pb.VolumeListRequest
+		want    string
+	}{
+		// A caller passing through its own "" is answered too much, not wrongly.
+		{"an empty request", &master_pb.VolumeListRequest{}, "<every>"},
+		{"an empty remote storage name", &master_pb.VolumeListRequest{RemoteStorageName: ""}, "<every>"},
+		{"a named remote storage", &master_pb.VolumeListRequest{RemoteStorageName: "s3.backup"}, "s3.backup"},
+		// A wildcard is the caller's pattern, carried across untouched.
+		{"a wildcarded storage", &master_pb.VolumeListRequest{RemoteStorageName: "*"}, "*"},
+		// The one storage (local) the empty string cannot name.
+		{"the local volumes only", &master_pb.VolumeListRequest{LocalVolumeOnly: true}, ""},
+		// Contradictory, so the more specific of the two wins.
+		{"both", &master_pb.VolumeListRequest{RemoteStorageName: "s3.backup", LocalVolumeOnly: true}, "s3.backup"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := remoteStorageOf(NewVolumeFilter(tc.request)); got != tc.want {
+				t.Errorf("selected remote storage %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestNewVolumeFilterReadsTheVolumeIds(t *testing.T) {
+	idsOf := func(f VolumeFilter) []uint32 {
+		ids := make([]uint32, 0, len(f.VolumeIds))
+		for id := range f.VolumeIds {
+			ids = append(ids, uint32(id))
+		}
+		return ids
+	}
+
+	for _, tc := range []struct {
+		name    string
+		request *master_pb.VolumeListRequest
+		want    []uint32
+	}{
+		// No id asked of the request, so every volume is listed.
+		{"an empty request", &master_pb.VolumeListRequest{}, nil},
+		{"one id", &master_pb.VolumeListRequest{VolumeIds: []uint32{7}}, []uint32{7}},
+		{"a list of ids", &master_pb.VolumeListRequest{VolumeIds: []uint32{2, 8}}, []uint32{2, 8}},
+		// Asking twice for a volume selects it once.
+		{"duplicates", &master_pb.VolumeListRequest{VolumeIds: []uint32{2, 2}}, []uint32{2}},
+		// The list is taken literally: no volume answers to zero, so a zero in
+		// it narrows the listing rather than widening it.
+		{"a zero among the ids", &master_pb.VolumeListRequest{VolumeIds: []uint32{0}}, []uint32{0}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := idsOf(NewVolumeFilter(tc.request)); !equalIds(got, tc.want) {
+				t.Errorf("selected volume ids %v, want %v", got, tc.want)
+			}
+		})
 	}
 }