filer: make a retried ObjectTransaction replay its recorded outcome (#11788)

* filer_pb: add request_id to ObjectTransactionRequest

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* filer: replay recorded outcome for a retried ObjectTransaction

A failover replay carries the same request_id; under the same path lock
the owner replays the recorded outcome instead of re-applying the
mutations over whatever superseded them. Only application-level
outcomes are recorded — a lookup error leaves the request free to
re-run.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* s3: stamp routed object transactions with a request id

One id per logical transaction so failover retries replay the recorded
outcome instead of re-applying the mutations.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* filer: scope transaction replay tokens per path and keep them for their TTL

The dedup key includes the lock path so a token reused on a different
object cannot replay this outcome. Records are never evicted early —
an early eviction would let a replay re-apply — so the expiry sweep is
amortized and the map is bounded by the insert rate over the TTL.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* filer: bound the transaction replay cache at capacity

* filer: do not record indeterminate transaction outcomes

A mutation error can be transient or follow a partial apply; recording
it replays that failure to a retry for the whole TTL instead of letting
the same idempotent mutations converge. Only successful transactions
and precondition failures are definitive outcomes worth replaying.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* filer: replay recorded mutation failures instead of re-applying

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

weed/server/filer_grpc_server.go

@@ -344,6 +344,7 @@ func (fs *FilerServer) ObjectTransaction(ctx context.Context, req *filer_pb.Obje
 			ConditionKey:       req.ConditionKey,
 			RouteKey:           req.RouteKey,
 			IsMoved:            true,
+			RequestId:          req.RequestId,
 			ConditionAltKey:    req.ConditionAltKey,
 		}
 		var resp *filer_pb.ObjectTransactionResponse
@@ -367,6 +368,28 @@ func (fs *FilerServer) ObjectTransaction(ctx context.Context, req *filer_pb.Obje
 	pathLock := fs.entryLockTable.AcquireLock("ObjectTransaction", lockPath, util.ExclusiveLock)
 	defer fs.entryLockTable.ReleaseLock(lockPath, pathLock)
 
+	// Under the same lock the first application committed with, a failover
+	// replay of the same request replays its recorded outcome instead of
+	// re-applying mutations over whatever superseded it.
+	if resp, found := fs.recentObjectTxn(req.RequestId, lockPath); found {
+		glog.V(2).InfofCtx(ctx, "ObjectTransaction %s: replaying recorded outcome for request %s", req.LockKey, req.RequestId)
+		return resp, nil
+	}
+
+	resp := &filer_pb.ObjectTransactionResponse{}
+	// Outcomes are replayed rather than re-run: a mutation error may follow a
+	// partial apply, and a retry that re-runs could write the stale mutation
+	// over whatever superseded it, while replaying the recorded failure at
+	// least matches what the first attempt reported. The one exception is the
+	// condition lookup below: it fails before any mutation runs, so the retry
+	// must re-evaluate rather than replay an empty outcome.
+	record := true
+	defer func() {
+		if record {
+			fs.rememberObjectTxn(req.RequestId, lockPath, resp)
+		}
+	}()
+
 	if conditionIsSet(req.Condition) {
 		// The condition is evaluated against condition_key when set (e.g. a
 		// version entry whose WORM guards gate the delete), while the lock stays
@@ -393,28 +416,102 @@ func (fs *FilerServer) ObjectTransaction(ctx context.Context, req *filer_pb.Obje
 			current, conditionPath, findErr = fs.latestVersionedConditionEntry(ctx, util.FullPath(req.ConditionAltKey))
 		}
 		if findErr != nil && findErr != filer_pb.ErrNotFound {
-			return &filer_pb.ObjectTransactionResponse{}, fmt.Errorf("ObjectTransaction condition %s: %w", conditionPath, findErr)
+			record = false // a lookup error decides nothing; a retry must re-run, not replay
+			return nil, fmt.Errorf("ObjectTransaction condition %s: %w", conditionPath, findErr)
 		}
 		if findErr == filer_pb.ErrNotFound {
 			current = nil
 		}
 		if !writeConditionSatisfied(req.Condition, current) {
 			glog.V(3).InfofCtx(ctx, "ObjectTransaction %s: precondition failed", conditionPath)
-			return &filer_pb.ObjectTransactionResponse{
-				Error:     "precondition failed",
-				ErrorCode: filer_pb.FilerError_PRECONDITION_FAILED,
-			}, nil
+			resp.Error = "precondition failed"
+			resp.ErrorCode = filer_pb.FilerError_PRECONDITION_FAILED
+			return resp, nil
 		}
 	}
 
 	for i, m := range req.Mutations {
 		if err := fs.applyObjectMutation(ctx, m, req.IsFromOtherCluster, req.Signatures); err != nil {
 			glog.V(2).InfofCtx(ctx, "ObjectTransaction %s mutation %d (%v): %v", lockPath, i, m.Type, err)
-			return &filer_pb.ObjectTransactionResponse{Error: fmt.Sprintf("mutation %d: %v", i, err)}, nil
+			resp.Error = fmt.Sprintf("mutation %d: %v", i, err)
+			return resp, nil
 		}
 	}
 
-	return &filer_pb.ObjectTransactionResponse{}, nil
+	return resp, nil
+}
+
+const (
+	recentObjectTxnTTL         = 2 * time.Minute
+	recentObjectTxnSweepCap    = 10000
+	recentObjectTxnSweepPeriod = 30 * time.Second
+)
+
+type recentObjectTxn struct {
+	resp      *filer_pb.ObjectTransactionResponse
+	expiresAt time.Time
+}
+
+// recentObjectTxnKey scopes an idempotency token to the transaction's lock
+// path, so a token reused on a different object cannot replay this outcome.
+func recentObjectTxnKey(requestID string, lockPath util.FullPath) string {
+	return requestID + "\x00" + string(lockPath)
+}
+
+// recentObjectTxn returns the recorded outcome of an already-applied request,
+// so a failover replay of the same ObjectTransaction does not re-apply it.
+func (fs *FilerServer) recentObjectTxn(requestID string, lockPath util.FullPath) (*filer_pb.ObjectTransactionResponse, bool) {
+	if requestID == "" {
+		return nil, false
+	}
+	fs.recentObjectTxnsMu.Lock()
+	defer fs.recentObjectTxnsMu.Unlock()
+
+	key := recentObjectTxnKey(requestID, lockPath)
+	txn, found := fs.recentObjectTxns[key]
+	if !found {
+		return nil, false
+	}
+	if time.Now().After(txn.expiresAt) {
+		delete(fs.recentObjectTxns, key)
+		return nil, false
+	}
+	return txn.resp, true
+}
+
+func (fs *FilerServer) rememberObjectTxn(requestID string, lockPath util.FullPath, resp *filer_pb.ObjectTransactionResponse) {
+	if requestID == "" || resp == nil {
+		return
+	}
+	fs.recentObjectTxnsMu.Lock()
+	defer fs.recentObjectTxnsMu.Unlock()
+	if fs.recentObjectTxns == nil {
+		fs.recentObjectTxns = make(map[string]recentObjectTxn)
+	}
+	// A live record is never evicted early: a replay that misses re-applies the
+	// transaction. The sweep is amortized so a hot cache does not scan on every
+	// write; between sweeps the map may exceed the cap, still bounded by the
+	// insert rate over the TTL.
+	now := time.Now()
+	if len(fs.recentObjectTxns) >= recentObjectTxnSweepCap {
+		if now.After(fs.recentObjectTxnsSweepAt) {
+			fs.recentObjectTxnsSweepAt = now.Add(recentObjectTxnSweepPeriod)
+			for id, txn := range fs.recentObjectTxns {
+				if now.After(txn.expiresAt) {
+					delete(fs.recentObjectTxns, id)
+				}
+			}
+		}
+		if len(fs.recentObjectTxns) >= recentObjectTxnSweepCap {
+			// Full of unexpired records: drop the new one rather than evict
+			// an outcome a replay may still need.
+			return
+		}
+	}
+	fs.recentObjectTxns[recentObjectTxnKey(requestID, lockPath)] = recentObjectTxn{
+		resp:      resp,
+		expiresAt: now.Add(recentObjectTxnTTL),
+	}
 }
 
 // latestVersionedConditionEntry resolves the object a conditional write should

weed/server/filer_grpc_server_object_txn_test.go

@@ -2,6 +2,7 @@ package weed_server
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"os"
 	"strconv"
@@ -855,6 +856,139 @@ func TestObjectTransactionForwardsToOwner(t *testing.T) {
 	}
 }
 
