Unlock read lock on uploadID upon errors (#6283)

This commit is contained in:
Harshavardhana 2018-08-14 18:35:30 -07:00 committed by GitHub
parent 0286e61aee
commit 380524ae27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 9 deletions

View File

@ -314,10 +314,15 @@ func (e InvalidUploadID) Error() string {
} }
// InvalidPart One or more of the specified parts could not be found // InvalidPart One or more of the specified parts could not be found
type InvalidPart struct{} type InvalidPart struct {
PartNumber int
ExpETag string
GotETag string
}
func (e InvalidPart) Error() string { func (e InvalidPart) Error() string {
return "One or more of the specified parts could not be found. The part may not have been uploaded, or the specified entity tag may not match the part's entity tag." return fmt.Sprintf("Specified part could not be found. PartNumber %d, Expected %s, got %s",
e.PartNumber, e.ExpETag, e.GotETag)
} }
// PartTooSmall - error if part size is less than 5MB. // PartTooSmall - error if part size is less than 5MB.

View File

@ -19,8 +19,10 @@ package cmd
import ( import (
"bytes" "bytes"
"context" "context"
"encoding/hex"
"fmt" "fmt"
"os" "os"
"reflect"
"strings" "strings"
"testing" "testing"
@ -1877,8 +1879,8 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T
// Asserting for Invalid UploadID (Test number 9). // Asserting for Invalid UploadID (Test number 9).
{bucketNames[0], objectNames[0], "abc", []CompletePart{}, "", InvalidUploadID{UploadID: "abc"}, false}, {bucketNames[0], objectNames[0], "abc", []CompletePart{}, "", InvalidUploadID{UploadID: "abc"}, false},
// Test case with invalid Part Etag (Test number 10-11). // Test case with invalid Part Etag (Test number 10-11).
{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abc"}}, "", fmt.Errorf("encoding/hex: odd length hex string"), false}, {bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abc"}}, "", hex.ErrLength, false},
{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcz"}}, "", fmt.Errorf("encoding/hex: invalid byte: U+007A 'z'"), false}, {bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcz"}}, "", hex.InvalidByteError(00), false},
// Part number 0 doesn't exist, expecting InvalidPart error (Test number 12). // Part number 0 doesn't exist, expecting InvalidPart error (Test number 12).
{bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcd", PartNumber: 0}}, "", InvalidPart{}, false}, {bucketNames[0], objectNames[0], uploadIDs[0], []CompletePart{{ETag: "abcd", PartNumber: 0}}, "", InvalidPart{}, false},
// // Upload and PartNumber exists, But a deliberate ETag mismatch is introduced (Test number 13). // // Upload and PartNumber exists, But a deliberate ETag mismatch is introduced (Test number 13).
@ -1909,7 +1911,7 @@ func testObjectCompleteMultipartUpload(obj ObjectLayer, instanceType string, t T
} }
// Failed as expected, but does it fail for the expected reason. // Failed as expected, but does it fail for the expected reason.
if actualErr != nil && !testCase.shouldPass { if actualErr != nil && !testCase.shouldPass {
if !strings.Contains(actualErr.Error(), testCase.expectedErr.Error()) { if reflect.TypeOf(actualErr) != reflect.TypeOf(testCase.expectedErr) {
t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but instead failed with error \"%s\"", i+1, instanceType, testCase.expectedErr, actualErr) t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but instead failed with error \"%s\"", i+1, instanceType, testCase.expectedErr, actualErr)
} }
} }

View File

@ -331,6 +331,7 @@ func (xl xlObjects) PutObjectPart(ctx context.Context, bucket, object, uploadID
// get Quorum for this object // get Quorum for this object
_, writeQuorum, err := objectQuorumFromMeta(ctx, xl, partsMetadata, errs) _, writeQuorum, err := objectQuorumFromMeta(ctx, xl, partsMetadata, errs)
if err != nil { if err != nil {
preUploadIDLock.RUnlock()
return pi, toObjectErr(err, bucket, object) return pi, toObjectErr(err, bucket, object)
} }
@ -665,14 +666,23 @@ func (xl xlObjects) CompleteMultipartUpload(ctx context.Context, bucket string,
partIdx := objectPartIndex(currentXLMeta.Parts, part.PartNumber) partIdx := objectPartIndex(currentXLMeta.Parts, part.PartNumber)
// All parts should have same part number. // All parts should have same part number.
if partIdx == -1 { if partIdx == -1 {
logger.LogIf(ctx, InvalidPart{}) invp := InvalidPart{
return oi, InvalidPart{} PartNumber: part.PartNumber,
GotETag: part.ETag,
}
logger.LogIf(ctx, invp)
return oi, invp
} }
// All parts should have same ETag as previously generated. // All parts should have same ETag as previously generated.
if currentXLMeta.Parts[partIdx].ETag != part.ETag { if currentXLMeta.Parts[partIdx].ETag != part.ETag {
logger.LogIf(ctx, InvalidPart{}) invp := InvalidPart{
return oi, InvalidPart{} PartNumber: part.PartNumber,
ExpETag: currentXLMeta.Parts[partIdx].ETag,
GotETag: part.ETag,
}
logger.LogIf(ctx, invp)
return oi, invp
} }
// All parts except the last part has to be atleast 5MB. // All parts except the last part has to be atleast 5MB.