fix optimalPartInfo calculation (#1100)

- when client has not configured part size, use optimal part size determined by SDK. Otherwise honor the client specified part size fixes: #1099
minio · Apr 22, 2019 · a8704b6 · a8704b6
1 parent 80c2fe3
commit a8704b6
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 32 deletions.
diff --git a/api-put-object-common.go b/api-put-object-common.go
@@ -79,29 +79,36 @@ func optimalPartInfo(objectSize int64, configuredPartSize uint64) (totalPartsCou
 		return
 	}
 
-	if int64(configuredPartSize) > objectSize {
-		err = ErrEntityTooLarge(int64(configuredPartSize), objectSize, "", "")
-		return
-	}
+	var partSizeFlt float64
+	if configuredPartSize > 0 {
+		if int64(configuredPartSize) > objectSize {
+			err = ErrEntityTooLarge(int64(configuredPartSize), objectSize, "", "")
+			return
+		}
 
-	if objectSize > (int64(configuredPartSize) * maxPartsCount) {
-		err = ErrInvalidArgument("Part size * max_parts(10000) is lesser than input objectSize.")
-	}
+		if objectSize > (int64(configuredPartSize) * maxPartsCount) {
+			err = ErrInvalidArgument("Part size * max_parts(10000) is lesser than input objectSize.")
+			return
+		}
 
-	if configuredPartSize < absMinPartSize {
-		err = ErrInvalidArgument("Input part size is smaller than allowed minimum of 5MiB.")
-		return
-	}
+		if configuredPartSize < absMinPartSize {
+			err = ErrInvalidArgument("Input part size is smaller than allowed minimum of 5MiB.")
+			return
+		}
 
-	if configuredPartSize > maxPartSize {
-		err = ErrInvalidArgument("Input part size is bigger than allowed maximum of 5GiB.")
-		return
+		if configuredPartSize > maxPartSize {
+			err = ErrInvalidArgument("Input part size is bigger than allowed maximum of 5GiB.")
+			return
+		}
+		partSizeFlt = float64(configuredPartSize)
+	} else {
+		configuredPartSize = minPartSize
+		// Use floats for part size for all calculations to avoid
+		// overflows during float64 to int64 conversions.
+		partSizeFlt = math.Ceil(float64(objectSize / maxPartsCount))
+		partSizeFlt = math.Ceil(partSizeFlt/float64(configuredPartSize)) * float64(configuredPartSize)
 	}
 
-	// Use floats for part size for all calculations to avoid
-	// overflows during float64 to int64 conversions.
-	partSizeFlt := math.Ceil(float64(objectSize / maxPartsCount))
-	partSizeFlt = math.Ceil(partSizeFlt/float64(configuredPartSize)) * float64(configuredPartSize)
 	// Total parts count.
 	totalPartsCount = int(math.Ceil(float64(objectSize) / partSizeFlt))
 	// Part size.

diff --git a/api-put-object.go b/api-put-object.go
@@ -148,12 +148,13 @@ func (c Client) putObjectCommon(ctx context.Context, bucketName, objectName stri
 		return c.putObjectNoChecksum(ctx, bucketName, objectName, reader, size, opts)
 	}
 
+	partSize := opts.PartSize
 	if opts.PartSize == 0 {
-		opts.PartSize = minPartSize
+		partSize = minPartSize
 	}
 
 	if c.overrideSignerType.IsV2() {
-		if size >= 0 && size < int64(opts.PartSize) {
+		if size >= 0 && size < int64(partSize) {
 			return c.putObjectNoChecksum(ctx, bucketName, objectName, reader, size, opts)
 		}
 		return c.putObjectMultipart(ctx, bucketName, objectName, reader, size, opts)
@@ -162,7 +163,7 @@ func (c Client) putObjectCommon(ctx context.Context, bucketName, objectName stri
 		return c.putObjectMultipartStreamNoLength(ctx, bucketName, objectName, reader, opts)
 	}
 
-	if size < int64(opts.PartSize) {
+	if size < int64(partSize) {
 		return c.putObjectNoChecksum(ctx, bucketName, objectName, reader, size, opts)
 	}
 

diff --git a/api_unit_test.go b/api_unit_test.go
@@ -120,18 +120,31 @@ func TestPartSize(t *testing.T) {
 	if err == nil {
 		t.Fatal("Error: should fail")
 	}
-	totalPartsCount, partSize, lastPartSize, err := optimalPartInfo(5497558138880, minPartSize)
+	totalPartsCount, partSize, lastPartSize, err := optimalPartInfo(5243928576, 5*1024*1024)
 	if err != nil {
 		t.Fatal("Error: ", err)
 	}
-	if totalPartsCount != 9103 {
-		t.Fatalf("Error: expecting total parts count of 9987: got %v instead", totalPartsCount)
+	if totalPartsCount != 1001 {
+		t.Fatalf("Error: expecting total parts count of 1001: got %v instead", totalPartsCount)
 	}
-	if partSize != 603979776 {
-		t.Fatalf("Error: expecting part size of 550502400: got %v instead", partSize)
+	if partSize != 5242880 {
+		t.Fatalf("Error: expecting part size of 5242880: got %v instead", partSize)
 	}
-	if lastPartSize != 134217728 {
-		t.Fatalf("Error: expecting last part size of 241172480: got %v instead", lastPartSize)
+	if lastPartSize != 1048576 {
+		t.Fatalf("Error: expecting last part size of 1048576: got %v instead", lastPartSize)
+	}
+	totalPartsCount, partSize, lastPartSize, err = optimalPartInfo(5243928576, 0)
+	if err != nil {
+		t.Fatal("Error: ", err)
+	}
+	if totalPartsCount != 79 {
+		t.Fatalf("Error: expecting total parts count of 79: got %v instead", totalPartsCount)
+	}
+	if partSize != 67108864 {
+		t.Fatalf("Error: expecting part size of 67108864: got %v instead", partSize)
+	}
+	if lastPartSize != 9437184 {
+		t.Fatalf("Error: expecting last part size of 9437184: got %v instead", lastPartSize)
 	}
 	_, partSize, _, err = optimalPartInfo(5000000000, minPartSize)
 	if err != nil {
@@ -140,18 +153,24 @@ func TestPartSize(t *testing.T) {
 	if partSize != minPartSize {
 		t.Fatalf("Error: expecting part size of %v: got %v instead", minPartSize, partSize)
 	}
-	totalPartsCount, partSize, lastPartSize, err = optimalPartInfo(-1, minPartSize)
+	// if stream and client configured min part size
+	_, _, _, err = optimalPartInfo(-1, minPartSize)
+	if err == nil {
+		t.Fatal("Error:", err)
+	}
+	// if stream and using default optimal part size determined by sdk
+	totalPartsCount, partSize, lastPartSize, err = optimalPartInfo(-1, 0)
 	if err != nil {
 		t.Fatal("Error:", err)
 	}
 	if totalPartsCount != 9103 {
-		t.Fatalf("Error: expecting total parts count of 9987: got %v instead", totalPartsCount)
+		t.Fatalf("Error: expecting total parts count of 9103: got %v instead", totalPartsCount)
 	}
 	if partSize != 603979776 {
-		t.Fatalf("Error: expecting part size of 550502400: got %v instead", partSize)
+		t.Fatalf("Error: expecting part size of 603979776: got %v instead", partSize)
 	}
 	if lastPartSize != 134217728 {
-		t.Fatalf("Error: expecting last part size of 241172480: got %v instead", lastPartSize)
+		t.Fatalf("Error: expecting last part size of 134217728: got %v instead", lastPartSize)
 	}
 }