diff --git a/weed/shell/command_volume_fix_replication.go b/weed/shell/command_volume_fix_replication.go index 73497e530..6476b41d5 100644 --- a/weed/shell/command_volume_fix_replication.go +++ b/weed/shell/command_volume_fix_replication.go @@ -4,7 +4,6 @@ import ( "flag" "fmt" "io" - "path/filepath" "strconv" "strings" "time" @@ -15,6 +14,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/needle_map" "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/util/wildcard" "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" @@ -114,6 +114,12 @@ func (c *commandVolumeFixReplication) Do(args []string, commandEnv *CommandEnv, var underReplicatedVolumeIds, overReplicatedVolumeIds, misplacedVolumeIds []uint32 for vid, replicas := range volumeReplicas { replica := replicas[0] + + // Filter here so the termination counter matches what gets fixed; else -apply loops forever. + if !c.matchCollectionPattern(replica.info.Collection) { + continue + } + replicaPlacement, _ := super_block.NewReplicaPlacementFromByte(byte(replica.info.ReplicaPlacement)) // build locations list for optional verbose output @@ -255,6 +261,18 @@ func checkOneVolume(a *VolumeReplica, b *VolumeReplica, writer io.Writer, comman return } +// matchCollectionPattern reports whether collection matches -collectionPattern: +// empty matches everything, CollectionDefault matches the unnamed collection. +func (c *commandVolumeFixReplication) matchCollectionPattern(collection string) bool { + if *c.collectionPattern == "" { + return true + } + if *c.collectionPattern == CollectionDefault { + return collection == "" + } + return wildcard.MatchesWildcard(*c.collectionPattern, collection) +} + func (c *commandVolumeFixReplication) deleteOneVolume(commandEnv *CommandEnv, writer io.Writer, applyChanges bool, doCheck bool, volumeIds []uint32, volumeReplicas map[uint32][]*VolumeReplica, allLocations []location, selectOneVolumeFn SelectOneVolumeFunc) error { if len(volumeIds) == 0 { // nothing to do @@ -271,23 +289,6 @@ func (c *commandVolumeFixReplication) deleteOneVolume(commandEnv *CommandEnv, wr continue } - // check collection name pattern - if *c.collectionPattern != "" { - var matched bool - if *c.collectionPattern == CollectionDefault { - matched = replica.info.Collection == "" - } else { - var err error - matched, err = filepath.Match(*c.collectionPattern, replica.info.Collection) - if err != nil { - return fmt.Errorf("match pattern %s with collection %s: %v", *c.collectionPattern, replica.info.Collection, err) - } - } - if !matched { - continue - } - } - collectionIsMismatch := false for _, volumeReplica := range replicas { if volumeReplica.info.Collection != replica.info.Collection { @@ -365,30 +366,11 @@ func (c *commandVolumeFixReplication) fixOneUnderReplicatedVolume(commandEnv *Co replica := pickOneReplicaToCopyFrom(replicas) replicaPlacement, _ := super_block.NewReplicaPlacementFromByte(byte(replica.info.ReplicaPlacement)) foundNewLocation := false - hasSkippedCollection := false keepDataNodesSorted(allLocations, types.ToDiskType(replica.info.DiskType)) fn := capacityByFreeVolumeCount(types.ToDiskType(replica.info.DiskType)) for _, dst := range allLocations { // check whether data nodes satisfy the constraints if fn(dst.dataNode) > 0 && satisfyReplicaPlacement(replicaPlacement, replicas, dst) { - // check collection name pattern - if *c.collectionPattern != "" { - var matched bool - if *c.collectionPattern == CollectionDefault { - matched = replica.info.Collection == "" - } else { - var err error - matched, err = filepath.Match(*c.collectionPattern, replica.info.Collection) - if err != nil { - return false, fmt.Errorf("match pattern %s with collection %s: %v", *c.collectionPattern, replica.info.Collection, err) - } - } - if !matched { - hasSkippedCollection = true - break - } - } - // ask the volume server to replicate the volume foundNewLocation = true fmt.Fprintf(writer, "replicating volume %d %s from %s to dataNode %s ...\n", replica.info.Id, replicaPlacement, replica.location.dataNode.Id, dst.dataNode.Id) @@ -414,7 +396,7 @@ func (c *commandVolumeFixReplication) fixOneUnderReplicatedVolume(commandEnv *Co } } - if !foundNewLocation && !hasSkippedCollection { + if !foundNewLocation { fmt.Fprintf(writer, "failed to place volume %d replica as %s, existing:%+v\n", replica.info.Id, replicaPlacement, len(replicas)) } return false, nil diff --git a/weed/shell/command_volume_fix_replication_collection_test.go b/weed/shell/command_volume_fix_replication_collection_test.go new file mode 100644 index 000000000..c304a9e57 --- /dev/null +++ b/weed/shell/command_volume_fix_replication_collection_test.go @@ -0,0 +1,34 @@ +package shell + +import "testing" + +func TestMatchCollectionPattern(t *testing.T) { + tests := []struct { + name string + pattern string + collection string + expected bool + }{ + {name: "empty pattern matches any collection", pattern: "", collection: "jfs-hdfs-test", expected: true}, + {name: "empty pattern matches empty collection", pattern: "", collection: "", expected: true}, + {name: "default pattern matches empty collection", pattern: CollectionDefault, collection: "", expected: true}, + {name: "default pattern rejects named collection", pattern: CollectionDefault, collection: "jfs-hdfs-test", expected: false}, + {name: "exact match", pattern: "smart-highlevel-test", collection: "smart-highlevel-test", expected: true}, + {name: "exact mismatch", pattern: "smart-highlevel-test", collection: "jfs-hdfs-test", expected: false}, + {name: "prefix wildcard match", pattern: "smart*", collection: "smart-highlevel-test", expected: true}, + {name: "prefix wildcard mismatch", pattern: "smart*", collection: "jfs-hdfs-test", expected: false}, + {name: "single char wildcard match", pattern: "vol?", collection: "vol1", expected: true}, + {name: "single char wildcard mismatch", pattern: "vol?", collection: "vol42", expected: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pattern := tt.pattern + c := &commandVolumeFixReplication{collectionPattern: &pattern} + if got := c.matchCollectionPattern(tt.collection); got != tt.expected { + t.Errorf("matchCollectionPattern(pattern=%q, collection=%q) = %v, want %v", + tt.pattern, tt.collection, got, tt.expected) + } + }) + } +}