fix(fsck): inspect references to deleted volumes (#12357)

* fix(fsck): inspect references to deleted volumes * volume.fsck: treat every match-all collection filter as global ALL_COLLECTIONS and EACH_COLLECTION compile to the same match-all matcher as an empty filter or "*", but the string check treated them as a restricted scope: a selected missing volume was rejected and references to master-absent volumes were reported without being purged. Use the compiled matcher instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: purge an entry whose manifest is on a missing volume A top-level chunk on a missing volume already proves the entry is unreadable, whether or not its manifests resolve. Skipping such entries kept the very case where the manifest needle sat on the retired volume, and the remaining unresolved manifest then blocked every later orphan purge. The missing-needle check on live volumes never had this gate, so both paths now treat these entries the same. The warning now says only orphan purging is disabled, which is the only thing the counter gates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: report a failed Filer delete A rejected purge DELETE printed nothing unless -v was set, so the run looked successful after "deleting path" while the entry stayed. The filer answers 204 for an entry that is already gone, so only real failures print. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: tighten help text, comments and missing-volume tests The help now wraps like the rest of it and says that a purge removes the whole entry, including its chunks on other volumes. The live test header no longer calls the run report-only, and the reverse-scan test checks the report line for each missing chunk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: drop the missing-volume design notes The notes said no destructive change was approved while the change adds a confirmed purge, still described the dropped manifest gate, and mostly recorded local runs and a reverse index that is not part of this change. The operator contract lives in help volume.fsck. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: allow a report-only missing-volume scan with -collection The collection check exists to stop a purge that cannot be scoped, but it also rejected a report-only run that deletes nothing. Apply it only with -reallyDeleteFilerEntries, and limit the -confirmMissingVolumeDeletion description to the -volumeId case it guards. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: count only a 2xx answer as a purged entry A redirect the client could not follow left the entry in place but was recorded as deleted, which also queued its parent for empty-directory cleanup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * volume.fsck: say that a filtered purge of a missing -volumeId is refused The help and the early warning both said a purge with -collection only reports references to unlisted volumes, but a -volumeId naming one is rejected before the scan. The help now says so, and the warning prints after that check, so a refused run no longer claims it will report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: MZ01-CHOIMINSU <mz01-juwannam@MZ01-CHOIMINSU.local> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-04 02:57:44 +01:00 · 2026-11-03 07:57:43 -08:00
1 parent e8b284fef5
commit 76198d15cf
@@ -0,0 +1,184 @@
+package shell
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
+	"github.com/seaweedfs/seaweedfs/weed/storage/types"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+	"github.com/seaweedfs/seaweedfs/weed/util/wildcard"
+)
+
+// A selected volume can already be absent from the master when fsck is used to
+// clean stale Filer references after physical volume deletion. Only that
+// explicitly selected missing ID should enter the purge queue.
+func TestVolumeFsckFindMissingFilerEntriesForDeletedSelectedVolume(t *testing.T) {
+	tempDir := t.TempDir()
+	chunk := func(volumeID uint32, fileKey uint64) *filer_pb.FileChunk {
+		return &filer_pb.FileChunk{
+			Fid:  &filer_pb.FileId{VolumeId: volumeID, FileKey: fileKey, Cookie: 1},
+			Size: 1,
+		}
+	}
+	stub := &stubFilerServer{entries: map[string]*filer_pb.Entry{
+		"/":            {IsDirectory: true},
+		"/archive":     {IsDirectory: true},
+		"/archive/old": {Chunks: []*filer_pb.FileChunk{chunk(99, 1), chunk(99, 4), chunk(100, 2), chunk(7, 5)}},
+		"/live":        {Chunks: []*filer_pb.FileChunk{chunk(7, 3)}},
+	}}
+
+	verbose := false
+	findMissing := true
+	var output bytes.Buffer
+	c := &commandVolumeFsck{
+		tempFolder:               tempDir,
+		verbose:                  &verbose,
+		findMissingChunksInFiler: &findMissing,
+		writer:                   &output,
+		env:                      startStubFiler(t, stub),
+		volumeIds:                map[uint32]bool{99: true},
+		knownVolumeIds:           map[uint32]bool{7: true},
+	}
+	currentVolumes := map[string]map[uint32]VInfo{"dn1": {7: {collection: "live"}}}
+	var purged []purgeItem
+	if err := c.collectFilerFileIdAndPaths(currentVolumes, true, func(item purgeItem) {
+		purged = append(purged, item)
+	}, 0, 0); err != nil {
+		t.Fatalf("collect filer references: %v", err)
+	}
+
+	if len(purged) != 1 || purged[0].path != util.FullPath("/archive/old") {
+		t.Fatalf("purged paths = %v, want only /archive/old for selected missing volume 99", purged)
+	}
+	// The same logical file also references a live volume. The scan preserves
+	// that reference while queuing the path only once for the missing chunks.
+	foundSurvivingChunk := false
+	if err := c.readFilerFileIdFile(7, func(_ types.NeedleId, itemPath util.FullPath, _ []byte) {
+		if itemPath == util.FullPath("/archive/old") {
+			foundSurvivingChunk = true
+		}
+	}); err != nil {
+		t.Fatalf("read surviving volume references: %v", err)
+	}
+	if !foundSurvivingChunk {
+		t.Fatal("the surviving chunk reference was not recorded for volume 7")
+	}
+	for _, line := range []string{
+		"99,100000001 /archive/old volume not found\n",
+		"99,400000001 /archive/old volume not found\n",
+	} {
+		if !strings.Contains(output.String(), line) {
+			t.Errorf("missing-volume report %q lacks %q", output.String(), line)
+		}
+	}
+	if strings.Contains(output.String(), "100,200000001") {
+		t.Errorf("missing-volume report %q includes unselected volume 100", output.String())
+	}
+}
+
+// Unknown IDs remain errors outside the reverse Filer scan, preserving typo detection.
+func TestVolumeFsckSelectedVolumeIDValidation(t *testing.T) {
+	known := map[uint32]bool{7: true}
+	requested := map[uint32]bool{99: true, 100: true}
+
+	if _, err := validateSelectedVolumeIDs(requested, known, false); err == nil {
+		t.Fatal("normal fsck accepted volume IDs not present on the master")
+	}
+	missing, err := validateSelectedVolumeIDs(requested, known, true)
+	if err != nil {
+		t.Fatalf("reverse Filer scan rejected missing volume IDs: %v", err)
+	}
+	if len(missing) != 2 || missing[0] != 99 || missing[1] != 100 {
+		t.Fatalf("missing IDs = %v, want sorted [99 100]", missing)
+	}
+}
+
+// A missing volume has no trustworthy collection metadata; a collection
+// restriction must fail closed instead of silently widening or narrowing scope.
+func TestValidateMissingVolumeCollectionFilter(t *testing.T) {
+	missing := []uint32{12}
+	compile := func(filter string) *wildcard.CollectionMatcher {
+		matcher, err := wildcard.CompileCollectionMatcher(filter)
+		if err != nil {
+			t.Fatalf("compile %q: %v", filter, err)
+		}
+		return matcher
+	}
+	for _, filter := range []string{"", "*", "ALL_COLLECTIONS", "EACH_COLLECTION"} {
+		if err := validateMissingVolumeCollectionFilter(missing, compile(filter)); err != nil {
+			t.Errorf("filter %q: %v", filter, err)
+		}
+	}
+	if err := validateMissingVolumeCollectionFilter(missing, compile("photos")); err == nil || !strings.Contains(err.Error(), "collection is unknown") {
+		t.Fatalf("expected an unknown-collection error, got %v", err)
+	}
+}
+
+// Missing IDs require an extra acknowledgement only when metadata deletion
+// is requested; read-only diagnosis and known-volume purges are unchanged.
+func TestValidateMissingVolumePurgeConfirmation(t *testing.T) {
+	missing := []uint32{12}
+	for _, test := range []struct {
+		name        string
+		missing     []uint32
+		purgeAbsent bool
+		confirmed   bool
+		wantError   bool
+	}{
+		{name: "read-only scan", missing: missing},
+		{name: "purge known volumes only", purgeAbsent: true},
+		{name: "confirmed missing volume purge", missing: missing, purgeAbsent: true, confirmed: true},
+		{name: "unconfirmed missing volume purge", missing: missing, purgeAbsent: true, wantError: true},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			err := validateMissingVolumePurgeConfirmation(test.missing, test.purgeAbsent, test.confirmed)
+			if (err != nil) != test.wantError {
+				t.Fatalf("validateMissingVolumePurgeConfirmation() error = %v, wantError %v", err, test.wantError)
+			}
+			if err != nil && !strings.Contains(err.Error(), "confirmMissingVolumeDeletion") {
+				t.Fatalf("error %q does not name the required acknowledgement flag", err)
+			}
+		})
+	}
+}
+
+func TestVolumeFsckPurgesEntryWhoseManifestIsOnMissingVolume(t *testing.T) {
+	manifest := &filer_pb.FileChunk{
+		Fid:             &filer_pb.FileId{VolumeId: 99, FileKey: 1, Cookie: 1},
+		Size:            1,
+		IsChunkManifest: true,
+	}
+	stub := &stubFilerServer{entries: map[string]*filer_pb.Entry{
+		"/":     {IsDirectory: true},
+		"/big":  {Chunks: []*filer_pb.FileChunk{manifest}},
+		"/live": {Chunks: []*filer_pb.FileChunk{{Fid: &filer_pb.FileId{VolumeId: 7, FileKey: 2, Cookie: 1}, Size: 1}}},
+	}}
+
+	verbose := false
+	findMissing := true
+	var output bytes.Buffer
+	c := &commandVolumeFsck{
+		tempFolder:               t.TempDir(),
+		verbose:                  &verbose,
+		findMissingChunksInFiler: &findMissing,
+		writer:                   &output,
+		env:                      startStubFiler(t, stub),
+		volumeIds:                map[uint32]bool{99: true},
+		knownVolumeIds:           map[uint32]bool{7: true},
+	}
+	var purged []purgeItem
+	if err := c.collectFilerFileIdAndPaths(map[string]map[uint32]VInfo{"dn1": {7: {}}}, true, func(item purgeItem) {
+		purged = append(purged, item)
+	}, 0, 0); err != nil {
+		t.Fatalf("collect filer references: %v", err)
+	}
+
+	if !strings.Contains(output.String(), "ResolveChunkManifest failed for /big") {
+		t.Fatalf("manifest on the missing volume resolved unexpectedly: %q", output.String())
+	}
+	if len(purged) != 1 || purged[0].path != util.FullPath("/big") {
+		t.Fatalf("purged paths = %v, want /big, whose manifest is on missing volume 99", purged)
+	}
+}