fix(shell): fs.log.purge purges every filer, not just one (#12524)

* fix(shell): fs.log.purge purges every filer, not just one

Each filer keeps its own metadata logs in its own store, so purging a
single filer leaves the chunks referenced by the others and the volume
vacuum cannot reclaim anything.

The shell picked that single filer at random (commands.go:208) when no
-filer flag was given, and the admin script worker pinned it to
filers[0] (admin_script_handler.go:574), so the same command purged a
different filer on every run and never the whole cluster.

fs.log.purge now resolves the filer list from the master and purges each
of them, continuing when one is unreachable and reporting the failures
at the end. -filer=<addr>,... restricts the purge to a subset.

Also stops mutating the shared command environment per filer, so a
long-lived shell keeps its original filer.

* fix(shell): bound filer discovery and cover the cluster path

Address review feedback:

- resolveLogPurgeFilers waited on the master with context.Background(): a
  shell whose master connection dropped would hang before purging
  anything, where it used to purge the connected filer. Bound the master
  round trip with a timeout and fall back to the connected filer when the
  master cannot be reached or lists no filer.
- the tests only exercised the single-filer fallback. Rename that test to
  what it actually covers and add one asserting every filer of the list is
  attempted and the shared command environment keeps its original filer.

* shell: reject an fs.log.purge -filer list with no addresses

A comma or whitespace only -filer value fell through to master
discovery and purged every filer instead of the intended subset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: fall back to the shell's filer without a second master lookup

When the master listed no filer, FilerAddress() ran filer discovery again
and could wait out the shell's ConnectTimeout before failing. Also cover
master discovery with the fake discovery master.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: test the fs.log.purge filer loop against a fake filer

The previous test looped over the addresses itself, so it passed no
matter what Do did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Martin Morez <martin@morez.org>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-11-10 21:24:04 -08:00
1 parent fb5ffb815f
commit f068fe7f09
2 files changed
+251 -5

No files matched your search

+141
View File
@@ -0,0 +1,141 @@
package shell
import (
"slices"
"strings"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/weed/filer"
"github.com/seaweedfs/seaweedfs/weed/pb"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
)
func TestResolveLogPurgeFilersExplicitList(t *testing.T) {
commandEnv := &CommandEnv{}
filers, err := resolveLogPurgeFilers(" filer-a:8888, filer-b:8888 ,, filer-c:8888 ", commandEnv, false, &strings.Builder{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
want := []pb.ServerAddress{"filer-a:8888", "filer-b:8888", "filer-c:8888"}
if len(filers) != len(want) {
t.Fatalf("got %d filers, want %d: %v", len(filers), len(want), filers)
}
for i, address := range want {
if filers[i] != address {
t.Errorf("filer[%d] = %q, want %q", i, filers[i], address)
}
}
}
func TestResolveLogPurgeFilersRejectsEmptyExplicitList(t *testing.T) {
commandEnv := &CommandEnv{option: &ShellOptions{FilerAddress: "filer-only:8888"}}
for _, filerList := range []string{" ", ",,", " , "} {
if filers, err := resolveLogPurgeFilers(filerList, commandEnv, false, &strings.Builder{}); err == nil {
t.Errorf("-filer=%q: got %v, want an error", filerList, filers)
}
}
}
func TestResolveLogPurgeFilersFallsBackToConnectedFiler(t *testing.T) {
// no explicit list and no master client: fall back to the connected filer
commandEnv := &CommandEnv{option: &ShellOptions{FilerAddress: "filer-only:8888"}}
filers, err := resolveLogPurgeFilers("", commandEnv, false, &strings.Builder{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(filers) != 1 || filers[0] != "filer-only:8888" {
t.Fatalf("got %v, want [filer-only:8888]", filers)
}
}
func TestResolveLogPurgeFilersFromMaster(t *testing.T) {
srv := &fakeDiscoveryServer{}
addr := startFakeDiscoveryMaster(t, srv)
srv.leader = addr
srv.filers.Store(3)
commandEnv, cancel := discoveryEnv(addr, 10*time.Second)
defer cancel()
commandEnv.option.FilerAddress = "filer-connected:8888"
filers, err := resolveLogPurgeFilers("", commandEnv, false, &strings.Builder{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
want := []pb.ServerAddress{"127.0.0.1:8800", "127.0.0.1:8801", "127.0.0.1:8802"}
if !slices.Equal(filers, want) {
t.Fatalf("got %v, want %v", filers, want)
}
}
func TestResolveLogPurgeFilersEmptyMasterSkipsSecondLookup(t *testing.T) {
srv := &fakeDiscoveryServer{}
addr := startFakeDiscoveryMaster(t, srv)
srv.leader = addr
commandEnv, cancel := discoveryEnv(addr, 10*time.Second)
defer cancel()
start := time.Now()
if filers, err := resolveLogPurgeFilers("", commandEnv, false, &strings.Builder{}); err == nil {
t.Fatalf("got %v, want an error", filers)
}
if elapsed := time.Since(start); elapsed > 5*time.Second {
t.Fatalf("resolve took %v, it should not wait out the shell's filer discovery", elapsed)
}
commandEnv.option.FilerAddress = "filer-connected:8888"
filers, err := resolveLogPurgeFilers("", commandEnv, false, &strings.Builder{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !slices.Equal(filers, []pb.ServerAddress{"filer-connected:8888"}) {
t.Fatalf("got %v, want [filer-connected:8888]", filers)
}
}
func TestResolveLogPurgeFilersNoneFound(t *testing.T) {
commandEnv := &CommandEnv{option: &ShellOptions{}}
if _, err := resolveLogPurgeFilers("", commandEnv, false, &strings.Builder{}); err == nil {
t.Fatal("expected an error when no filer can be resolved")
}
}
func TestPurgeFilerLogsKeepsCommandEnvFiler(t *testing.T) {
// the per-filer purge must not mutate the shared command environment,
// otherwise the next filer of the loop would be purged twice and the
// original one never.
commandEnv := &CommandEnv{option: &ShellOptions{FilerAddress: "filer-original:8888"}}
_ = purgeFilerLogs(commandEnv, "filer-other:8888", 0, false, &strings.Builder{})
if got := commandEnv.option.FilerAddress; got != "filer-original:8888" {
t.Fatalf("command env filer changed to %q, want filer-original:8888", got)
}
}
func TestFsLogPurgeContinuesPastUnreachableFiler(t *testing.T) {
stub := &stubFilerServer{entries: map[string]*filer_pb.Entry{
filer.SystemLogDir + "/2020-01-01": {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Mtime: 1}},
filer.SystemLogDir + "/today": {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Mtime: time.Now().Unix()}},
}}
commandEnv := startStubFiler(t, stub)
reachable := commandEnv.option.FilerAddress
err := (&commandFsLogPurge{}).Do([]string{"-daysAgo=1", "-filer=127.0.0.1:1.1," + string(reachable) + ",127.0.0.1:2.2"}, commandEnv, &strings.Builder{})
if err == nil || !strings.Contains(err.Error(), "failed to purge 2 of 3 filers") {
t.Fatalf("got error %v, want both unreachable filers reported", err)
}
if !slices.Equal(stub.deleted, []string{filer.SystemLogDir + "/2020-01-01"}) {
t.Fatalf("reachable filer deleted %v, want only the old log directory", stub.deleted)
}
if commandEnv.option.FilerAddress != reachable {
t.Fatalf("command env filer changed to %q, want %q", commandEnv.option.FilerAddress, reachable)
}
}