From cf17fc77747ee8cfbf6b7b2d7dd6f6fa21b0c67b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Sat, 3 Dec 2016 11:53:12 -0800 Subject: [PATCH] fs: PutObject create 0byte objects properly. (#3387) Current code always appends to a file only if 1byte or more was sent on the wire was affecting both PutObject and PutObjectPart uploads. This patch fixes such a situation and resolves #3385 --- cmd/fs-createfile.go | 8 ++--- cmd/fs-v1-multipart.go | 1 + cmd/fs-v1-multipart_test.go | 10 +++++-- cmd/fs-v1.go | 58 ++++++++++++++++--------------------- 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/cmd/fs-createfile.go b/cmd/fs-createfile.go index a0ec0a6c7..db0afd017 100644 --- a/cmd/fs-createfile.go +++ b/cmd/fs-createfile.go @@ -27,11 +27,9 @@ func fsCreateFile(disk StorageAPI, reader io.Reader, buf []byte, tmpBucket, temp return 0, traceError(rErr) } bytesWritten += int64(n) - if n > 0 { - wErr := disk.AppendFile(tmpBucket, tempObj, buf[0:n]) - if wErr != nil { - return 0, traceError(wErr) - } + wErr := disk.AppendFile(tmpBucket, tempObj, buf[0:n]) + if wErr != nil { + return 0, traceError(wErr) } if rErr == io.EOF { break diff --git a/cmd/fs-v1-multipart.go b/cmd/fs-v1-multipart.go index 6003d66fe..c4718a3ad 100644 --- a/cmd/fs-v1-multipart.go +++ b/cmd/fs-v1-multipart.go @@ -301,6 +301,7 @@ func (fs fsObjects) PutObjectPart(bucket, object, uploadID string, partID int, s fs.storage.DeleteFile(minioMetaTmpBucket, tmpPartPath) return "", toObjectErr(cErr, minioMetaTmpBucket, tmpPartPath) } + // Should return IncompleteBody{} error when reader has fewer // bytes than specified in request header. if bytesWritten < size { diff --git a/cmd/fs-v1-multipart_test.go b/cmd/fs-v1-multipart_test.go index 29d9311e0..52c30d7c6 100644 --- a/cmd/fs-v1-multipart_test.go +++ b/cmd/fs-v1-multipart_test.go @@ -59,6 +59,12 @@ func TestNewMultipartUploadFaultyDisk(t *testing.T) { // TestPutObjectPartFaultyDisk - test PutObjectPart with faulty disks func TestPutObjectPartFaultyDisk(t *testing.T) { + root, err := newTestConfig("us-east-1") + if err != nil { + t.Fatal(err) + } + defer removeAll(root) + // Prepare for tests disk := filepath.Join(os.TempDir(), "minio-"+nextSuffix()) defer removeAll(disk) @@ -69,7 +75,7 @@ func TestPutObjectPartFaultyDisk(t *testing.T) { data := []byte("12345") dataLen := int64(len(data)) - if err := obj.MakeBucket(bucketName); err != nil { + if err = obj.MakeBucket(bucketName); err != nil { t.Fatal("Cannot create bucket, err: ", err) } @@ -97,7 +103,7 @@ func TestPutObjectPartFaultyDisk(t *testing.T) { t.Fatal("Unexpected error ", err) } case 3: - case 2, 4, 5: + case 2, 4, 5, 6: if !isSameType(errorCause(err), InvalidUploadID{}) { t.Fatal("Unexpected error ", err) } diff --git a/cmd/fs-v1.go b/cmd/fs-v1.go index 906ec6818..4d3d933b6 100644 --- a/cmd/fs-v1.go +++ b/cmd/fs-v1.go @@ -392,45 +392,37 @@ func (fs fsObjects) PutObject(bucket string, object string, size int64, data io. limitDataReader = data } - if size == 0 { - // For size 0 we write a 0byte file. - err = fs.storage.AppendFile(minioMetaTmpBucket, tempObj, []byte("")) + // Prepare file to avoid disk fragmentation + if size > 0 { + err = fs.storage.PrepareFile(minioMetaTmpBucket, tempObj, size) if err != nil { - fs.storage.DeleteFile(minioMetaTmpBucket, tempObj) - return ObjectInfo{}, toObjectErr(traceError(err), bucket, object) + return ObjectInfo{}, toObjectErr(err, bucket, object) } - } else { + } - // Prepare file to avoid disk fragmentation - if size > 0 { - err = fs.storage.PrepareFile(minioMetaTmpBucket, tempObj, size) - if err != nil { - return ObjectInfo{}, toObjectErr(err, bucket, object) - } - } + // Allocate a buffer to Read() from request body + bufSize := int64(readSizeV1) + if size > 0 && bufSize > size { + bufSize = size + } - // Allocate a buffer to Read() from request body - bufSize := int64(readSizeV1) - if size > 0 && bufSize > size { - bufSize = size - } - buf := make([]byte, int(bufSize)) - teeReader := io.TeeReader(limitDataReader, multiWriter) - var bytesWritten int64 - bytesWritten, err = fsCreateFile(fs.storage, teeReader, buf, minioMetaTmpBucket, tempObj) - if err != nil { - fs.storage.DeleteFile(minioMetaTmpBucket, tempObj) - errorIf(err, "Failed to create object %s/%s", bucket, object) - return ObjectInfo{}, toObjectErr(err, bucket, object) - } + buf := make([]byte, int(bufSize)) + teeReader := io.TeeReader(limitDataReader, multiWriter) + var bytesWritten int64 + bytesWritten, err = fsCreateFile(fs.storage, teeReader, buf, minioMetaTmpBucket, tempObj) + if err != nil { + fs.storage.DeleteFile(minioMetaTmpBucket, tempObj) + errorIf(err, "Failed to create object %s/%s", bucket, object) + return ObjectInfo{}, toObjectErr(err, bucket, object) + } - // Should return IncompleteBody{} error when reader has fewer - // bytes than specified in request header. - if bytesWritten < size { - fs.storage.DeleteFile(minioMetaTmpBucket, tempObj) - return ObjectInfo{}, traceError(IncompleteBody{}) - } + // Should return IncompleteBody{} error when reader has fewer + // bytes than specified in request header. + if bytesWritten < size { + fs.storage.DeleteFile(minioMetaTmpBucket, tempObj) + return ObjectInfo{}, traceError(IncompleteBody{}) } + // Delete the temporary object in the case of a // failure. If PutObject succeeds, then there would be // nothing to delete.