From ba90ae5c94631dc406ff4bead86db2b99c24ffdf Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 8 Apr 2026 17:08:57 -0700 Subject: [PATCH] fix(s3): don't count ErrNotFound as filer health failure in failover (#8995) * fix(s3): don't count ErrNotFound as filer health failure in failover The S3 gateway's filer client failover was recording ErrNotFound (entry doesn't exist) as a filer health failure. In multi-filer setups where filers have separate metadata stores, normal object lookups that return "not found" accumulated in the circuit breaker, eventually marking healthy filers as unhealthy after just 3 lookups. This caused the distributed lock integration test to fail with 500 InternalError: once a filer was circuit-broken, subsequent lookups could no longer fall back, turning a would-be 412 PreconditionFailed into an unrecoverable internal error. Only record actual transport/server failures in the health tracker. The failover still tries other filers for data locality, but no longer penalizes filers for correctly reporting missing entries. * style: inline isNotFound variable for consistency The variable was only used once; inlining it matches the pattern already used in the failover loop a few lines below. --- weed/s3api/s3api_handlers.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/weed/s3api/s3api_handlers.go b/weed/s3api/s3api_handlers.go index d74741944..7f74e5dd4 100644 --- a/weed/s3api/s3api_handlers.go +++ b/weed/s3api/s3api_handlers.go @@ -2,6 +2,7 @@ package s3api import ( "encoding/base64" + "errors" "fmt" "net/http" @@ -46,8 +47,13 @@ func (s3a *S3ApiServer) withFilerClientFailover(streamingMode bool, fn func(file return nil } - // Record failure for current filer - s3a.filerClient.RecordFilerFailure(currentFiler) + // ErrNotFound is a valid application-level response (entry doesn't exist on this filer), + // not a filer health issue. Only record true failures (transport errors, timeouts, etc.) + // in the health tracker to avoid poisoning the circuit breaker with normal "not found" + // responses in multi-filer setups. + if !errors.Is(err, filer_pb.ErrNotFound) { + s3a.filerClient.RecordFilerFailure(currentFiler) + } // Current filer failed - try all other filers with health-aware selection filers := s3a.filerClient.GetAllFilers() @@ -77,8 +83,10 @@ func (s3a *S3ApiServer) withFilerClientFailover(streamingMode bool, fn func(file return nil } - // Record failure for health tracking - s3a.filerClient.RecordFilerFailure(filer) + // Only record real failures, not ErrNotFound + if !errors.Is(err, filer_pb.ErrNotFound) { + s3a.filerClient.RecordFilerFailure(filer) + } glog.V(2).Infof("WithFilerClient: failover to %s failed: %v", filer, err) lastErr = err }