From 4180772c02ce3b6ef65415a0394a5919fa10cdbc Mon Sep 17 00:00:00 2001
From: Ryan Leung <rleungx@gmail.com>
Date: Mon, 5 Dec 2022 13:44:02 +0800
Subject: [PATCH] core: reorder the check (#5653)

ref tikv/pd#5648

Signed-off-by: Ryan Leung <rleungx@gmail.com>
---
 server/core/region.go      | 39 +++++++++++++++++++++-----------------
 server/core/region_test.go | 31 ++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/server/core/region.go b/server/core/region.go
index 03da953ba60..fa68e1b5248 100644
--- a/server/core/region.go
+++ b/server/core/region.go
@@ -625,6 +625,9 @@ func GenerateRegionGuideFunc(enableLog bool) RegionGuideFunc {
 				logutil.ZapRedactStringer("meta-region", RegionToHexMeta(region.GetMeta())))
 			saveKV, saveCache, isNew = true, true, true
 		} else {
+			if !origin.IsFromHeartbeat() {
+				isNew = true
+			}
 			r := region.GetRegionEpoch()
 			o := origin.GetRegionEpoch()
 			if r.GetVersion() > o.GetVersion() {
@@ -655,27 +658,17 @@ func GenerateRegionGuideFunc(enableLog bool) RegionGuideFunc {
 						zap.Uint64("to", region.GetLeader().GetStoreId()),
 					)
 				}
-				saveCache, needSync = true, true
-			}
-			if !SortedPeersStatsEqual(region.GetDownPeers(), origin.GetDownPeers()) {
-				debug("down-peers changed", zap.Uint64("region-id", region.GetID()))
-				saveCache, needSync = true, true
-			}
-			if !SortedPeersEqual(region.GetPendingPeers(), origin.GetPendingPeers()) {
-				debug("pending-peers changed", zap.Uint64("region-id", region.GetID()))
+				// We check it first and do not return because the log is important for us to investigate,
 				saveCache, needSync = true, true
 			}
 			if len(region.GetPeers()) != len(origin.GetPeers()) {
 				saveKV, saveCache = true, true
+				return
 			}
 			if len(region.GetBuckets().GetKeys()) != len(origin.GetBuckets().GetKeys()) {
 				debug("bucket key changed", zap.Uint64("region-id", region.GetID()))
 				saveKV, saveCache = true, true
-			}
-
-			if region.GetApproximateSize() != origin.GetApproximateSize() ||
-				region.GetApproximateKeys() != origin.GetApproximateKeys() {
-				saveCache = true
+				return
 			}
 			// Once flow has changed, will update the cache.
 			// Because keys and bytes are strongly related, only bytes are judged.
@@ -683,16 +676,28 @@ func GenerateRegionGuideFunc(enableLog bool) RegionGuideFunc {
 				region.GetRoundBytesRead() != origin.GetRoundBytesRead() ||
 				region.flowRoundDivisor < origin.flowRoundDivisor {
 				saveCache, needSync = true, true
+				return
+			}
+			if !SortedPeersStatsEqual(region.GetDownPeers(), origin.GetDownPeers()) {
+				debug("down-peers changed", zap.Uint64("region-id", region.GetID()))
+				saveCache, needSync = true, true
+				return
+			}
+			if !SortedPeersEqual(region.GetPendingPeers(), origin.GetPendingPeers()) {
+				debug("pending-peers changed", zap.Uint64("region-id", region.GetID()))
+				saveCache, needSync = true, true
+				return
+			}
+			if region.GetApproximateSize() != origin.GetApproximateSize() ||
+				region.GetApproximateKeys() != origin.GetApproximateKeys() {
+				saveCache = true
+				return
 			}
-
 			if region.GetReplicationStatus().GetState() != replication_modepb.RegionReplicationState_UNKNOWN &&
 				(region.GetReplicationStatus().GetState() != origin.GetReplicationStatus().GetState() ||
 					region.GetReplicationStatus().GetStateId() != origin.GetReplicationStatus().GetStateId()) {
 				saveCache = true
 			}
-			if !origin.IsFromHeartbeat() {
-				isNew = true
-			}
 		}
 		return
 	}
diff --git a/server/core/region_test.go b/server/core/region_test.go
index d6030997fa3..98d7909ab0a 100644
--- a/server/core/region_test.go
+++ b/server/core/region_test.go
@@ -694,3 +694,34 @@ func BenchmarkAddRegion(b *testing.B) {
 		regions.UpdateSubTree(items[i], origin, overlaps, rangeChanged)
 	}
 }
+
+func BenchmarkRegionFromHeartbeat(b *testing.B) {
+	peers := make([]*metapb.Peer, 0, 3)
+	for i := uint64(1); i <= 3; i++ {
+		peers = append(peers, &metapb.Peer{
+			Id:      i,
+			StoreId: i,
+		})
+	}
+	regionReq := &pdpb.RegionHeartbeatRequest{
+		Region: &metapb.Region{
+			Id:       1,
+			Peers:    peers,
+			StartKey: []byte{byte(2)},
+			EndKey:   []byte{byte(3)},
+			RegionEpoch: &metapb.RegionEpoch{
+				ConfVer: 2,
+				Version: 1,
+			},
+		},
+		Leader:          peers[0],
+		Term:            5,
+		ApproximateSize: 10,
+		PendingPeers:    []*metapb.Peer{peers[1]},
+		DownPeers:       []*pdpb.PeerStats{{Peer: peers[2], DownSeconds: 100}},
+	}
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		RegionFromHeartbeat(regionReq)
+	}
+}