+// A failover replay carries the same request_id. Under the path lock the owner
+// replays the recorded outcome instead of re-applying the mutations over a
+// newer entry.
+func TestObjectTransactionRequestIdDedupesReplay(t *testing.T) {
+	now := time.Unix(1700000000, 0)
+	fs, store := newTxnTestServer(t, nil)
+
+	put := func(etag string) *filer_pb.ObjectMutation {
+		return &filer_pb.ObjectMutation{
+			Type:      filer_pb.ObjectMutation_PUT,
+			Directory: "/buckets/bkt",
+			Entry: &filer_pb.Entry{
+				Name:       "obj",
+				Attributes: &filer_pb.FuseAttributes{Mtime: now.Unix(), FileMode: 0644, Inode: 1},
+				Extended:   map[string][]byte{"etag": []byte(etag)},
+			},
+		}
+	}
+	txn := func(requestID, etag string) *filer_pb.ObjectTransactionRequest {
+		return &filer_pb.ObjectTransactionRequest{
+			LockKey:   "/buckets/bkt/obj",
+			RequestId: requestID,
+			Mutations: []*filer_pb.ObjectMutation{put(etag)},
+		}
+	}
+
+	respA, err := fs.ObjectTransaction(context.Background(), txn("req-a", "A"))
+	if err != nil || respA.Error != "" {
+		t.Fatalf("first put: err=%v resp=%q", err, respA.Error)
+	}
+
+	// a newer writer lands the same path
+	respB, err := fs.ObjectTransaction(context.Background(), txn("req-b", "B"))
+	if err != nil || respB.Error != "" {
+		t.Fatalf("second put: err=%v resp=%q", err, respB.Error)
+	}
+
+	// the replayed request replays A's outcome; the stored entry stays B
+	respReplay, err := fs.ObjectTransaction(context.Background(), txn("req-a", "A"))
+	if err != nil || respReplay.Error != "" {
+		t.Fatalf("replayed put: err=%v resp=%q", err, respReplay.Error)
+	}
+	entry := store.entries["/buckets/bkt/obj"]
+	if entry == nil {
+		t.Fatal("entry missing after replay")
+	}
+	if got := string(entry.Extended["etag"]); got != "B" {
+		t.Fatalf("replayed transaction re-applied the stale entry: etag = %q, want B", got)
+	}
+}
+
+// A recorded application-level failure replays to the retried request even
+// after the world changes enough for the condition to pass, and its mutation
+// still does not run.
+func TestObjectTransactionRequestIdReplaysRecordedError(t *testing.T) {
+	now := time.Unix(1700000000, 0)
+	fs, store := newTxnTestServer(t, nil)
+
+	req := &filer_pb.ObjectTransactionRequest{
+		LockKey:   "/buckets/bkt/obj",
+		RequestId: "req-fail",
+		Condition: &filer_pb.WriteCondition{
+			Clauses: []*filer_pb.WriteCondition_Clause{{Kind: filer_pb.WriteCondition_IF_EXISTS}},
+		},
+		Mutations: []*filer_pb.ObjectMutation{{
+			Type:      filer_pb.ObjectMutation_PUT,
+			Directory: "/buckets/bkt",
+			Entry: &filer_pb.Entry{
+				Name:       "obj",
+				Attributes: &filer_pb.FuseAttributes{Mtime: now.Unix(), FileMode: 0644, Inode: 1},
+				Extended:   map[string][]byte{"etag": []byte("stale")},
+			},
+		}},
+	}
+
+	resp, err := fs.ObjectTransaction(context.Background(), req)
+	if err != nil || resp.ErrorCode != filer_pb.FilerError_PRECONDITION_FAILED {
+		t.Fatalf("precondition: err=%v code=%v", err, resp.GetErrorCode())
+	}
+
+	// another writer lands the object the precondition wanted
+	other := &filer_pb.ObjectTransactionRequest{
+		LockKey:   "/buckets/bkt/obj",
+		RequestId: "req-other",
+		Mutations: []*filer_pb.ObjectMutation{{
+			Type:      filer_pb.ObjectMutation_PUT,
+			Directory: "/buckets/bkt",
+			Entry: &filer_pb.Entry{
+				Name:       "obj",
+				Attributes: &filer_pb.FuseAttributes{Mtime: now.Unix(), FileMode: 0644, Inode: 2},
+				Extended:   map[string][]byte{"etag": []byte("live")},
+			},
+		}},
+	}
+	respOther, err := fs.ObjectTransaction(context.Background(), other)
+	if err != nil || respOther.Error != "" {
+		t.Fatalf("other put: err=%v resp=%q", err, respOther.Error)
+	}
+
+	// the retry replays the recorded failure instead of re-evaluating and
+	// overwriting the live entry
+	resp2, err := fs.ObjectTransaction(context.Background(), req)
+	if err != nil || resp2.ErrorCode != filer_pb.FilerError_PRECONDITION_FAILED {
+		t.Fatalf("replayed precondition: err=%v code=%v", err, resp2.GetErrorCode())
+	}
+	if got := string(store.entries["/buckets/bkt/obj"].Extended["etag"]); got != "live" {
+		t.Fatalf("replayed transaction overwrote the live entry: etag = %q", got)
+	}
+}
+
+// At capacity the cache drops new recordings rather than evicting a live
+// outcome a replay may still need.
+func TestObjectTransactionReplayCacheDropsNewAtCap(t *testing.T) {
+	fs, _ := newTxnTestServer(t, nil)
+	path := util.FullPath("/buckets/bkt/obj")
+	resp := &filer_pb.ObjectTransactionResponse{}
+
+	for i := 0; i < recentObjectTxnSweepCap; i++ {
+		fs.rememberObjectTxn(fmt.Sprintf("req-%d", i), path, resp)
+	}
+	fs.rememberObjectTxn("req-overflow", path, resp)
+
+	if _, found := fs.recentObjectTxn("req-0", path); !found {
+		t.Fatal("a live record was evicted to admit a new one")
+	}
+	if _, found := fs.recentObjectTxn("req-overflow", path); found {
+		t.Fatal("overflow record was recorded instead of dropped")
+	}
+	if len(fs.recentObjectTxns) > recentObjectTxnSweepCap {
+		t.Fatalf("cache exceeded cap: %d", len(fs.recentObjectTxns))
+	}
+}
+
 // A suspended-versioning conditional write falls back to the latest version
 // under the transaction lock: a real latest satisfies existence checks, and a
 // delete marker reads as absent.

weed/server/filer_server.go

@@ -142,6 +142,13 @@ type FilerServer struct {
 	recentCopyRequestsMu sync.Mutex
 	recentCopyRequests   map[string]recentCopyRequest
 
+	// outcomes of recently applied object transactions keyed by request_id, so
+	// a failover replay of the same transaction replays its outcome instead of
+	// re-applying its mutations over a newer entry
+	recentObjectTxnsMu      sync.Mutex
+	recentObjectTxns        map[string]recentObjectTxn
+	recentObjectTxnsSweepAt time.Time
+
 	// credential manager for IAM operations
 	CredentialManager *credential.CredentialManager
 
@@ -236,6 +243,7 @@ func NewFilerServer(defaultMux, readonlyMux *http.ServeMux, option *FilerOption)
 		subscribers:           make(map[int32]*metadataSubscriber),
 		inFlightDataLimitCond: sync.NewCond(new(sync.Mutex)),
 		recentCopyRequests:    make(map[string]recentCopyRequest),
+		recentObjectTxns:      make(map[string]recentObjectTxn),
 		CredentialManager:     option.CredentialManager,
 		entryLockTable:        util.NewLockTable[util.FullPath](),
 		posixLocks:            posixlock.NewManager(),