From 77e94087cf220f09b54a715685c6125986ddb046 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Tue, 10 Oct 2023 13:47:35 -0700
Subject: [PATCH] fix: calling statfs() call moves the disk head (#18203)

if erasure upgrade is needed rely on the in-memory
values, instead of performing a "DiskInfo()" call.

https://brendangregg.com/blog/2016-09-03/sudden-disk-busy.html

for HDDs these are problematic, lets avoid this because
there is no value in "being" absolutely strict here
in terms of parity. We are okay to increase parity
as we see based on the in-memory online/offline ratio.
---
 cmd/erasure-multipart.go | 39 ++++++++++++++-------------------------
 cmd/erasure-object.go    | 31 +++++++------------------------
 2 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go
index 5a5c19e33..373782c40 100644
--- a/cmd/erasure-multipart.go
+++ b/cmd/erasure-multipart.go
@@ -40,7 +40,6 @@ import (
 	"github.com/minio/minio/internal/logger"
 	"github.com/minio/pkg/v2/mimedb"
 	"github.com/minio/pkg/v2/sync/errgroup"
-	uatomic "go.uber.org/atomic"
 )
 
 func (er erasureObjects) getUploadIDDir(bucket, object, uploadID string) string {
@@ -273,9 +272,15 @@ func (er erasureObjects) ListMultipartUploads(ctx context.Context, bucket, objec
 	if len(disks) == 0 {
 		// using er.getLoadBalancedLocalDisks() has one side-affect where
 		// on a pooled setup all disks are remote, add a fallback
-		disks = er.getOnlineDisks()
+		disks = er.getDisks()
 	}
 	for _, disk = range disks {
+		if disk == nil {
+			continue
+		}
+		if !disk.IsOnline() {
+			continue
+		}
 		uploadIDs, err = disk.ListDir(ctx, minioMetaMultipartBucket, er.getMultipartSHADir(bucket, object), -1)
 		if err != nil {
 			if errors.Is(err, errDiskNotFound) {
@@ -399,47 +404,31 @@ func (er erasureObjects) newMultipartUpload(ctx context.Context, bucket string,
 	// If we have offline disks upgrade the number of erasure codes for this object.
 	parityOrig := parityDrives
 
-	atomicParityDrives := uatomic.NewInt64(0)
-	atomicOfflineDrives := uatomic.NewInt64(0)
-
-	// Start with current parityDrives
-	atomicParityDrives.Store(int64(parityDrives))
-
-	var wg sync.WaitGroup
+	var offlineDrives int
 	for _, disk := range onlineDisks {
 		if disk == nil {
-			atomicParityDrives.Inc()
-			atomicOfflineDrives.Inc()
+			parityDrives++
+			offlineDrives++
 			continue
 		}
 		if !disk.IsOnline() {
-			atomicParityDrives.Inc()
-			atomicOfflineDrives.Inc()
+			parityDrives++
+			offlineDrives++
 			continue
 		}
-		wg.Add(1)
-		go func(disk StorageAPI) {
-			defer wg.Done()
-			di, err := disk.DiskInfo(ctx, false)
-			if err != nil || di.ID == "" {
-				atomicOfflineDrives.Inc()
-				atomicParityDrives.Inc()
-			}
-		}(disk)
 	}
-	wg.Wait()
 
-	if int(atomicOfflineDrives.Load()) >= (len(onlineDisks)+1)/2 {
+	if offlineDrives >= (len(onlineDisks)+1)/2 {
 		// if offline drives are more than 50% of the drives
 		// we have no quorum, we shouldn't proceed just
 		// fail at that point.
 		return nil, toObjectErr(errErasureWriteQuorum, bucket, object)
 	}
 
-	parityDrives = int(atomicParityDrives.Load())
 	if parityDrives >= len(onlineDisks)/2 {
 		parityDrives = len(onlineDisks) / 2
 	}
+
 	if parityOrig != parityDrives {
 		userDefined[minIOErasureUpgraded] = strconv.Itoa(parityOrig) + "->" + strconv.Itoa(parityDrives)
 	}
diff --git a/cmd/erasure-object.go b/cmd/erasure-object.go
index 6c22a42e2..a60dc5b14 100644
--- a/cmd/erasure-object.go
+++ b/cmd/erasure-object.go
@@ -47,7 +47,6 @@ import (
 	"github.com/minio/pkg/v2/sync/errgroup"
 	"github.com/minio/pkg/v2/wildcard"
 	"github.com/tinylib/msgp/msgp"
-	uatomic "go.uber.org/atomic"
 )
 
 // list all errors which can be ignored in object operations.
@@ -1097,47 +1096,31 @@ func (er erasureObjects) putObject(ctx context.Context, bucket string, object st
 		// If we have offline disks upgrade the number of erasure codes for this object.
 		parityOrig := parityDrives
 
-		atomicParityDrives := uatomic.NewInt64(0)
-		atomicOfflineDrives := uatomic.NewInt64(0)
-
-		// Start with current parityDrives
-		atomicParityDrives.Store(int64(parityDrives))
-
-		var wg sync.WaitGroup
+		var offlineDrives int
 		for _, disk := range storageDisks {
 			if disk == nil {
-				atomicParityDrives.Inc()
-				atomicOfflineDrives.Inc()
+				parityDrives++
+				offlineDrives++
 				continue
 			}
 			if !disk.IsOnline() {
-				atomicParityDrives.Inc()
-				atomicOfflineDrives.Inc()
+				parityDrives++
+				offlineDrives++
 				continue
 			}
-			wg.Add(1)
-			go func(disk StorageAPI) {
-				defer wg.Done()
-				di, err := disk.DiskInfo(ctx, false)
-				if err != nil || di.ID == "" {
-					atomicOfflineDrives.Inc()
-					atomicParityDrives.Inc()
-				}
-			}(disk)
 		}
-		wg.Wait()
 
-		if int(atomicOfflineDrives.Load()) >= (len(storageDisks)+1)/2 {
+		if offlineDrives >= (len(storageDisks)+1)/2 {
 			// if offline drives are more than 50% of the drives
 			// we have no quorum, we shouldn't proceed just
 			// fail at that point.
 			return ObjectInfo{}, toObjectErr(errErasureWriteQuorum, bucket, object)
 		}
 
-		parityDrives = int(atomicParityDrives.Load())
 		if parityDrives >= len(storageDisks)/2 {
 			parityDrives = len(storageDisks) / 2
 		}
+
 		if parityOrig != parityDrives {
 			userDefined[minIOErasureUpgraded] = strconv.Itoa(parityOrig) + "->" + strconv.Itoa(parityDrives)
 		}