Fix longest log verification during election

* Updated the check to keep track of the "highest" VoteResponse received
  and perform checks against it.
* Expanded test to cover the three possibilities scenarios in the
  {A,B,C} view.

Closes jgroups-extras#251.

src/org/jgroups/protocols/raft/election/BaseElection.java

@@ -229,32 +229,49 @@ private void handleVoteResponse(Message msg, VoteResponse hdr) {
 
     protected Address determineLeader() {
         Address leader=null;
+        VoteResponse higher = null;
         Map<Address,VoteResponse> results=votes.getResults();
         for(Address mbr: view.getMembersRaw()) {
             VoteResponse rsp=results.get(mbr);
             if(rsp == null)
                 continue;
-            if(leader == null)
-                leader=mbr;
-            if(isHigher(rsp.last_log_term, rsp.last_log_index))
-                leader=mbr;
+            if (isHigher(higher, rsp)) {
+                leader = mbr;
+                higher = rsp;
+            }
         }
         return leader;
     }
 
     /**
-     * Returns true if last_term greater than my own term, false if smaller. If they're equal, returns true if
-     * last_index is > my own last index, false otherwise
+     * Compares the {@link VoteResponse}s in search of the highest one.
+     * <p>
+     * The verification follows the precedence:
+     * <ol>
+     *     <li>Compare Raft terms;</li>
+     *     <li>Compare the log last appended.</li>
+     * </ol>
+     * This verification ensures the decided leader has the longest log.
+     * </p>
+     *
+     * @param one: The base to check against.
+     * @param other: The candidate response to check.
+     * @return <code>true</code> if {@param other} is higher than {@param one}. <code>false</code>, otherwise.
      */
-    protected boolean isHigher(long last_term, long last_index) {
-        long my_last_index=raft.log().lastAppended();
-        LogEntry entry=raft.log().get(my_last_index);
-        long my_last_term=entry != null? entry.term() : 0;
-        if(last_term > my_last_term)
+    private boolean isHigher(VoteResponse one, VoteResponse other) {
+        if (one == null) return true;
+
+        // The candidate response has a higher Raft term.
+        if (one.last_log_term < other.last_log_term)
             return true;
-        if(last_term < my_last_term)
+
+        // The candidate response has an outdated Raft term.
+        if (one.last_log_term > other.last_log_term)
             return false;
-        return last_index > my_last_index;
+
+        // Both responses have the same term.
+        // Break ties utilizing the index of the last appended entry.
+        return one.last_log_index < other.last_log_index;
     }
 
     /**

tests/junit-functional/org/jgroups/tests/ElectionsTest.java

@@ -14,6 +14,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 import org.testng.annotations.Test;
 
@@ -54,18 +55,46 @@ public void testSimpleElection(Class<?> ignore) {
     /**
      * B and C have longer logs than A: one of {B,C} must become coordinator, but *not* A
      */
-    public void testElectionWithLongLog(Class<?> ignore) {
+    public void testElectionWithLongLogTie(Class<?> ignore) {
+        testLongestLog(Map.of(b, new int[]{1, 1, 2}, c, new int[]{1, 1, 2}), b.getAddress(), c.getAddress());
+    }
+
+    /**
+     * B has the longest term in the view {A, B, C}.
+     * Node B has to become the leader.
+     */
+    public void testElectionWithLongLogMiddle(Class<?> ignore) {
+        int[] termsB = new int[] { 1, 1, 2, 2 };
+        int[] termsC = new int[] { 1, 1, 2 };
+        testLongestLog(Map.of(b, termsB, c, termsC), b.getAddress());
+    }
+
+    /**
+     * C has the longest term in the view {A, B, C}.
+     * Node C has to become the leader.
+     */
+    public void testElectionWithLongLogLast(Class<?> ignore) {
+        int[] termsB = new int[] { 1, 1, 2 };
+        int[] termsC = new int[] { 1, 1, 2, 2 };
+        testLongestLog(Map.of(b, termsB, c, termsC), c.getAddress());
+    }
+
+    private void testLongestLog(Map<JChannel, int[]> logs, Address ... possibleElected) {
         // Let node A be elected the first leader.
         assertLeader(10_000, a.getAddress(), a, b, c);
 
-        // Add the entries, creating longer logs.
-        setLog(b, 1, 1, 2);
-        setLog(c, 1, 1, 2);
+        for (Map.Entry<JChannel, int[]> entry : logs.entrySet()) {
+            setLog(entry.getKey(), entry.getValue());
+        }
 
-        // Assert that B and C have a longer log.
+        // Assert nodes have longer logs than A.
         long aSize = logLastAppended(a);
-        assert aSize < logLastAppended(b) : "A log longer than B";
-        assert aSize < logLastAppended(c) : "A log longer than C";
+
+        for (JChannel ch : logs.keySet()) {
+            assertThat(aSize)
+                    .withFailMessage(() -> String.format("Node A has a log longer than %s", ch.getAddress()))
+                    .isLessThan(logLastAppended(ch));
+        }
 
         JChannel coord = findCoord(a, b, c);
         assertThat(coord).isNotNull();
@@ -82,9 +111,10 @@ public void testElectionWithLongLog(Class<?> ignore) {
         el.startVotingThread();
         waitUntilVotingThreadStops(5_000, 0, 1, 2);
 
+        System.out.printf("-- waiting for leader in %s", Arrays.toString(possibleElected));
         Address leader = assertLeader(10_000, null, b, c);
-        assert leader.equals(b.getAddress()) || leader.equals(c.getAddress()) : dumpLeaderAndTerms();
-        assert !leader.equals(a.getAddress());
+        assertThat(possibleElected).contains(leader);
+        assertThat(leader).isNotEqualTo(a.getAddress());
     }
 
     protected static JChannel findCoord(JChannel... channels) {