From 36b5426f6e62d11d98a3144ace523a9735ab7f4b Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Sat, 20 Nov 2021 11:26:30 -0800
Subject: [PATCH] dataDir needs maxima calculation to be correct (#13715)

there is a corner case where the new check
doesn't work where dataDir has changed, especially
when xl.json -> xl.meta healing happens, if some
healing is partial this can make certain backend
files unreadable.

This PR fixes and updates unit-tests
---
 cmd/erasure-healing-common.go      | 17 ++++------
 cmd/erasure-healing-common_test.go | 50 +++++++++++++++++++++++++-----
 2 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/cmd/erasure-healing-common.go b/cmd/erasure-healing-common.go
index cbe48a8c6..167a5f26c 100644
--- a/cmd/erasure-healing-common.go
+++ b/cmd/erasure-healing-common.go
@@ -30,12 +30,14 @@ func commonTime(modTimes []time.Time, dataDirs []string) (modTime time.Time, dat
 	var maxima int // Counter for remembering max occurrence of elements.
 
 	timeOccurenceMap := make(map[int64]int, len(modTimes))
+	dataDirMap := make(map[int64]string, len(modTimes))
 	// Ignore the uuid sentinel and count the rest.
-	for _, time := range modTimes {
-		if time.Equal(timeSentinel) {
+	for i, t := range modTimes {
+		if t.Equal(timeSentinel) {
 			continue
 		}
-		timeOccurenceMap[time.UnixNano()]++
+		dataDirMap[t.UnixNano()] = dataDirs[i]
+		timeOccurenceMap[t.UnixNano()]++
 	}
 
 	// Find the common cardinality from previously collected
@@ -44,18 +46,11 @@ func commonTime(modTimes []time.Time, dataDirs []string) (modTime time.Time, dat
 		t := time.Unix(0, nano).UTC()
 		if count > maxima || (count == maxima && t.After(modTime)) {
 			maxima = count
+			dataDir = dataDirMap[nano]
 			modTime = t
 		}
 	}
 
-	for i, ddataDir := range dataDirs {
-		if modTimes[i].Equal(modTime) {
-			// Return the data-dir that matches modTime.
-			dataDir = ddataDir
-			break
-		}
-	}
-
 	// Return the collected common uuid.
 	return modTime, dataDir
 }
diff --git a/cmd/erasure-healing-common_test.go b/cmd/erasure-healing-common_test.go
index fde516f88..a76b5bd55 100644
--- a/cmd/erasure-healing-common_test.go
+++ b/cmd/erasure-healing-common_test.go
@@ -34,8 +34,10 @@ import (
 func TestCommonTime(t *testing.T) {
 	// List of test cases for common modTime.
 	testCases := []struct {
-		times []time.Time
-		time  time.Time
+		times    []time.Time
+		dataDirs []string
+		time     time.Time
+		dataDir  string
 	}{
 		{
 			// 1. Tests common times when slice has varying time elements.
@@ -47,7 +49,17 @@ func TestCommonTime(t *testing.T) {
 				time.Unix(0, 2).UTC(),
 				time.Unix(0, 3).UTC(),
 				time.Unix(0, 1).UTC(),
-			}, time.Unix(0, 3).UTC(),
+			}, []string{
+				errorDir,
+				delMarkerDir,
+				"cd3b36c0-49e6-11ec-8087-73a2b2fd4016",
+				"cd3b36c0-49e6-11ec-8087-73a2b2fd4016",
+				"cd3b36c0-49e6-11ec-8087-73a2b2fd4016",
+				"cd3b36c0-49e6-11ec-8087-73a2b2fd4016",
+				"cd3b36c0-49e6-11ec-8087-73a2b2fd4016",
+			},
+			time.Unix(0, 3).UTC(),
+			"cd3b36c0-49e6-11ec-8087-73a2b2fd4016",
 		},
 		{
 			// 2. Tests common time obtained when all elements are equal.
@@ -59,7 +71,17 @@ func TestCommonTime(t *testing.T) {
 				time.Unix(0, 3).UTC(),
 				time.Unix(0, 3).UTC(),
 				time.Unix(0, 3).UTC(),
-			}, time.Unix(0, 3).UTC(),
+			}, []string{
+				errorDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+			},
+			time.Unix(0, 3).UTC(),
+			delMarkerDir,
 		},
 		{
 			// 3. Tests common time obtained when elements have a mixture
@@ -75,7 +97,17 @@ func TestCommonTime(t *testing.T) {
 				timeSentinel,
 				timeSentinel,
 				timeSentinel,
-			}, time.Unix(0, 3).UTC(),
+			}, []string{
+				errorDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+				delMarkerDir,
+			},
+			time.Unix(0, 3).UTC(),
+			delMarkerDir,
 		},
 	}
 
@@ -83,9 +115,13 @@ func TestCommonTime(t *testing.T) {
 	// common modtime. Tests fail if modtime does not match.
 	for i, testCase := range testCases {
 		// Obtain a common mod time from modTimes slice.
-		ctime, _ := commonTime(testCase.times, nil)
+		ctime, dataDir := commonTime(testCase.times, testCase.dataDirs)
 		if !testCase.time.Equal(ctime) {
-			t.Fatalf("Test case %d, expect to pass but failed. Wanted modTime: %s, got modTime: %s\n", i+1, testCase.time, ctime)
+			t.Errorf("Test case %d, expect to pass but failed. Wanted modTime: %s, got modTime: %s\n", i+1, testCase.time, ctime)
+			continue
+		}
+		if dataDir != testCase.dataDir {
+			t.Errorf("Test case %d, expect to pass but failed. Wanted dataDir: %s, got dataDir: %s\n", i+1, testCase.dataDir, dataDir)
 		}
 	}
 }