diff --git a/seaweed-volume/proto/master.proto b/seaweed-volume/proto/master.proto index 82117df1e..963070cff 100644 --- a/seaweed-volume/proto/master.proto +++ b/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. diff --git a/weed/admin/dash/ec_shard_management.go b/weed/admin/dash/ec_shard_management.go index dab4ad1a2..02862247e 100644 --- a/weed/admin/dash/ec_shard_management.go +++ b/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 } diff --git a/weed/admin/dash/volume_management.go b/weed/admin/dash/volume_management.go index 0a25c220f..b07f7409a 100644 --- a/weed/admin/dash/volume_management.go +++ b/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 } diff --git a/weed/pb/master.proto b/weed/pb/master.proto index 82117df1e..963070cff 100644 --- a/weed/pb/master.proto +++ b/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. diff --git a/weed/pb/master_pb/master.pb.go b/weed/pb/master_pb/master.pb.go index b6cf96c4d..283ebca1c 100644 --- a/weed/pb/master_pb/master.pb.go +++ b/weed/pb/master_pb/master.pb.go @@ -2784,8 +2784,8 @@ type VolumeListRequest struct { // Empty and zero take everything. Only the volumes and ec shards listed // 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"` + Collection string `protobuf:"bytes,1,opt,name=collection,proto3" json:"collection,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" + diff --git a/weed/topology/volume_filter.go b/weed/topology/volume_filter.go index 80137fca8..c7628cd6b 100644 --- a/weed/topology/volume_filter.go +++ b/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() { - return false + if len(f.VolumeIds) > 0 { + if _, ok := f.VolumeIds[vi.GetVolumeId()]; !ok { + return false + } } return true } diff --git a/weed/topology/volume_filter_test.go b/weed/topology/volume_filter_test.go index 801848d20..1c329b77e 100644 --- a/weed/topology/volume_filter_test.go +++ b/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") +func TestNewVolumeFilterReadsTheRemoteStorageRequest(t *testing.T) { + remoteStorageOf := func(f VolumeFilter) string { + if f.remoteStorageName == nil { + return "" + } + return *f.remoteStorageName } - 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) + + 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{}, ""}, + {"an empty remote storage name", &master_pb.VolumeListRequest{RemoteStorageName: ""}, ""}, + {"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) + } + }) } }