Files
seaweedfs/weed/s3api/s3api_versioning_reconciler_test.go
T
Chris Lu f5a4bfb514 fix(s3/versioning): repair dangling latest-version pointer after partial delete (#9460)
* fix(s3/versioning): repair dangling latest-version pointer after partial delete

deleteSpecificObjectVersion did two non-atomic filer ops: rm the version
blob, then update the .versions/ pointer. Step 2 failures were silently
logged and the client got 204 OK, so any transient blip (filer timeout,
process restart between RPCs, lock contention) left the .versions/
directory naming a missing file. Subsequent GETs paid the 10-retry
self-heal cost and returned NoSuchKey — surfacing as "Storage not found"
to Veeam, which is what triggered this investigation.

Three changes:

1. Pre-roll the pointer for the singleton / multi-version-deleting-latest
   cases. The pointer is repointed (multi) or cleared (singleton) before
   the blob rm. A failure between leaves a recoverable orphan blob —
   pointer is consistent, GETs succeed or correctly miss without
   entering the stale-pointer self-heal path.

2. Wrap the load-bearing filer ops in updateLatestVersionAfterDeletion
   with bounded retries (~6.3s worst case). When retries are exhausted
   the function now returns a non-nil error instead of swallowing it.
   The caller logs at Error level and queues the path for the
   reconciler.

3. Background reconciler drains stranded .versions/ pointer-to-missing
   states off the hot path. Bounded in-memory queue with capped retries;
   read-path heal remains as a last-resort safety net.

* fix(s3/versioning): address review on #9460

Four fixes addressing review on PR #9460. All four are correctness;
no behavioural change for the happy path.

1. repointLatestBeforeDeletion: discriminate NotFound from transient
   errors when re-fetching the .versions/ entry. Previously any error
   returned rolled=true,nil — a transient filer hiccup at that point
   would cause the caller to skip the post-delete reconciliation AND
   proceed with the blob rm, producing exactly the dangling pointer
   state the PR aims to prevent. NotFound stays "vacuously consistent"
   (directory already gone); other errors surface so the caller aborts
   before removing the blob.

2. Move the singleton .versions/ teardown out of
   repointLatestBeforeDeletion (where it ran BEFORE the blob rm and
   always failed with "non-empty folder") into deleteSpecificObjectVersion
   AFTER the blob rm. Adds a wasSingleton return value so the caller
   knows when to run the teardown. Without this, every singleton-version
   delete in a versioned bucket leaked an empty .versions/ directory.

3. Wrap the list, getEntry, and mkFile calls inside
   repointLatestBeforeDeletion with retryFilerOp so the pre-roll has
   the same transient-failure resilience as the post-roll path. Without
   retries, a single transient blip causes the caller to fall back to
   the legacy non-atomic flow even when the filer recovers immediately.

4. healVersionsPointer in the reconciler: same NotFound-vs-transient
   discrimination on both the .versions/ getEntry and the latest-file
   presence probe. Previously a transient filer error would silently
   evict the candidate from the queue as "healed", leaving the real
   stranded state until a client read happened to surface it.

Also fixes the gemini-flagged consistency nit: the queued-for-reconciler
error log now uses normalizedObject instead of object so it matches the
queue entry's key.

* fix(s3/versioning): short-circuit terminal errors in retryFilerOp

Add isRetryableFilerErr that returns false for filer_pb.ErrNotFound,
gRPC NotFound, context.Canceled, and context.DeadlineExceeded.
retryFilerOp now bails immediately on a terminal error and returns it
unwrapped, so callers like repointLatestBeforeDeletion.getEntry and
updateLatestVersionAfterDeletion.rm see the raw NotFound instead of
paying the ~6.3 s retry-budget delay AND parsing it out of an
"exhausted N retries" wrapper.

errors.Is and status.Code already walk the %w chain so today's call
sites still work, but the delay was real on the hot DELETE path
whenever a key was genuinely absent. Test added covering all five
terminal-error shapes — each must run the wrapped fn exactly once and
return in under 50 ms.
2026-05-13 10:14:27 -07:00

145 lines
4.8 KiB
Go

package s3api
import (
"context"
"errors"
"fmt"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
// TestVersionsHealQueue_DedupOnEnqueue ensures multiple enqueues of the
// same bucket/object collapse into a single pending entry, so a hot
// failure path doesn't bloat the queue.
func TestVersionsHealQueue_DedupOnEnqueue(t *testing.T) {
q := newVersionsHealQueue()
for i := 0; i < 5; i++ {
q.Enqueue("b", "obj")
}
assert.Equal(t, 1, q.Len(), "duplicate enqueues collapse")
}
// TestVersionsHealQueue_CapacityCap ensures the queue refuses growth
// past the static cap and logs at V(1) instead of OOM-ing.
func TestVersionsHealQueue_CapacityCap(t *testing.T) {
q := newVersionsHealQueue()
for i := 0; i < versionsHealQueueCapacity+50; i++ {
q.Enqueue("b", string(rune(i)))
}
assert.Equal(t, versionsHealQueueCapacity, q.Len(), "queue clamps at capacity")
}
// TestVersionsHealQueue_PopReadyOnlyDueItems checks that nextRetry
// gating keeps not-yet-ready candidates in the queue.
func TestVersionsHealQueue_PopReadyOnlyDueItems(t *testing.T) {
q := newVersionsHealQueue()
q.Enqueue("b", "due")
// Inject a deferred candidate directly so we control its nextRetry.
q.pending[versionsHealKey("b", "later")] = &versionsHealCandidate{
bucket: "b",
object: "later",
enqueued: time.Now(),
nextRetry: time.Now().Add(10 * time.Minute),
}
due := q.popReady(time.Now())
require.Len(t, due, 1, "only the due candidate pops")
assert.Equal(t, "due", due[0].object)
assert.Equal(t, 1, q.Len(), "deferred candidate still queued")
}
// TestVersionsHealQueue_RequeueWithBackoff verifies failed candidates
// re-enter the queue with an extended nextRetry.
func TestVersionsHealQueue_RequeueWithBackoff(t *testing.T) {
q := newVersionsHealQueue()
c := &versionsHealCandidate{bucket: "b", object: "obj", attempts: 1}
q.requeue(c, 500*time.Millisecond)
assert.Equal(t, 1, q.Len())
now := time.Now()
due := q.popReady(now)
assert.Empty(t, due, "not yet due immediately after requeue")
due = q.popReady(now.Add(time.Second))
assert.Len(t, due, 1, "due after backoff window passes")
}
// TestVersionsHealQueue_GiveUpAfterMaxAttempts ensures we don't loop
// forever against a deterministically broken state.
func TestVersionsHealQueue_GiveUpAfterMaxAttempts(t *testing.T) {
q := newVersionsHealQueue()
c := &versionsHealCandidate{bucket: "b", object: "obj", attempts: versionsHealMaxRetries}
q.requeue(c, time.Millisecond)
assert.Equal(t, 0, q.Len(), "candidate at max attempts is dropped, read-path heal still covers it")
}
// TestRetryFilerOp_SucceedsBeforeExhaustion confirms a flaky op that
// eventually succeeds is reported as success without surfacing prior
// errors.
func TestRetryFilerOp_SucceedsBeforeExhaustion(t *testing.T) {
calls := 0
err := retryFilerOp("test", func() error {
calls++
if calls < 3 {
return errors.New("transient")
}
return nil
})
assert.NoError(t, err)
assert.Equal(t, 3, calls, "stops calling once the op succeeds")
}
// TestRetryFilerOp_PropagatesAfterExhaustion confirms a deterministic
// failure is wrapped with the attempt count so operators can tell at
// a glance whether the underlying issue is transient.
func TestRetryFilerOp_PropagatesAfterExhaustion(t *testing.T) {
calls := 0
err := retryFilerOp("test", func() error {
calls++
return errors.New("permanent")
})
require.Error(t, err)
assert.Equal(t, updateLatestRetryAttempts, calls, "ran the full retry budget")
assert.Contains(t, err.Error(), "exhausted")
assert.Contains(t, err.Error(), "permanent", "underlying error preserved")
}
// TestRetryFilerOp_TerminalErrorsShortCircuit confirms that errors which
// won't change on retry (NotFound, context cancellation) are returned
// immediately and unwrapped, without burning the retry budget.
func TestRetryFilerOp_TerminalErrorsShortCircuit(t *testing.T) {
cases := []struct {
name string
err error
}{
{"filer_pb.ErrNotFound", filer_pb.ErrNotFound},
{"wrapped filer_pb.ErrNotFound", fmt.Errorf("wrap: %w", filer_pb.ErrNotFound)},
{"grpc NotFound status", status.Error(codes.NotFound, "missing")},
{"context canceled", context.Canceled},
{"context deadline exceeded", context.DeadlineExceeded},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
start := time.Now()
calls := 0
err := retryFilerOp("test", func() error {
calls++
return tc.err
})
elapsed := time.Since(start)
require.Error(t, err)
assert.Equal(t, 1, calls, "terminal error must not be retried")
assert.Less(t, elapsed, 50*time.Millisecond, "terminal error must not delay")
assert.NotContains(t, err.Error(), "exhausted", "terminal error must not be wrapped with retry-budget prefix")
})
}
}