Fix test failures

Revert "Only flush term and vote when they have changed." This reverts commit 332c0cf.
neo4j · Feb 12, 2016 · 437960a · 437960a
1 parent b2986b7
commit 437960a
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 106 deletions.
diff --git a/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/state/RaftState.java b/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/state/RaftState.java
@@ -128,16 +128,12 @@ public ReadableRaftLog entryLog()
 
     public void update( Outcome<MEMBER> outcome ) throws RaftStorageException
     {
+        termState.update( outcome.getTerm() );
+        voteState.votedFor( outcome.getVotedFor(), outcome.getTerm() );
         try
         {
-            if ( termState.update( outcome.getTerm() ) )
+            termStorage.persistStoreData( termState );
-            {
+            voteStorage.persistStoreData( voteState );
-                termStorage.persistStoreData( termState );
-            }
-            if ( voteState.update( outcome.getVotedFor(), outcome.getTerm() ) )
-            {
-                voteStorage.persistStoreData( voteState );
-            }
         }
         catch ( IOException e )
         {

diff --git a/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/state/term/TermState.java b/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/state/term/TermState.java
@@ -52,12 +52,10 @@ public long currentTerm()
      *
      * @param newTerm The new value.
      */
-    public boolean update( long newTerm )
+    public void update( long newTerm )
     {
         failIfInvalid( newTerm );
-        boolean changed = term != newTerm;
         term = newTerm;
-        return changed;
     }
 
     /**

diff --git a/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/state/vote/VoteState.java b/enterprise/core-edge/src/main/java/org/neo4j/coreedge/raft/state/vote/VoteState.java
@@ -47,37 +47,34 @@ public MEMBER votedFor()
         return votedFor;
     }
 
-    public boolean update( MEMBER votedFor, long term )
+    public void votedFor( MEMBER votedFor, long term )
     {
-        if ( termChanged( term ) )
+        assert ensureVoteIsUniquePerTerm( votedFor, term ) : "Votes for any instance should always be in more recent terms";
+
+        this.votedFor = votedFor;
+        this.term = term;
+    }
+
+    private boolean ensureVoteIsUniquePerTerm( MEMBER votedFor, long term )
+    {
+        if ( votedFor == null && this.votedFor == null )
+        {
+            return true;
+        }
+        else if ( votedFor == null )
+        {
+            return term > this.term;
+        }
+        else if ( this.votedFor == null )
         {
-            this.votedFor = votedFor;
-            this.term = term;
             return true;
         }
         else
         {
-            if ( this.votedFor == null )
+            return this.votedFor.equals( votedFor ) || term > this.term;
-            {
-                if ( votedFor != null )
-                {
-                    this.votedFor = votedFor;
-                    return true;
-                }
-            }
-            else if ( !this.votedFor.equals( votedFor ) )
-            {
-                throw new IllegalArgumentException( "Can only vote once per term." );
-            }
-            return false;
         }
     }
 
-    private boolean termChanged( long term )
-    {
-        return term != this.term;
-    }
-
     public long term()
     {
         return term;

diff --git a/enterprise/core-edge/src/test/java/org/neo4j/coreedge/raft/state/VoteStateTest.java b/enterprise/core-edge/src/test/java/org/neo4j/coreedge/raft/state/VoteStateTest.java
@@ -26,10 +26,7 @@
 import org.neo4j.coreedge.server.CoreMember;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 public class VoteStateTest
 {
@@ -42,7 +39,7 @@ public void shouldStoreVote() throws Exception
                 new AdvertisedSocketAddress( "host1:2001" ) );
 
         // when
-        voteState.update( member, 0 );
+        voteState.votedFor( member, 0 );
 
         // then
         assertEquals( member, voteState.votedFor() );
@@ -69,8 +66,8 @@ public void shouldUpdateVote() throws Exception
                 new AdvertisedSocketAddress( "host2:2001" ) );
 
         // when
-        voteState.update( member1, 0 );
+        voteState.votedFor( member1, 0 );
-        voteState.update( member2, 1 );
+        voteState.votedFor( member2, 1 );
 
         // then
         assertEquals( member2, voteState.votedFor() );
@@ -83,77 +80,12 @@ public void shouldClearVote() throws Exception
         VoteState<CoreMember> voteState = new VoteState<>();
         CoreMember member = new CoreMember( new AdvertisedSocketAddress( "host1:1001" ),
                 new AdvertisedSocketAddress( "host1:2001" ) );
-        voteState.update( member, 0 );
+        voteState.votedFor( member, 0 );
 
         // when
-        voteState.update( null, 1 );
+        voteState.votedFor( null, 1 );
 
         // then
         assertNull( voteState.votedFor() );
     }
-
-    @Test
-    public void shouldNotUpdateVoteForSameTerm() throws Exception
-    {
-        // given
-        VoteState<CoreMember> voteState = new VoteState<>();
-        CoreMember member1 = new CoreMember( new AdvertisedSocketAddress( "host1:1001" ),
-                new AdvertisedSocketAddress( "host1:2001" ) );
-        CoreMember member2 = new CoreMember( new AdvertisedSocketAddress( "host2:1001" ),
-                new AdvertisedSocketAddress( "host2:2001" ) );
-
-        voteState.update( member1, 0 );
-
-        try
-        {
-            // when
-            voteState.update( member2, 0 );
-            fail( "Should have thrown IllegalArgumentException" );
-        }
-        catch ( IllegalArgumentException expected )
-        {
-            // expected
-        }
-    }
-
-    @Test
-    public void shouldNotClearVoteForSameTerm() throws Exception
-    {
-        // given
-        VoteState<CoreMember> voteState = new VoteState<>();
-        CoreMember member1 = new CoreMember( new AdvertisedSocketAddress( "host1:1001" ),
-                new AdvertisedSocketAddress( "host1:2001" ) );
-
-        voteState.update( member1, 0 );
-
-        try
-        {
-            // when
-            voteState.update( null, 0 );
-            fail( "Should have thrown IllegalArgumentException" );
-        }
-        catch ( IllegalArgumentException expected )
-        {
-            // expected
-        }
-    }
-
-    @Test
-    public void shouldReportNoUpdateWhenVoteStateUnchanged() throws Exception
-    {
-        // given
-        VoteState<CoreMember> voteState = new VoteState<>();
-        CoreMember member1 = new CoreMember( new AdvertisedSocketAddress( "host1:1001" ),
-                new AdvertisedSocketAddress( "host1:2001" ) );
-        CoreMember member2 = new CoreMember( new AdvertisedSocketAddress( "host2:1001" ),
-                new AdvertisedSocketAddress( "host2:2001" ) );
-
-        // when
-        assertTrue( voteState.update( null, 0 ) );
-        assertFalse( voteState.update( null, 0 ) );
-        assertTrue( voteState.update( member1, 0 ) );
-        assertFalse( voteState.update( member1, 0 ) );
-        assertTrue( voteState.update( member2, 1 ) );
-        assertFalse( voteState.update( member2, 1 ) );
-    }
 }