diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index 29986cbbd..d6b634156 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -1824,6 +1824,27 @@ func (s3a *S3ApiServer) getLatestObjectVersion(bucket, object string) (*filer_pb return s3a.doGetLatestObjectVersion(bucket, object, 8) } +// lookupVersionsEntryWithRetry retries a .versions directory lookup, but only +// for errors that can change on retry (isRetryableFilerErr). A definitive +// NotFound is an answer, not a transient failure: retrying it walks the whole +// backoff ladder (12.7s at 8 attempts) before the caller's pre-versioning +// fallback gets to run, stalling every missing-key retention/tagging/ACL call. +func lookupVersionsEntryWithRetry(lookup func() (*filer_pb.Entry, error), maxRetries int) (entry *filer_pb.Entry, err error) { + for attempt := 1; attempt <= maxRetries; attempt++ { + entry, err = lookup() + if err == nil || !isRetryableFilerErr(err) { + return entry, err + } + + if attempt < maxRetries { + // Exponential backoff with higher base: 100ms, 200ms, 400ms, 800ms, 1600ms, 3200ms, 6400ms + delay := time.Millisecond * time.Duration(100*(1<<(attempt-1))) + time.Sleep(delay) + } + } + return entry, err +} + func (s3a *S3ApiServer) doGetLatestObjectVersion(bucket, object string, maxRetries int) (*filer_pb.Entry, error) { // Normalize object path to ensure consistency with toFilerPath behavior normalizedObject := s3_constants.NormalizeObjectKey(object) @@ -1834,20 +1855,9 @@ func (s3a *S3ApiServer) doGetLatestObjectVersion(bucket, object string, maxRetri glog.V(1).Infof("doGetLatestObjectVersion: looking for latest version of %s/%s (normalized: %s, retries: %d)", bucket, object, normalizedObject, maxRetries) // Get the .versions directory entry to read latest version metadata with retry logic for filer consistency - var versionsEntry *filer_pb.Entry - var err error - for attempt := 1; attempt <= maxRetries; attempt++ { - versionsEntry, err = s3a.getEntry(bucketDir, versionsObjectPath) - if err == nil { - break - } - - if attempt < maxRetries { - // Exponential backoff with higher base: 100ms, 200ms, 400ms, 800ms, 1600ms, 3200ms, 6400ms - delay := time.Millisecond * time.Duration(100*(1<<(attempt-1))) - time.Sleep(delay) - } - } + versionsEntry, err := lookupVersionsEntryWithRetry(func() (*filer_pb.Entry, error) { + return s3a.getEntry(bucketDir, versionsObjectPath) + }, maxRetries) if err != nil { // .versions directory doesn't exist - this can happen for objects that existed diff --git a/weed/s3api/s3api_object_versioning_lookup_retry_test.go b/weed/s3api/s3api_object_versioning_lookup_retry_test.go new file mode 100644 index 000000000..e985e33f8 --- /dev/null +++ b/weed/s3api/s3api_object_versioning_lookup_retry_test.go @@ -0,0 +1,62 @@ +package s3api + +import ( + "context" + "fmt" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/stretchr/testify/assert" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// A definitive NotFound (or an aborted request) is an answer, not a transient +// failure. Walking the full backoff ladder before the pre-versioning fallback +// turned every missing-key GetObjectRetention into a 12.7s stall in the field. +func TestLookupVersionsEntryTerminalErrorsSkipTheLadder(t *testing.T) { + for _, terminal := range []error{ + filer_pb.ErrNotFound, + fmt.Errorf("wrapped: %w", filer_pb.ErrNotFound), + status.Error(codes.NotFound, "gone"), + context.Canceled, + status.Error(codes.DeadlineExceeded, "context deadline exceeded"), + } { + calls := 0 + entry, err := lookupVersionsEntryWithRetry(func() (*filer_pb.Entry, error) { + calls++ + return nil, terminal + }, 8) + assert.Nil(t, entry) + assert.Equal(t, terminal, err) + assert.Equal(t, 1, calls, "%v is definitive, no retry", terminal) + } +} + +// Transient filer errors keep the original retry-with-backoff behavior. +func TestLookupVersionsEntryTransientErrorsStillRetry(t *testing.T) { + want := &filer_pb.Entry{Name: "obj" + ".versions"} + calls := 0 + entry, err := lookupVersionsEntryWithRetry(func() (*filer_pb.Entry, error) { + calls++ + if calls < 3 { + return nil, status.Error(codes.Unavailable, "transport is closing") + } + return want, nil + }, 8) + assert.NoError(t, err) + assert.Same(t, want, entry) + assert.Equal(t, 3, calls) +} + +// Exhausting the attempts surfaces the last transient error to the caller. +func TestLookupVersionsEntryTransientExhaustionReturnsLastErr(t *testing.T) { + calls := 0 + entry, err := lookupVersionsEntryWithRetry(func() (*filer_pb.Entry, error) { + calls++ + return nil, status.Error(codes.Unavailable, "still down") + }, 2) + assert.Nil(t, entry) + assert.Error(t, err) + assert.Equal(t, 2, calls) +}