From 8ff09c0e458252593f9e0a79231236f7b29405b4 Mon Sep 17 00:00:00 2001
From: William Song <48054931+SzyWilliam@users.noreply.github.com>
Date: Tue, 15 Aug 2023 02:15:39 +0800
Subject: [PATCH] RATIS-1870. Refactor hasMajority code during configuration
 changes. (#902)

---
 .../ratis/server/impl/LeaderStateImpl.java    | 33 ++-----------------
 .../ratis/server/impl/PeerConfiguration.java  | 16 ++++++---
 .../server/impl/RaftConfigurationImpl.java    | 20 +++++++++++
 3 files changed, 33 insertions(+), 36 deletions(-)

diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
index 7ea4d738dc..5dfbc009ba 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderStateImpl.java
@@ -875,36 +875,7 @@ private Optional<MinMajorityMax> getMajorityMin(ToLongFunction<FollowerInfo> fol
 
   private boolean hasMajority(Predicate<RaftPeerId> isAcked) {
     final RaftPeerId selfId = server.getId();
-    final RaftConfigurationImpl conf = server.getRaftConf();
-
-    final CurrentOldFollowerInfos infos = followerInfoMap.getFollowerInfos(conf);
-    final List<FollowerInfo> followers = infos.getCurrent();
-    final boolean includeSelf = conf.containsInConf(selfId);
-    final boolean newConf = hasMajority(isAcked, followers, includeSelf);
-
-    if (!conf.isTransitional()) {
-      return newConf;
-    } else {
-      final List<FollowerInfo> oldFollowers = infos.getOld();
-      final boolean includeSelfInOldConf = conf.containsInOldConf(selfId);
-      final boolean oldConf = hasMajority(isAcked, oldFollowers, includeSelfInOldConf);
-      return newConf && oldConf;
-    }
-  }
-
-  private boolean hasMajority(Predicate<RaftPeerId> isAcked, List<FollowerInfo> followers, boolean includeSelf) {
-    if (followers.isEmpty() && !includeSelf) {
-      return true;
-    }
-
-    int count = includeSelf ? 1 : 0;
-    for (FollowerInfo follower: followers) {
-      if (isAcked.test(follower.getId())) {
-        count++;
-      }
-    }
-    final int size = includeSelf ? followers.size() + 1 : followers.size();
-    return count > size / 2;
+    return server.getRaftConf().hasMajority(isAcked, selfId);
   }
 
   private void updateCommit(LogEntryHeader[] entriesToCommit) {
@@ -1094,7 +1065,7 @@ CompletableFuture<Long> getReadIndex() {
     final long readIndex = server.getRaftLog().getLastCommittedIndex();
 
     // if group contains only one member, fast path
-    if (server.getRaftConf().getCurrentPeers().size() == 1) {
+    if (server.getRaftConf().isSingleton()) {
       return CompletableFuture.completedFuture(readIndex);
     }
 
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
index 38e3602e85..600667c388 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/PeerConfiguration.java
@@ -31,6 +31,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.function.Predicate;
 import java.util.stream.Stream;
 
 /**
@@ -150,12 +151,17 @@ List<RaftPeer> getOtherPeers(RaftPeerId selfId) {
 
   boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
     Preconditions.assertTrue(!others.contains(selfId));
-    int num = 0;
-    if (contains(selfId)) {
-      num++;
+    return hasMajority(others::contains, contains(selfId));
+  }
+
+  boolean hasMajority(Predicate<RaftPeerId> activePeers, boolean includeSelf) {
+    if (peers.isEmpty() && !includeSelf) {
+      return true;
     }
-    for (RaftPeerId other : others) {
-      if (contains(other)) {
+
+    int num = includeSelf ? 1 : 0;
+    for (RaftPeerId peerId: peers.keySet()) {
+      if (activePeers.test(peerId)) {
         num++;
       }
     }
diff --git a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfigurationImpl.java b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfigurationImpl.java
index da9481a2ee..aba5ae176e 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfigurationImpl.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftConfigurationImpl.java
@@ -30,6 +30,7 @@
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
@@ -238,6 +239,20 @@ boolean hasMajority(Collection<RaftPeerId> others, RaftPeerId selfId) {
         (oldConf == null || oldConf.hasMajority(others, selfId));
   }
 
+  /** @return true if the self id together with the acknowledged followers reach majority. */
+  boolean hasMajority(Predicate<RaftPeerId> followers, RaftPeerId selfId) {
+    final boolean includeInCurrent = containsInConf(selfId);
+    final boolean hasMajorityInNewConf = conf.hasMajority(followers, includeInCurrent);
+
+    if (!isTransitional()) {
+      return hasMajorityInNewConf;
+    } else {
+      final boolean includeInOldConf = containsInOldConf(selfId);
+      final boolean hasMajorityInOldConf = oldConf.hasMajority(followers, includeInOldConf);
+      return hasMajorityInOldConf && hasMajorityInNewConf;
+    }
+  }
+
   int getMajorityCount() {
     return conf.getMajorityCount();
   }
@@ -248,6 +263,11 @@ boolean majorityRejectVotes(Collection<RaftPeerId> rejects) {
             (oldConf != null && oldConf.majorityRejectVotes(rejects));
   }
 
+  /** @return true if only one voting member (the leader) in the cluster */
+  boolean isSingleton() {
+    return getCurrentPeers().size() == 1 && getPreviousPeers().size() <= 1;
+  }
+
   @Override
   public String toString() {
     return logEntryIndex + ": " + conf + ", old=" + oldConf;