From c58795354a77811ab42f69cc986dc10b3e36b8bc Mon Sep 17 00:00:00 2001
From: Chris Lu <chrislusf@users.noreply.github.com>
Date: Sat, 22 Aug 2026 23:42:33 -0700
Subject: [PATCH] s3api: retry a transient filer failure on metadata listings
 (#10890)

* s3api: retry a transient failure when listing multipart uploads/parts

A blip on the way to the filer failed the whole ListMultipartUploads or
ListParts request. Both reported failure points sit inside one streaming
listing: the ListEntries call that opens the stream, and the stream.Recv
calls that drain it. Neither retried, so a single Unavailable answer from
a filer that was restarting turned into a 500 for the S3 client.

Replay the listing instead, bounded to three attempts with a 100ms
backoff that doubles. Only a transient failure is replayed. A not-found
answer stays authoritative so the empty-list branch still works, and
every other error still reaches the client on the first attempt.

This is scoped to (*S3ApiServer).list rather than added inside
DoSeaweedListWithSnapshot, which mount, the shell and the other object
listings share, and where a retry after a partial stream would
re-deliver entries the callback had already seen. Within one call to
list, a replay is safe: it collects into a fresh slice each time, so it
can neither duplicate nor drop entries.

That guarantee does not extend past this function. withFilerClientFailover
already re-runs its callback against the next filer on any non-NotFound
error without resetting the caller's accumulator, so on a multi-filer
gateway a mid-listing failover can itself produce a duplicated result
with err == nil, independent of this change and not fixed by it. Noted
in the PR rather than silently left for someone to rediscover.

Fixes #7221
References #7235

* s3api: move the listing retry inside list itself

---------

Co-authored-by: Junker der Provinz <jdp@braethoria.com>
---
 weed/s3api/filer_util.go | 55 +++++++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/weed/s3api/filer_util.go b/weed/s3api/filer_util.go
index fa1acd75f..e15fb9f18 100644
--- a/weed/s3api/filer_util.go
+++ b/weed/s3api/filer_util.go
@@ -30,20 +30,57 @@ func (s3a *S3ApiServer) mkFile(parentDirectoryPath string, fileName string, chun
 
 func (s3a *S3ApiServer) list(parentDirectoryPath, prefix, startFrom string, inclusive bool, limit uint32) (entries []*filer_pb.Entry, isLast bool, err error) {
 
-	err = filer_pb.List(context.Background(), s3a, parentDirectoryPath, prefix, func(entry *filer_pb.Entry, isLastEntry bool) error {
-		entries = append(entries, entry)
-		if isLastEntry {
+	return listWithRetry(parentDirectoryPath, func() (entries []*filer_pb.Entry, isLast bool, err error) {
+		err = filer_pb.List(context.Background(), s3a, parentDirectoryPath, prefix, func(entry *filer_pb.Entry, isLastEntry bool) error {
+			entries = append(entries, entry)
+			if isLastEntry {
+				isLast = true
+			}
+			return nil
+		}, startFrom, inclusive, limit)
+
+		if len(entries) == 0 {
 			isLast = true
 		}
-		return nil
-	}, startFrom, inclusive, limit)
 
-	if len(entries) == 0 {
-		isLast = true
+		return
+	})
+
+}
+
+// A listing has no side effects and collects into a fresh slice per attempt, so
+// a replay can neither duplicate nor drop entries; the bound caps a filer that
+// is genuinely down at two extra attempts and 300ms of added wait.
+const (
+	listRetryAttempts       = 3
+	listRetryInitialBackoff = 100 * time.Millisecond
+)
+
+// isRetryableListError classifies by message via util.IsTransientError because
+// DoSeaweedListWithSnapshot wraps a failed ListEntries call with %v, dropping
+// the gRPC status from the chain. Not-found is authoritative and must reach the
+// caller unchanged.
+func isRetryableListError(err error) bool {
+	return err != nil && !isFilerNotFound(err) && util.IsTransientError(err)
+}
+
+// listWithRetry replays doList while the filer answers with a transient error.
+// Both failure points, the ListEntries call itself and the stream.Recv that
+// follows it, surface as a plain error out of filer_pb.List, so a single retry
+// point above it covers both.
+func listWithRetry(parentDirectoryPath string, doList func() (entries []*filer_pb.Entry, isLast bool, err error)) (entries []*filer_pb.Entry, isLast bool, err error) {
+
+	backoff := listRetryInitialBackoff
+	for attempt := 1; ; attempt++ {
+		entries, isLast, err = doList()
+		if err == nil || attempt >= listRetryAttempts || !isRetryableListError(err) {
+			return entries, isLast, err
+		}
+		glog.V(1).Infof("list %s attempt %d/%d hit a transient error, retrying in %v: %v", parentDirectoryPath, attempt, listRetryAttempts, backoff, err)
+		time.Sleep(backoff)
+		backoff *= 2
 	}
 
-	return
-
 }
 
 // Bounds for replaying a listing that failed with a transient error. A listing