fix(restore): avoid duplicate completion locks for empty upload IDs

Keep the storage layer error precedence for missing buckets and uploads while retaining every real upload lock. Cover the alias with short deadlines and race-check the original complete-multipart suite in CI.

Signed-off-by: Feng Ruohang <rh@vonng.com>

.github/workflows/go.yml

@@ -99,7 +99,7 @@ jobs:
       - name: Run multipart listing, cancellation and cleanup tests under race detector
         env:
           MINIO_API_REQUESTS_MAX: "10000"
-        run: go test -race ./cmd -run '^Test(Multipart|PaginateMultipartUploads|ListMultipartUploads|CleanupStaleUploads|ExtractModTime)' -count=1 -timeout=5m
+        run: go test -race ./cmd -run '^Test(ObjectCompleteMultipartUpload|Multipart|PaginateMultipartUploads|ListMultipartUploads|CleanupStaleUploads|ExtractModTime)' -count=1 -timeout=5m
 
       - name: Run listing metadata recovery, pagination and cancellation tests under race detector
         run: go test -race ./cmd -run '^Test(ListFallback|ListingCache|ListingMerge|ListingRecovery|ListingPending|ListingEntry|ListingSecondLock|StreamMetadataParts|LocalLockerListing|DistributedListing|MergeXLV2LatestNullCrossParity|CountXLV2MixedECGroups|ListCrossParity|ListObjectVersionsCrossParityNull|MigrationCrossParity)' -count=1 -timeout=5m

cmd/erasure-server-pool.go

@@ -2391,13 +2391,17 @@ func (z *erasureServerPools) CompleteMultipartUpload(ctx context.Context, bucket
 
 	// Hold write locks to verify uploaded parts, also disallows any
 	// parallel PutObjectPart() requests.
-	uploadIDLock := z.NewNSLock(bucket, pathJoin(object, uploadID))
-	wlkctx, err := uploadIDLock.GetLock(ctx, globalOperationTimeout)
-	if err != nil {
-		return objInfo, err
+	// An empty upload ID aliases the object lock already held above.
+	// Let the storage layer return its normal missing bucket/upload error.
+	if uploadID != "" {
+		uploadIDLock := z.NewNSLock(bucket, pathJoin(object, uploadID))
+		wlkctx, err := uploadIDLock.GetLock(ctx, globalOperationTimeout)
+		if err != nil {
+			return objInfo, err
+		}
+		ctx = wlkctx.Context()
+		defer uploadIDLock.Unlock(wlkctx)
 	}
-	ctx = wlkctx.Context()
-	defer uploadIDLock.Unlock(wlkctx)
 
 	if z.SinglePool() {
 		return z.serverPools[0].CompleteMultipartUpload(ctx, bucket, object, uploadID, uploadedParts, opts)

cmd/restore-native_test.go

@@ -707,3 +707,35 @@ func (r *nativeRestorePartGateReader) Read(p []byte) (int, error) {
 	}
 	return r.ReadCloser.Read(p)
 }
+
+// Invalid upload paths must retain the backend's normal error precedence,
+// rather than reacquiring the object lock as the upload lock.
+func TestMultipartCompletionLockAlias(t *testing.T) {
+	for _, pools := range []int{1, 2} {
+		for _, id := range []string{"", ".", "./"} {
+			t.Run(fmt.Sprintf("pools=%d/id=%q", pools, id), func(t *testing.T) {
+				r := newRestoreSafetyTest(t, pools, versioning.Enabled)
+				for _, missingBucket := range []bool{false, true} {
+					bucket := r.bucket
+					if missingBucket {
+						bucket = "missing-bucket"
+					}
+					ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond)
+					_, err := r.z.CompleteMultipartUpload(ctx, bucket, r.object, id, nil, ObjectOptions{})
+					cancel()
+					if id != "" {
+						if !errors.As(err, &MalformedUploadID{}) {
+							t.Errorf("malformed upload: %v", err)
+						}
+					} else if missingBucket {
+						if !isErrBucketNotFound(err) {
+							t.Errorf("missing bucket: %v", err)
+						}
+					} else if !errors.As(err, &InvalidUploadID{}) {
+						t.Errorf("invalid upload: %v", err)
+					}
+				}
+			})
+		}
+	}
+}