Modify the salvageRevisionCounterDisabledReplicas logic

The old logic is that:
1. Filter replica candidates to keep only replicas which was modified
   within the last 5 seconds from the last modified replica
2. Then filter to keep only replicas with head size equals to the biggest
   one
3. Then pick a random replica from the set

The new logic:
1. Filter replica candiates to keep only replicas which was modified
   within the last 5 seconds from the last modified replica
2. Then filter to keep only replicas with head size equal to the biggest
   one
3. Then pick the last modified replica from the set

longhorn-8659
longhorn-8563

Signed-off-by: Phan Le <[email protected]>

pkg/controller/control.go

@@ -606,54 +606,52 @@ func (c *Controller) checkReplicaRevCounterSettingMatch() error {
 	return nil
 }
 
-// salvageRevisionCounterDisabledReplicas will find best replica
+// salvageRevisionCounterDisabledReplicas will find the best replica
 // for salvage recovering, based on lastModifyTime and HeadFileSize.
 func (c *Controller) salvageRevisionCounterDisabledReplicas() error {
-	replicaCandidates := make(map[types.Replica]types.ReplicaSalvageInfo)
-	var lastModifyTime int64
+	var replicaCandidates []*types.ReplicaSalvageInfo
+	var lastModifyTime time.Time
 	for _, r := range c.replicas {
 		if r.Mode == types.ERR {
 			continue
 		}
-		repLastModifyTime, err := c.backend.backends[r.Address].backend.GetLastModifyTime()
+		repLastModifyTimeInt, err := c.backend.backends[r.Address].backend.GetLastModifyTime()
 		if err != nil {
 			return err
 		}
+		repLastModifyTime := time.Unix(0, repLastModifyTimeInt)
 
 		repHeadFileSize, err := c.backend.backends[r.Address].backend.GetHeadFileSize()
 		if err != nil {
 			return err
 		}
 
-		replicaCandidates[r] = types.ReplicaSalvageInfo{
+		replicaCandidates = append(replicaCandidates, &types.ReplicaSalvageInfo{
+			Address:        r.Address,
 			LastModifyTime: repLastModifyTime,
 			HeadFileSize:   repHeadFileSize,
-		}
-		if lastModifyTime == 0 ||
-			repLastModifyTime > lastModifyTime {
+		})
+		if lastModifyTime.IsZero() || repLastModifyTime.After(lastModifyTime) {
 			lastModifyTime = repLastModifyTime
 		}
 	}
 
 	if len(replicaCandidates) == 0 {
 		return fmt.Errorf("cannot find any replica for salvage")
 	}
-	var bestCandidate types.Replica
-	lastTime := time.Unix(0, lastModifyTime)
-	var largestSize int64
-	for r, salvageReplica := range replicaCandidates {
-		t := time.Unix(0, salvageReplica.LastModifyTime)
-		// Any replica within 5 seconds before lastModifyTime
-		// can be good replica.
-		if t.Add(lastModifyCheckPeriod).After(lastTime) {
-			if salvageReplica.HeadFileSize >= largestSize {
-				bestCandidate = r
-				largestSize = salvageReplica.HeadFileSize
+	var bestCandidate *types.ReplicaSalvageInfo
+	for _, salvageReplica := range replicaCandidates {
+		if salvageReplica.LastModifyTime.Add(lastModifyCheckPeriod).After(lastModifyTime) {
+			if bestCandidate == nil ||
+				salvageReplica.HeadFileSize > bestCandidate.HeadFileSize ||
+				(salvageReplica.HeadFileSize == bestCandidate.HeadFileSize &&
+					salvageReplica.LastModifyTime.After(bestCandidate.LastModifyTime)) {
+				bestCandidate = salvageReplica
 			}
 		}
 	}
 
-	if bestCandidate == (types.Replica{}) {
+	if bestCandidate == nil {
 		return fmt.Errorf("BUG: Should find one candidate for salvage")
 	}
 

pkg/types/types.go

@@ -138,7 +138,8 @@ type Replica struct {
 }
 
 type ReplicaSalvageInfo struct {
-	LastModifyTime int64
+	Address        string
+	LastModifyTime time.Time
 	HeadFileSize   int64
 }