mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-08-18 13:17:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: apply collectionPattern during detection in volume.fix.replication (#10115)

Browse Source 
* fix(shell): correct volume.list -writable filter unit and comparison

* fix(shell): correct volume.list -writable filter unit and comparison

* chore(shell): fix typo in EC shard helper param names

* fix(shell): use exact match for volume.balance -racks/-nodes filter

The old strings.Contains-based filter quietly included any id that was a
  substring of the user-supplied flag value (e.g. -racks=rack10 also matched
  rack1). Replace it with an exact-match set parsed from the comma-separated
  flag value, and add regression tests for both -racks and -nodes paths.

  Also fix a small typo in the "remote storage" error returned by
  maybeMoveOneVolume.

* fix(shell): use exact match for volume.balance -racks/-nodes filter

The old strings.Contains-based filter quietly included any id that was a
  substring of the user-supplied flag value (e.g. -racks=rack10 also matched
  rack1). Replace it with an exact-match set parsed from the comma-separated
  flag value, and add regression tests for both -racks and -nodes paths.

  Also fix a small typo in the "remote storage" error returned by
  maybeMoveOneVolume.

* refactor(shell): drop nil sentinel in splitCSVSet, use len() in callers

* fix: apply collectionPattern during detection in volume.fix.replication

* use existing wildcard.MatchesWildcard for collection matching

It returns a plain bool, so drop the up-front filepath.Match validation
and the path/filepath import that only existed to handle its error.

* trim verbose comments to terse one-liners

* drop redundant per-path collection guards

Detection already filters by replicas[0].info.Collection. The repair guard
re-checked pickOneReplicaToCopyFrom's collection (a different replica), so a
mixed-collection volume could pass detection yet be skipped in repair without
decrementing the counter, spinning the -apply loop. deleteOneVolume keeps its
collectionIsMismatch safety.

---------

Co-authored-by: Chris Lu <chris.lu@gmail.com>
This commit is contained in:
qzhello
2026-06-26 00:48:29 -07:00
committed by GitHub
co-authored by Chris Lu
parent f475d60fcf
commit 378f9a64ff
2 changed files with 54 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/shell/command_volume_fix_replication.go
+20 -38
View File
@@ -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
weed/shell/command_volume_fix_replication_collection_test.go
+34
View File
@@ -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)
}
})
}
}