xl: avoid sending Delete() remote call for fully successful runs

an optimization to avoid extra syscalls in PutObject(),
adds up to our PutObject response times.

cmd/erasure-multipart.go

@@ -276,7 +276,6 @@ func (er erasureObjects) ListMultipartUploads(ctx context.Context, bucket, objec
 // disks. `uploads.json` carries metadata regarding on-going multipart
 // operation(s) on the object.
 func (er erasureObjects) newMultipartUpload(ctx context.Context, bucket string, object string, opts ObjectOptions) (string, error) {
-
 	onlineDisks := er.getDisks()
 	parityBlocks := globalStorageClass.GetParityForSC(opts.UserDefined[xhttp.AmzStorageClass])
 	if parityBlocks <= 0 {
@@ -317,7 +316,12 @@ func (er erasureObjects) newMultipartUpload(ctx context.Context, bucket string,
 	// 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 er.deleteObject(context.Background(), minioMetaTmpBucket, tempUploadIDPath, writeQuorum)
+	var online int
+	defer func() {
+		if online != len(onlineDisks) {
+			er.deleteObject(context.Background(), minioMetaTmpBucket, tempUploadIDPath, writeQuorum)
+		}
+	}()
 
 	var partsMetadata = make([]FileInfo, len(onlineDisks))
 	for i := range onlineDisks {
@@ -339,6 +343,8 @@ func (er erasureObjects) newMultipartUpload(ctx context.Context, bucket string,
 		return "", toObjectErr(err, minioMetaMultipartBucket, uploadIDPath)
 	}
 
+	online = countOnlineDisks(onlineDisks)
+
 	// Return success.
 	return uploadID, nil
 }
@@ -441,7 +447,12 @@ func (er erasureObjects) PutObjectPart(ctx context.Context, bucket, object, uplo
 	tmpPartPath := pathJoin(tmpPart, partSuffix)
 
 	// Delete the temporary object part. If PutObjectPart succeeds there would be nothing to delete.
-	defer er.deleteObject(context.Background(), minioMetaTmpBucket, tmpPart, writeQuorum)
+	var online int
+	defer func() {
+		if online != len(onlineDisks) {
+			er.deleteObject(context.Background(), minioMetaTmpBucket, tmpPart, writeQuorum)
+		}
+	}()
 
 	erasure, err := NewErasure(ctx, fi.Erasure.DataBlocks, fi.Erasure.ParityBlocks, fi.Erasure.BlockSize)
 	if err != nil {
@@ -563,6 +574,8 @@ func (er erasureObjects) PutObjectPart(ctx context.Context, bucket, object, uplo
 		return pi, toObjectErr(err, minioMetaMultipartBucket, uploadIDPath)
 	}
 
+	online = countOnlineDisks(onlineDisks)
+
 	// Return success.
 	return PartInfo{
 		PartNumber:   partID,

cmd/erasure-object.go

@@ -42,6 +42,15 @@ var objectOpIgnoredErrs = append(baseIgnoredErrs, errDiskAccessDenied, errUnform
 
 /// Object Operations
 
+func countOnlineDisks(onlineDisks []StorageAPI) (online int) {
+	for _, onlineDisk := range onlineDisks {
+		if onlineDisk != nil && onlineDisk.IsOnline() {
+			online++
+		}
+	}
+	return online
+}
+
 // CopyObject - copy object source object to destination object.
 // if source object and destination object are same we only
 // update metadata.
@@ -116,8 +125,13 @@ func (er erasureObjects) CopyObject(ctx context.Context, srcBucket, srcObject, d
 
 	tempObj := mustGetUUID()
 
+	var online int
 	// Cleanup in case of xl.meta writing failure
-	defer er.deleteObject(context.Background(), minioMetaTmpBucket, tempObj, writeQuorum)
+	defer func() {
+		if online != len(onlineDisks) {
+			er.deleteObject(context.Background(), minioMetaTmpBucket, tempObj, writeQuorum)
+		}
+	}()
 
 	// Write unique `xl.meta` for each disk.
 	if onlineDisks, err = writeUniqueFileInfo(ctx, onlineDisks, minioMetaTmpBucket, tempObj, metaArr, writeQuorum); err != nil {
@@ -129,6 +143,8 @@ func (er erasureObjects) CopyObject(ctx context.Context, srcBucket, srcObject, d
 		return oi, toObjectErr(err, srcBucket, srcObject)
 	}
 
+	online = countOnlineDisks(onlineDisks)
+
 	return fi.ToObjectInfo(srcBucket, srcObject), nil
 }
 
@@ -641,11 +657,6 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st
 		writeQuorum++
 	}
 
-	// Delete temporary object in the event of failure.
-	// If PutObject succeeded there would be no temporary
-	// object to delete.
-	defer er.deleteObject(context.Background(), minioMetaTmpBucket, tempObj, writeQuorum)
-
 	// Validate input data size and it can never be less than zero.
 	if data.Size() < -1 {
 		logger.LogIf(ctx, errInvalidArgument, logger.Application)
@@ -713,6 +724,16 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st
 	partName := "part.1"
 	tempErasureObj := pathJoin(uniqueID, fi.DataDir, partName)
 
+	// Delete temporary object in the event of failure.
+	// If PutObject succeeded there would be no temporary
+	// object to delete.
+	var online int
+	defer func() {
+		if online != len(onlineDisks) {
+			er.deleteObject(context.Background(), minioMetaTmpBucket, tempObj, writeQuorum)
+		}
+	}()
+
 	writers := make([]io.Writer, len(onlineDisks))
 	for i, disk := range onlineDisks {
 		if disk == nil {
@@ -806,6 +827,7 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st
 			break
 		}
 	}
+	online = countOnlineDisks(onlineDisks)
 
 	return fi.ToObjectInfo(bucket, object), nil
 }
@@ -859,8 +881,8 @@ func (er erasureObjects) deleteObject(ctx context.Context, bucket, object string
 	var err error
 	defer ObjectPathUpdated(pathJoin(bucket, object))
 
-	tmpObj := mustGetUUID()
 	disks := er.getDisks()
+	tmpObj := mustGetUUID()
 	if bucket == minioMetaTmpBucket {
 		tmpObj = object
 	} else {

cmd/http-stats.go

@@ -17,7 +17,6 @@
 package cmd
 
 import (
-	"context"
 	"net/http"
 	"strings"
 	"sync"
@@ -177,15 +176,15 @@ func (st *HTTPStats) updateStats(api string, r *http.Request, w *logger.Response
 		!strings.HasSuffix(r.URL.Path, prometheusMetricsV2ClusterPath) ||
 		!strings.HasSuffix(r.URL.Path, prometheusMetricsV2NodePath) {
 		st.totalS3Requests.Inc(api)
-		if !successReq && w.StatusCode != 0 {
+		if !successReq {
+			switch w.StatusCode {
+			case 0:
+			case 499:
+				// 499 is a good error, shall be counted at canceled.
+				st.totalS3Canceled.Inc(api)
+			default:
 				st.totalS3Errors.Inc(api)
 			}
-		select {
-		case <-r.Context().Done():
-			if err := r.Context().Err(); err == context.Canceled {
-				st.totalS3Canceled.Inc(api)
-			}
-		default:
 		}
 	}
 

cmd/metrics-v2.go

@@ -107,7 +107,7 @@ const (
 	versionInfo MetricName = "version_info"
 
 	sizeDistribution = "size_distribution"
-	ttfbDistribution = "ttbf_seconds_distribution"
+	ttfbDistribution = "ttfb_seconds_distribution"
 
 	lastActivityTime = "last_activity_nano_seconds"
 	startTime        = "starttime_seconds"

cmd/xl-storage.go

@@ -832,7 +832,7 @@ func (s *xlStorage) DeleteVersion(ctx context.Context, volume, path string, fi F
 	if !isXL2V1Format(buf) {
 		// Delete the meta file, if there are no more versions the
 		// top level parent is automatically removed.
-		return s.deleteFile(volumeDir, pathJoin(volumeDir, path), true, volume == minioMetaTmpBucket)
+		return s.deleteFile(volumeDir, pathJoin(volumeDir, path), true)
 	}
 
 	var xlMeta xlMetaV2
@@ -878,7 +878,7 @@ func (s *xlStorage) DeleteVersion(ctx context.Context, volume, path string, fi F
 		return err
 	}
 
-	return s.deleteFile(volumeDir, filePath, false, volume == minioMetaTmpBucket)
+	return s.deleteFile(volumeDir, filePath, false)
 }
 
 // WriteMetadata - writes FileInfo metadata for path at `xl.meta`
@@ -1664,7 +1664,7 @@ func (s *xlStorage) CheckFile(ctx context.Context, volume string, path string) e
 // move up the tree, deleting empty parent directories until it finds one
 // with files in it. Returns nil for a non-empty directory even when
 // recursive is set to false.
-func (s *xlStorage) deleteFile(basePath, deletePath string, recursive bool, tmpbucket bool) error {
+func (s *xlStorage) deleteFile(basePath, deletePath string, recursive bool) error {
 	if basePath == "" || deletePath == "" {
 		return nil
 	}
@@ -1677,11 +1677,7 @@ func (s *xlStorage) deleteFile(basePath, deletePath string, recursive bool, tmpb
 
 	var err error
 	if recursive {
-		if tmpbucket {
-			err = removeAll(deletePath)
-		} else {
 		err = renameAll(deletePath, pathutil.Join(s.diskPath, minioMetaTmpDeletedBucket, mustGetUUID()))
-		}
 	} else {
 		err = Remove(deletePath)
 	}
@@ -1712,7 +1708,7 @@ func (s *xlStorage) deleteFile(basePath, deletePath string, recursive bool, tmpb
 
 	// Delete parent directory obviously not recursively. Errors for
 	// parent directories shouldn't trickle down.
-	s.deleteFile(basePath, deletePath, false, tmpbucket)
+	s.deleteFile(basePath, deletePath, false)
 
 	return nil
 }
@@ -1745,7 +1741,7 @@ func (s *xlStorage) Delete(ctx context.Context, volume string, path string, recu
 	}
 
 	// Delete file and delete parent directory as well if it's empty.
-	return s.deleteFile(volumeDir, filePath, recursive, volume == minioMetaTmpBucket)
+	return s.deleteFile(volumeDir, filePath, recursive)
 }
 
 // RenameData - rename source path to destination path atomically, metadata and data directory.
@@ -1987,7 +1983,7 @@ func (s *xlStorage) RenameData(ctx context.Context, srcVolume, srcPath, dataDir,
 
 	// Remove parent dir of the source file if empty
 	parentDir := pathutil.Dir(srcFilePath)
-	s.deleteFile(srcVolumeDir, parentDir, false, srcVolume == minioMetaTmpBucket)
+	s.deleteFile(srcVolumeDir, parentDir, false)
 	return nil
 }
 
@@ -2066,7 +2062,7 @@ func (s *xlStorage) RenameFile(ctx context.Context, srcVolume, srcPath, dstVolum
 
 	// Remove parent dir of the source file if empty
 	parentDir := pathutil.Dir(srcFilePath)
-	s.deleteFile(srcVolumeDir, parentDir, false, srcVolume == minioMetaTmpBucket)
+	s.deleteFile(srcVolumeDir, parentDir, false)
 
 	return nil
 }