mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 12:16:36 +00:00
9fd7075bea
* filer: stop silently dropping metadata replay failures from peers When two filers do not share a store (e.g. one leveldb3 per pod), each subscribes to its peers' metadata streams and replays their events locally (meta_aggregator.go's maybeReplicateMetadataChange, wired into doSubscribeToOneFiler). A failed Replay() was logged and then treated as done anyway: processEventFn always returned nil regardless of the replay outcome, and processOne advanced lastTsNs unconditionally. The offset is the only record of subscription progress, so a dropped event was gone for good - no retry, and nothing else ever observed it. An entry that fails to replay this way diverges from its peer permanently. This is how a bucket's quota (entry.Quota, carried on peer events like everything else - see entry_codec.go's EqualEntry comparing Quota, and FromPbEntry copying it in entry.go) can end up different across filers indefinitely: one replay hiccup on one filer, and its enforcement and any metric reading its own store diverges from the others' with no signal anything went wrong. Fix: replicateMetadataChange now retries a failure with util.Retry, which already distinguishes transient errors (timeouts, connection resets, throttling, ...) from everything else and bounds the backoff. That covers the common case - a busy store, a blip talking to a remote-backed backend - without changing behavior when replay succeeds. An error that is not transient, or outlives the retry budget, is not retried further: propagating it so the offset never advances would stall this peer's entire stream behind one event that may never replay, which is worse than the one entry staying stale. Instead it is skipped, loudly - counted in a new stats.FilerMetaAggregatorReplayFailures metric and logged at error level - so the divergence is discoverable instead of silent. Tested: go build ./... and go test ./weed/filer/... ./weed/server/... Added meta_aggregator_replay_test.go: one test fails against the old one-shot Replay call (a single transient failure is never retried, so the store never converges) and passes with the fix; a second covers a permanently-failing event completing quickly and being counted instead of retried forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * filer: keep the test quota constant int64 for 32-bit builds An untyped shift constant passed to t.Fatalf's ...any defaults to int and overflows on 32-bit, failing go vet there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * filer: name the diverged entry in the give-up log line event.Directory is only the parent (typically /buckets), so for any directory with more than one child the previous log line could not say which entry failed to replay - the exact thing the change exists to make discoverable. Name comes from NewEntry, falling back to OldEntry for deletes; both getters are nil-safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(filer): trim metadata replay comments to the non-obvious why Compress the added comments on replicateMetadataChange and its tests down to the reasoning a maintainer cannot get from the code: why a retry-exhausted failure is skipped rather than propagated, what the old one-shot Replay body did that the test pins, and why the quota constant is typed int64. Drops deployment-specific narration and restatement of the code. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: restore load-bearing clauses trimmed in the comment pass Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * filer: document and test the multi-step DeleteEntry replay hazard CodeRabbit flagged that FilerStoreWrapper.DeleteEntry skips the delete once FindEntry reports the path already gone, and that the redis store families remove the primary key before parent-directory membership. Chained together, a delete that fails between those two steps is retried as a no-op: the stale membership is never revisited, and replicateMetadataChange now reports overall success for it without incrementing FilerMetaAggregatorReplayFailures, whereas before this PR every such failure was unconditionally logged. The underlying store inconsistency is pre-existing (a single non-retried Replay already leaves the same stale membership behind); what retry adds is that this one case no longer surfaces it. Making Replay atomic or teaching every store to repair secondary mutations on retry is out of scope here. Instead: document the hazard at Replay, filerstore_wrapper.go's DeleteEntry, and replicateMetadataChange, and add a test against the real FilerStoreWrapper (not a strawman) that pins down the current, documented behavior. * filer: trim replay retry comments and tests Drop the comment-only hunks documenting the pre-existing DeleteEntry partial-failure hazard, the test that asserted that hazard still exists, and the second hand-rolled fake store. Reuse stubFilerStore for the two retry tests. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>