mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-10-01 04:05:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(shell): volume.fsck keeps going past a single broken chunk manifest (#9140)

Browse Source 
* fix(shell): volume.fsck no longer aborts on a single broken chunk manifest

Previously a single entry whose chunk-manifest could not be read (e.g. the
manifest needle was missing or its sub-chunks pointed at a now-gone volume)
caused collectFilerFileIdAndPaths to return immediately with
"failed to ResolveChunkManifest". The whole fsck run failed, so an operator
with even one corrupted file could not use volume.fsck to find or clean up
unrelated orphan needles on other volumes — they had to locate and delete
the bad entries first, blind, with no help from fsck.

Log the resolution failure with the entry path, fall back to recording the
top-level chunk fids the entry references (data fids and manifest fids
themselves; sub-chunks behind the unresolvable manifest stay unknown), and
keep traversing. Track the count of unresolved entries on the command struct
and refuse -reallyDeleteFromVolume for the run when the count is non-zero,
since the in-use fid set is incomplete and a purge could otherwise delete
live sub-chunks behind the broken manifest. Read-only fsck still produces a
useful (if conservatively over-reported) orphan listing so the operator can
see and fix the broken entries first, then re-run with apply.

Discovered while diagnosing #9116.

* address review: use callback ctx and atomic counter

- Pass the BFS callback's ctx to ResolveChunkManifest so a Ctrl+C / first-error
  cancellation propagates into the manifest fetch instead of using
  context.Background().
- TraverseBfs runs the callback across K=5 worker goroutines (filer_pb/filer_client_bfs.go),
  so the unresolvedManifestEntries field on commandVolumeFsck is shared across
  workers and was racing. Switch it to atomic.Int64 with Add/Load.

* address review: reset counter per Do(), pass through ctx errors

- commandVolumeFsck is a singleton registered in init() and reused across
  shell invocations. Without resetting the unresolved-manifest counter at
  the top of Do(), a single failed run permanently suppressed
  -reallyDeleteFromVolume in the same shell session. Reset to 0 right
  after flag parsing.
- Treating context cancellation as manifest corruption was wrong: a
  Ctrl+C or deadline mid-traversal would inflate the counter and emit
  misleading "manifest broken" warnings for entries that were never
  examined. Detect context.Canceled / context.DeadlineExceeded and
  return the error so the BFS unwinds cleanly.

Not changing the findMissingChunksInFiler branch's purgeAbsent /
applyPurging gating: that path checks recorded filer fids against
volume idx files, and a broken-manifest entry's recorded manifest fid
will fail the existence check and get purged — which is the cleanup
the operator wants for those entries. Adding a gate would block the
exact use case the warning points them at.
This commit is contained in:
Chris Lu
2026-04-19 23:06:28 -07:00
committed by GitHub
parent 3ff92f797d
commit 9a6b566fb1
1 changed files with 53 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/shell/command_volume_fsck.go
+53 -15
View File
@@ -17,6 +17,7 @@ import (
"strconv"
"strings"
"sync"
"sync/atomic"
"time"
"github.com/seaweedfs/seaweedfs/weed/filer"
@@ -45,18 +46,19 @@ const (
)
type commandVolumeFsck struct {
env *CommandEnv
writer io.Writer
bucketsPath string
collection *string
volumeIds map[uint32]bool
tempFolder string
verbose *bool
forcePurging *bool
skipEcVolumes *bool
findMissingChunksInFiler *bool
verifyNeedle *bool
filerSigningKey string
env *CommandEnv
writer io.Writer
bucketsPath string
collection *string
volumeIds map[uint32]bool
tempFolder string
verbose *bool
forcePurging *bool
skipEcVolumes *bool
findMissingChunksInFiler *bool
verifyNeedle *bool
filerSigningKey string
unresolvedManifestEntries atomic.Int64
}
func (c *commandVolumeFsck) Name() string {
@@ -113,6 +115,12 @@ func (c *commandVolumeFsck) Do(args []string, commandEnv *CommandEnv, writer io.
return nil
}
// The command struct is a singleton registered in init(), so any state
// not bound to a flag persists across shell invocations. Reset the
// unresolved-manifest counter so a previous failed run can't permanently
// suppress -reallyDeleteFromVolume in this session.
c.unresolvedManifestEntries.Store(0)
if err = commandEnv.confirmIsLocked(args); err != nil {
return
}
@@ -219,8 +227,19 @@ func (c *commandVolumeFsck) Do(args []string, commandEnv *CommandEnv, writer io.
if err = c.collectFilerFileIdAndPaths(dataNodeVolumeIdToVInfo, false, 0, 0); err != nil {
return fmt.Errorf("failed to collect file ids from filer: %w", err)
}
// If any entry's manifest could not be resolved, our in-use fid set
// is missing the sub-chunks behind it. Purging orphans now would
// delete live data referenced only via the unresolved manifest, so
// disable -reallyDeleteFromVolume for this run and tell the operator
// to fix the broken entries first.
applyPurgingEffective := *applyPurging
if unresolved := c.unresolvedManifestEntries.Load(); unresolved > 0 && applyPurgingEffective {
fmt.Fprintf(c.writer, "WARNING: %d entry(ies) had unresolvable chunk manifests; refusing to apply -reallyDeleteFromVolume to avoid deleting live sub-chunks. Fix the entries listed above (e.g. delete or repair them) and re-run.\n",
unresolved)
applyPurgingEffective = false
}
// volume file ids subtract filer file ids
if err = c.findExtraChunksInVolumeServers(dataNodeVolumeIdToVInfo, *applyPurging, uint64(collectModifyFromAtNs), uint64(collectCutoffFromAtNs)); err != nil {
if err = c.findExtraChunksInVolumeServers(dataNodeVolumeIdToVInfo, applyPurgingEffective, uint64(collectModifyFromAtNs), uint64(collectCutoffFromAtNs)); err != nil {
return fmt.Errorf("findExtraChunksInVolumeServers: %w", err)
}
}
@@ -257,9 +276,28 @@ func (c *commandVolumeFsck) collectFilerFileIdAndPaths(dataNodeVolumeIdToVInfo m
if *c.verbose && entry.Entry.IsDirectory {
fmt.Fprintf(c.writer, "checking directory %s\n", util.NewFullPath(entry.Dir, entry.Entry.Name))
}
dataChunks, manifestChunks, resolveErr := filer.ResolveChunkManifest(context.Background(), filer.LookupFn(c.env), entry.Entry.GetChunks(), 0, math.MaxInt64)
dataChunks, manifestChunks, resolveErr := filer.ResolveChunkManifest(ctx, filer.LookupFn(c.env), entry.Entry.GetChunks(), 0, math.MaxInt64)
if resolveErr != nil {
return fmt.Errorf("failed to ResolveChunkManifest: %+v", resolveErr)
// Cancellation/deadline isn't manifest corruption; surface it
// so the BFS bails out cleanly without polluting the
// unresolved-manifest counter (which would otherwise block
// purges and mislead the operator about the failure cause).
if errors.Is(resolveErr, context.Canceled) || errors.Is(resolveErr, context.DeadlineExceeded) {
return resolveErr
}
// A single broken manifest used to abort the whole traversal,
// leaving the operator with no way to identify orphans without
// first fixing the broken file. Instead, record only the
// top-level chunk fids (data chunks plus the manifest needles
// themselves — sub-chunks behind the unreadable manifest are
// unknown), warn, and keep going. The unresolved counter blocks
// any purge step downstream so we never delete a sub-chunk we
// couldn't account for.
fmt.Fprintf(c.writer, "WARNING: ResolveChunkManifest failed for %s: %v — recording top-level chunk fids only; purging will be disabled\n",
util.NewFullPath(entry.Dir, entry.Entry.Name), resolveErr)
c.unresolvedManifestEntries.Add(1)
dataChunks = entry.Entry.GetChunks()
manifestChunks = nil
}
dataChunks = append(dataChunks, manifestChunks...)
for _, chunk := range dataChunks {