From 27ef1262bf851089fd10c5eaccbf5b377be5b259 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Thu, 25 Apr 2019 15:33:26 +0100 Subject: [PATCH] xl: Use random UUID during complete multipart upload (#7527) One user has seen this following error log: API: CompleteMultipartUpload(bucket=vertica, object=perf-dss-v03/cc2/02596813aecd4e476d810148586c2a3300d00000013557ef_0.gt) Time: 15:44:07 UTC 04/11/2019 RequestID: 159475EFF4DEDFFB RemoteHost: 172.26.87.184 UserAgent: vertica-v9.1.1-5 Error: open /data/.minio.sys/tmp/100bb3ec-6c0d-4a37-8b36-65241050eb02/xl.json: file exists 1: cmd/xl-v1-metadata.go:448:cmd.writeXLMetadata() 2: cmd/xl-v1-metadata.go:501:cmd.writeUniqueXLMetadata.func1() This can happen when CompleteMultipartUpload fails with write quorum, the S3 client will retry (since write quorum is 500 http response), however the second call of CompleteMultipartUpload will fail because this latter doesn't truly use a random uuid under .minio.sys/tmp/ directory but pick the upload id. This commit fixes the behavior to choose a random uuid for generating xl.json --- cmd/xl-v1-healing.go | 4 ++++ cmd/xl-v1-metadata.go | 37 ------------------------------------- cmd/xl-v1-multipart.go | 32 +++++++++++++++++--------------- cmd/xl-v1-object.go | 3 +++ 4 files changed, 24 insertions(+), 52 deletions(-) diff --git a/cmd/xl-v1-healing.go b/cmd/xl-v1-healing.go index cf8eabca4..380397998 100644 --- a/cmd/xl-v1-healing.go +++ b/cmd/xl-v1-healing.go @@ -425,6 +425,10 @@ func (xl xlObjects) healObject(ctx context.Context, bucket string, object string } } + // Cleanup in case of xl.json writing failure + writeQuorum := latestMeta.Erasure.DataBlocks + 1 + defer xl.deleteObject(ctx, minioMetaTmpBucket, tmpID, writeQuorum, false) + // Generate and write `xl.json` generated from other disks. outDatedDisks, aErr := writeUniqueXLMetadata(ctx, outDatedDisks, minioMetaTmpBucket, tmpID, partsMetadata, diskCount(outDatedDisks)) diff --git a/cmd/xl-v1-metadata.go b/cmd/xl-v1-metadata.go index c60cbc5ac..734e20718 100644 --- a/cmd/xl-v1-metadata.go +++ b/cmd/xl-v1-metadata.go @@ -425,14 +425,6 @@ func (xl xlObjects) readXLMetaStat(ctx context.Context, bucket, object string) ( return statInfo{}, nil, reduceReadQuorumErrs(ctx, ignoredErrs, nil, readQuorum) } -// deleteXLMetadata - deletes `xl.json` on a single disk. -func deleteXLMetdata(ctx context.Context, disk StorageAPI, bucket, prefix string) error { - jsonFile := path.Join(prefix, xlMetaJSONFile) - err := disk.DeleteFile(bucket, jsonFile) - logger.LogIf(ctx, err) - return err -} - // writeXLMetadata - writes `xl.json` to a single disk. func writeXLMetadata(ctx context.Context, disk StorageAPI, bucket, prefix string, xlMeta xlMetaV1) error { jsonFile := path.Join(prefix, xlMetaJSONFile) @@ -450,27 +442,6 @@ func writeXLMetadata(ctx context.Context, disk StorageAPI, bucket, prefix string return err } -// deleteAllXLMetadata - deletes all partially written `xl.json` depending on errs. -func deleteAllXLMetadata(ctx context.Context, disks []StorageAPI, bucket, prefix string, errs []error) { - var wg = &sync.WaitGroup{} - // Delete all the `xl.json` left over. - for index, disk := range disks { - if disk == nil { - continue - } - // Undo rename object in parallel. - wg.Add(1) - go func(index int, disk StorageAPI) { - defer wg.Done() - if errs[index] != nil { - return - } - _ = deleteXLMetdata(ctx, disk, bucket, prefix) - }(index, disk) - } - wg.Wait() -} - // Rename `xl.json` content to destination location for each disk in order. func renameXLMetadata(ctx context.Context, disks []StorageAPI, srcBucket, srcEntry, dstBucket, dstEntry string, quorum int) ([]StorageAPI, error) { isDir := false @@ -510,10 +481,6 @@ func writeUniqueXLMetadata(ctx context.Context, disks []StorageAPI, bucket, pref wg.Wait() err := reduceWriteQuorumErrs(ctx, mErrs, objectOpIgnoredErrs, quorum) - if err == errXLWriteQuorum { - // Delete all `xl.json` successfully renamed. - deleteAllXLMetadata(ctx, disks, bucket, prefix, mErrs) - } return evalDisks(disks, mErrs), err } @@ -548,9 +515,5 @@ func writeSameXLMetadata(ctx context.Context, disks []StorageAPI, bucket, prefix wg.Wait() err := reduceWriteQuorumErrs(ctx, mErrs, objectOpIgnoredErrs, writeQuorum) - if err == errXLWriteQuorum { - // Delete all `xl.json` successfully renamed. - deleteAllXLMetadata(ctx, disks, bucket, prefix, mErrs) - } return evalDisks(disks, mErrs), err } diff --git a/cmd/xl-v1-multipart.go b/cmd/xl-v1-multipart.go index f59554be7..32f2ebd87 100644 --- a/cmd/xl-v1-multipart.go +++ b/cmd/xl-v1-multipart.go @@ -136,10 +136,6 @@ func commitXLMetadata(ctx context.Context, disks []StorageAPI, srcBucket, srcPre wg.Wait() err := reduceWriteQuorumErrs(ctx, mErrs, objectOpIgnoredErrs, quorum) - if err == errXLWriteQuorum { - // Delete all `xl.json` successfully renamed. - deleteAllXLMetadata(ctx, disks, dstBucket, dstPrefix, mErrs) - } return evalDisks(disks, mErrs), err } @@ -217,15 +213,16 @@ func (xl xlObjects) newMultipartUpload(ctx context.Context, bucket string, objec uploadIDPath := xl.getUploadIDDir(bucket, object, uploadID) tempUploadIDPath := uploadID + // Delete the tmp path later in case we fail to commit (ignore + // returned errors) - this will be a no-op in case of a commit + // success. + defer xl.deleteObject(ctx, minioMetaTmpBucket, tempUploadIDPath, writeQuorum, false) + // Write updated `xl.json` to all disks. disks, err := writeSameXLMetadata(ctx, xl.getDisks(), minioMetaTmpBucket, tempUploadIDPath, xlMeta, writeQuorum) if err != nil { return "", toObjectErr(err, minioMetaTmpBucket, tempUploadIDPath) } - // delete the tmp path later in case we fail to rename (ignore - // returned errors) - this will be a no-op in case of a rename - // success. - defer xl.deleteObject(ctx, minioMetaTmpBucket, tempUploadIDPath, writeQuorum, false) // Attempt to rename temp upload object to actual upload path object _, rErr := rename(ctx, disks, minioMetaTmpBucket, tempUploadIDPath, minioMetaMultipartBucket, uploadIDPath, true, writeQuorum, nil) @@ -453,8 +450,10 @@ func (xl xlObjects) PutObjectPart(ctx context.Context, bucket, object, uploadID } // Write all the checksum metadata. - newUUID := mustGetUUID() - tempXLMetaPath := newUUID + tempXLMetaPath := mustGetUUID() + + // Cleanup in case of xl.json writing failure + defer xl.deleteObject(ctx, minioMetaTmpBucket, tempXLMetaPath, writeQuorum, false) // Writes a unique `xl.json` each disk carrying new checksum related information. if onlineDisks, err = writeUniqueXLMetadata(ctx, onlineDisks, minioMetaTmpBucket, tempXLMetaPath, partsMetadata, writeQuorum); err != nil { @@ -732,8 +731,6 @@ func (xl xlObjects) CompleteMultipartUpload(ctx context.Context, bucket string, // Save the consolidated actual size. xlMeta.Meta[ReservedMetadataPrefix+"actual-size"] = strconv.FormatInt(objectActualSize, 10) - tempUploadIDPath := uploadID - // Update all xl metadata, make sure to not modify fields like // checksum which are different on each disks. for index := range partsMetadata { @@ -742,13 +739,18 @@ func (xl xlObjects) CompleteMultipartUpload(ctx context.Context, bucket string, partsMetadata[index].Parts = xlMeta.Parts } + tempXLMetaPath := mustGetUUID() + + // Cleanup in case of failure + defer xl.deleteObject(ctx, minioMetaTmpBucket, tempXLMetaPath, writeQuorum, false) + // Write unique `xl.json` for each disk. - if onlineDisks, err = writeUniqueXLMetadata(ctx, onlineDisks, minioMetaTmpBucket, tempUploadIDPath, partsMetadata, writeQuorum); err != nil { - return oi, toObjectErr(err, minioMetaTmpBucket, tempUploadIDPath) + if onlineDisks, err = writeUniqueXLMetadata(ctx, onlineDisks, minioMetaTmpBucket, tempXLMetaPath, partsMetadata, writeQuorum); err != nil { + return oi, toObjectErr(err, minioMetaTmpBucket, tempXLMetaPath) } var rErr error - onlineDisks, rErr = commitXLMetadata(ctx, onlineDisks, minioMetaTmpBucket, tempUploadIDPath, minioMetaMultipartBucket, uploadIDPath, writeQuorum) + onlineDisks, rErr = commitXLMetadata(ctx, onlineDisks, minioMetaTmpBucket, tempXLMetaPath, minioMetaMultipartBucket, uploadIDPath, writeQuorum) if rErr != nil { return oi, toObjectErr(rErr, minioMetaMultipartBucket, uploadIDPath) } diff --git a/cmd/xl-v1-object.go b/cmd/xl-v1-object.go index 5349a0909..c01d61a40 100644 --- a/cmd/xl-v1-object.go +++ b/cmd/xl-v1-object.go @@ -103,6 +103,9 @@ func (xl xlObjects) CopyObject(ctx context.Context, srcBucket, srcObject, dstBuc tempObj := mustGetUUID() + // Cleanup in case of xl.json writing failure + defer xl.deleteObject(ctx, minioMetaTmpBucket, tempObj, writeQuorum, false) + // Write unique `xl.json` for each disk. if onlineDisks, err = writeUniqueXLMetadata(ctx, storageDisks, minioMetaTmpBucket, tempObj, metaArr, writeQuorum); err != nil { return oi, toObjectErr(err, srcBucket, srcObject)