s3: evaluate multipart completion conditions under the object lock (#12209)

* filer_pb,filer: per-mutation preconditions on ObjectTransaction

A transaction's request-level condition evaluates one entry before any
mutation runs, so a commit guarding two paths — the object it publishes
and the upload directory it assembles — could not be expressed. Give each
ObjectMutation an optional condition (evaluated against condition_key,
else the mutation's target, under the same lock just before the mutation
applies) and report which mutation failed through failed_mutation on the
response, so callers can tell a mutation guard's rejection apart from the
request-level precondition's.

* s3: evaluate multipart completion conditions under the object lock

CompleteMultipartUpload checked conditional headers gateway-side before
committing, and the routed commit transaction guarded only the upload
directory — nothing made If-None-Match: * atomic with the publish, so
concurrent conditional completions of the same absent key could all
report success.

The request's reducible write condition (If-None-Match/If-Match, star or
single strong ETag) now rides the commit as the transaction's request
condition on the owner filer: on the object path for unversioned buckets,
falling back to the .versions directory for suspended and enabled
versioning. The upload-exists guard moves onto the PUT mutation, keeping
its check-before-publish ordering. Conditions the reducer cannot express
stay on the distributed write lock, where check and commit already
serialize.

* filer,s3: resolve the latest version first for versioned conditional writes

* filer: keep directory entries holding content out of the absent-slot rule

* s3: keep a gateway-side upload-exists check for filers without mutation conditions

* filer: honor null-version-is-latest resolving prefer-alt conditions

* drop scratch multipart-race repro programs

* s3: note the write lock does not cover routed writes on non-reducible conditions
This commit is contained in:
Chris Lu authored and GitHub committed 2026-10-29 11:46:25 -07:00
1 parent 3955e7218f
commit 2737315e9d
12 files changed
+670 -212

No files matched your search

+17 -10
View File
@@ -514,7 +514,7 @@ func TestRoutedMultipartFinalize(t *testing.T) {
chunks := []*filer_pb.FileChunk{{FileId: "1,01637037d6"}}
code := s3a.routedMultipartFinalize(owner, "b", "obj", false, "/buckets/b/obj.versions", "v_1", chunks, func(entry *filer_pb.Entry) {
entry.Extended = map[string][]byte{s3_constants.SeaweedFSUploadId: []byte("up1")}
}, "up1")
}, "up1", clause(filer_pb.WriteCondition_IF_NOT_EXISTS))
if code != s3err.ErrNone {
t.Fatalf("routedMultipartFinalize = %v", code)
}
@@ -529,10 +529,15 @@ func TestRoutedMultipartFinalize(t *testing.T) {
if req.RouteKey != "s3.object.write:/buckets/b/obj" {
t.Fatalf("RouteKey = %q", req.RouteKey)
}
if req.ConditionKey != "/buckets/b/.uploads/up1" {
// The request condition guards the object (at lock_key, falling back to
// .versions); the upload directory rides the PUT as a mutation guard.
if req.ConditionKey != "" {
t.Fatalf("ConditionKey = %q", req.ConditionKey)
}
if req.Condition == nil || len(req.Condition.Clauses) != 1 || req.Condition.Clauses[0].Kind != filer_pb.WriteCondition_IF_EXISTS {
if req.ConditionAltKey != "/buckets/b/obj.versions" {
t.Fatalf("ConditionAltKey = %q", req.ConditionAltKey)
}
if req.Condition == nil || len(req.Condition.Clauses) != 1 || req.Condition.Clauses[0].Kind != filer_pb.WriteCondition_IF_NOT_EXISTS {
t.Fatalf("Condition = %+v", req.Condition)
}
if len(req.Mutations) != 3 {
@@ -548,6 +553,10 @@ func TestRoutedMultipartFinalize(t *testing.T) {
string(put.Entry.Extended[s3_constants.SeaweedFSUploadId]) != "up1" {
t.Fatalf("put mutation = %+v", put)
}
if put.ConditionKey != "/buckets/b/.uploads/up1" ||
put.Condition == nil || len(put.Condition.Clauses) != 1 || put.Condition.Clauses[0].Kind != filer_pb.WriteCondition_IF_EXISTS {
t.Fatalf("put condition = %+v key %q", put.Condition, put.ConditionKey)
}
removeUpload := req.Mutations[1]
if removeUpload.Type != filer_pb.ObjectMutation_DELETE ||
removeUpload.Directory != "/buckets/b/.uploads" ||
@@ -580,7 +589,7 @@ func TestWriteMultipartObjectRemovesUploadDir(t *testing.T) {
if err != nil {
t.Fatalf("routedUploadRemoval: %v", err)
}
if err := s3a.writeMultipartObject(owner, "s3.object.write:/buckets/b/o", "/buckets/b", "o", nil, nil, removal); err != nil {
if err := s3a.writeMultipartObject(owner, "s3.object.write:/buckets/b/o", "/buckets/b", "o", nil, nil, removal, nil, ""); err != nil {
t.Fatalf("writeMultipartObject: %v", err)
}
@@ -591,12 +600,6 @@ func TestWriteMultipartObjectRemovesUploadDir(t *testing.T) {
if req.LockKey != "/buckets/b/o" {
t.Fatalf("LockKey = %q", req.LockKey)
}
if req.ConditionKey != "/buckets/b/.uploads/up1" {
t.Fatalf("ConditionKey = %q", req.ConditionKey)
}
if req.Condition == nil || len(req.Condition.Clauses) != 1 || req.Condition.Clauses[0].Kind != filer_pb.WriteCondition_IF_EXISTS {
t.Fatalf("Condition = %+v", req.Condition)
}
if len(req.Mutations) != 2 {
t.Fatalf("mutations = %d, want 2", len(req.Mutations))
}
@@ -604,6 +607,10 @@ func TestWriteMultipartObjectRemovesUploadDir(t *testing.T) {
if put.Type != filer_pb.ObjectMutation_PUT || put.Directory != "/buckets/b" || put.Entry == nil || put.Entry.Name != "o" {
t.Fatalf("put mutation = %+v", put)
}
if put.ConditionKey != "/buckets/b/.uploads/up1" ||
put.Condition == nil || len(put.Condition.Clauses) != 1 || put.Condition.Clauses[0].Kind != filer_pb.WriteCondition_IF_EXISTS {
t.Fatalf("put condition = %+v key %q", put.Condition, put.ConditionKey)
}
removeUpload := req.Mutations[1]
if removeUpload.Type != filer_pb.ObjectMutation_DELETE ||
removeUpload.Directory != "/buckets/b/.uploads" ||