Fix Raft vote review findings for 04c78b3c-2267-495b-9aca-84b544a1882f

2 files changed, 95 insertions, 11 deletions

diff --git a/src/main/java/protocols/implementations/VSRaftProtocol.java b/src/main/java/protocols/implementations/VSRaftProtocol.java
index 597c1e0..230c6d9 100644
--- a/src/main/java/protocols/implementations/VSRaftProtocol.java
+++ b/src/main/java/protocols/implementations/VSRaftProtocol.java
@@ -40,6 +40,9 @@ public class VSRaftProtocol extends VSAbstractProtocol {
     /** PIDs which still have to acknowledge the current operation. */
     private ArrayList<Integer> ackPids;
 
+    /** Peer PIDs whose vote responses have been counted in this election. */
+    private ArrayList<Integer> voteResponsePids;
+
     /** The local log index. */
     private int logIndex;
 
@@ -152,6 +155,12 @@ public class VSRaftProtocol extends VSAbstractProtocol {
         } else {
             ackPids.clear();
         }
+
+        if (voteResponsePids == null) {
+            voteResponsePids = new ArrayList<Integer>();
+        } else {
+            voteResponsePids.clear();
+        }
     }
 
     /**
@@ -161,6 +170,7 @@ public class VSRaftProtocol extends VSAbstractProtocol {
         isLeader = true;
         isCandidate = false;
         votesReceived = 0;
+        voteResponsePids.clear();
         leaderId = process.getProcessID();
         lastHeartbeatTime = process.getTime();
         isServer(true);
@@ -193,6 +203,7 @@ public class VSRaftProtocol extends VSAbstractProtocol {
         leaderId = newLeaderId;
         votedFor = -1;
         votesReceived = 0;
+        voteResponsePids.clear();
         resetElectionTimeout();
     }
 
@@ -230,6 +241,7 @@ public class VSRaftProtocol extends VSAbstractProtocol {
         currentTerm++;
         votedFor = process.getProcessID();
         votesReceived = 1;
+        voteResponsePids.clear();
         isLeader = false;
         isCandidate = true;
         leaderId = -1;
@@ -284,9 +296,12 @@ public class VSRaftProtocol extends VSAbstractProtocol {
         int candidateId = recvMessage.getInteger("candidateId");
         boolean voteGranted = false;
 
-        if (messageTerm >= currentTerm &&
-                (votedFor == -1 || votedFor == candidateId)) {
+        if (messageTerm > currentTerm) {
             becomeFollower(messageTerm, -1);
+        }
+
+        if (messageTerm == currentTerm &&
+                (votedFor == -1 || votedFor == candidateId)) {
             votedFor = candidateId;
             voteGranted = true;
         }
@@ -307,6 +322,7 @@ public class VSRaftProtocol extends VSAbstractProtocol {
      */
     private void handleVoteResponse(VSMessage recvMessage) {
         int messageTerm = recvMessage.getInteger("term");
+        Integer responderPid = recvMessage.getIntegerObj("pid");
 
         if (messageTerm > currentTerm) {
             becomeFollower(messageTerm, -1);
@@ -315,10 +331,12 @@ public class VSRaftProtocol extends VSAbstractProtocol {
 
         if (!isCandidate || !isForMe(recvMessage) ||
                 !recvMessage.getBoolean("voteGranted") ||
-                messageTerm != currentTerm) {
+                messageTerm != currentTerm ||
+                voteResponsePids.contains(responderPid)) {
             return;
         }
 
+        voteResponsePids.add(responderPid);
         votesReceived++;
 
         if (votesReceived > getClusterSize() / 2) {
diff --git a/src/test/java/protocols/implementations/VSRaftProtocolTest.java b/src/test/java/protocols/implementations/VSRaftProtocolTest.java
index 5f0fced..380aaae 100644
--- a/src/test/java/protocols/implementations/VSRaftProtocolTest.java
+++ b/src/test/java/protocols/implementations/VSRaftProtocolTest.java
@@ -265,10 +265,11 @@ class VSRaftProtocolTest {
     }
 
     @Test
-    void testClientReceiveVoteRequestGrantsEligibleCandidate() throws Exception {
+    void testServerReceiveVoteRequestGrantsEligibleCandidate() throws Exception {
         protocol.currentContextIsServer(false);
         protocol.onClientInit();
         clearInvocations(mockProcess, mockTaskManager);
+        protocol.currentContextIsServer(true);
         when(mockProcess.getTime()).thenReturn(200L, 200L);
 
         VSMessage voteRequest = new VSMessage();
@@ -280,7 +281,7 @@ class VSRaftProtocolTest {
             ArgumentCaptor.forClass(VSMessage.class);
         ArgumentCaptor<VSTask> taskCaptor = ArgumentCaptor.forClass(VSTask.class);
 
-        protocol.onClientRecv(voteRequest);
+        protocol.onServerRecv(voteRequest);
 
         verify(mockProcess).sendMessage(messageCaptor.capture());
         verify(mockTaskManager, times(2)).removeAllTasks(any());
@@ -300,11 +301,11 @@ class VSRaftProtocolTest {
     }
 
     @Test
-    void testClientReceiveVoteRequestDeniesWhenAlreadyVotedForOtherCandidate()
+    void testServerReceiveVoteRequestDeniesWhenAlreadyVotedForOtherCandidate()
     throws Exception {
         setIntField("currentTerm", 3);
         setIntField("votedFor", 9);
-        protocol.currentContextIsServer(false);
+        protocol.currentContextIsServer(true);
 
         VSMessage voteRequest = new VSMessage();
         voteRequest.setString("type", "voteRequest");
@@ -314,7 +315,7 @@ class VSRaftProtocolTest {
         ArgumentCaptor<VSMessage> messageCaptor =
             ArgumentCaptor.forClass(VSMessage.class);
 
-        protocol.onClientRecv(voteRequest);
+        protocol.onServerRecv(voteRequest);
 
         verify(mockProcess).sendMessage(messageCaptor.capture());
         verify(mockTaskManager, never()).removeAllTasks(any());
@@ -329,6 +330,44 @@ class VSRaftProtocolTest {
     }
 
     @Test
+    void testServerReceiveHigherTermVoteRequestResetsVoteAndGrants()
+    throws Exception {
+        setIntField("currentTerm", 3);
+        setIntField("votedFor", 9);
+        setBooleanField("isCandidate", true);
+        protocol.currentContextIsServer(false);
+        protocol.onClientInit();
+        clearInvocations(mockProcess, mockTaskManager);
+        protocol.currentContextIsServer(true);
+        when(mockProcess.getTime()).thenReturn(250L, 250L);
+
+        VSMessage voteRequest = new VSMessage();
+        voteRequest.setString("type", "voteRequest");
+        voteRequest.setInteger("term", 4);
+        voteRequest.setInteger("candidateId", 11);
+
+        ArgumentCaptor<VSMessage> messageCaptor =
+            ArgumentCaptor.forClass(VSMessage.class);
+        ArgumentCaptor<VSTask> taskCaptor = ArgumentCaptor.forClass(VSTask.class);
+
+        protocol.onServerRecv(voteRequest);
+
+        verify(mockProcess).sendMessage(messageCaptor.capture());
+        verify(mockTaskManager, times(2)).removeAllTasks(any());
+        verify(mockTaskManager).addTask(taskCaptor.capture());
+
+        VSMessage voteResponse = messageCaptor.getValue();
+        assertEquals("voteResponse", voteResponse.getString("type"));
+        assertEquals(4, voteResponse.getInteger("term"));
+        assertTrue(voteResponse.getBoolean("voteGranted"));
+        assertEquals(4, getIntField("currentTerm"));
+        assertEquals(11, getIntField("votedFor"));
+        assertFalse(getBooleanField("isCandidate"));
+        assertFalse(getBooleanField("isLeader"));
+        assertEquals(4750L, taskCaptor.getValue().getTaskTime());
+    }
+
+    @Test
     void testVoteResponseMajorityPromotesCandidateToLeader() throws Exception {
         protocol.currentContextIsServer(false);
         setIntField("currentTerm", 3);
@@ -347,7 +386,7 @@ class VSRaftProtocolTest {
             ArgumentCaptor.forClass(VSMessage.class);
         ArgumentCaptor<VSTask> taskCaptor = ArgumentCaptor.forClass(VSTask.class);
 
-        protocol.onServerRecv(voteResponse);
+        protocol.onClientRecv(voteResponse);
 
         verify(mockProcess).sendMessage(messageCaptor.capture());
         verify(mockTaskManager).removeAllTasks(any());
@@ -378,7 +417,7 @@ class VSRaftProtocolTest {
         voteResponse.setBoolean("voteGranted", true);
         voteResponse.setInteger("targetPid", 99);
 
-        protocol.onServerRecv(voteResponse);
+        protocol.onClientRecv(voteResponse);
 
         verify(mockProcess, never()).sendMessage(any());
         verify(mockTaskManager, never()).removeAllTasks(any());
@@ -407,7 +446,7 @@ class VSRaftProtocolTest {
 
         ArgumentCaptor<VSTask> taskCaptor = ArgumentCaptor.forClass(VSTask.class);
 
-        protocol.onServerRecv(voteResponse);
+        protocol.onClientRecv(voteResponse);
 
         verify(mockProcess, never()).sendMessage(any());
         verify(mockTaskManager, times(2)).removeAllTasks(any());
@@ -420,6 +459,33 @@ class VSRaftProtocolTest {
         assertEquals(5000L, taskCaptor.getValue().getTaskTime());
     }
 
+    @Test
+    void testDuplicateVoteResponsesFromSamePeerDoNotCreateMajority()
+    throws Exception {
+        protocol.currentContextIsServer(false);
+        when(mockCanvas.getNumProcesses()).thenReturn(5);
+        setIntField("currentTerm", 3);
+        setIntField("votesReceived", 1);
+        setBooleanField("isCandidate", true);
+
+        VSMessage voteResponse = new VSMessage();
+        voteResponse.setString("type", "voteResponse");
+        voteResponse.setInteger("term", 3);
+        voteResponse.setInteger("pid", 2);
+        voteResponse.setBoolean("voteGranted", true);
+        voteResponse.setInteger("targetPid", 7);
+
+        protocol.onClientRecv(voteResponse);
+        protocol.onClientRecv(voteResponse);
+
+        verify(mockProcess, never()).sendMessage(any());
+        verify(mockTaskManager, never()).removeAllTasks(any());
+        verify(mockTaskManager, never()).addTask(any());
+        assertEquals(2, getIntField("votesReceived"));
+        assertTrue(getBooleanField("isCandidate"));
+        assertFalse(getBooleanField("isLeader"));
+    }
+
     private void invokeBecomeFollower(int term, int leaderId) throws Exception {
         Method method = VSRaftProtocol.class.getDeclaredMethod(
             "becomeFollower", int.class, int.class);