Skip to content

Commit

Permalink
Remove raft log entryExists which is unnecessary and confusing.
Browse files Browse the repository at this point in the history
  • Loading branch information
martinfurmanski authored and apcj committed Mar 9, 2016
1 parent 0f8f3a5 commit d22631e
Show file tree
Hide file tree
Showing 17 changed files with 13 additions and 109 deletions.
Expand Up @@ -121,12 +121,6 @@ public synchronized void truncate( long fromIndex )
term = readEntryTerm( appendIndex );
}

@Override
public boolean entryExists( long logIndex )
{
return raftLog.containsKey( logIndex );
}

@Override
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException
{
Expand Down
Expand Up @@ -93,12 +93,6 @@ public long readEntryTerm( long logIndex ) throws IOException
return delegate.readEntryTerm( logIndex );
}

@Override
public boolean entryExists( long logIndex ) throws IOException
{
return delegate.entryExists( logIndex );
}

@Override
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException
{
Expand Down
Expand Up @@ -257,12 +257,6 @@ public long readEntryTerm( long logIndex ) throws IOException
return readEntry( logIndex ).term;
}

@Override
public boolean entryExists( long logIndex )
{
return appendIndex >= logIndex;
}

private static class Entry
{
private final long term;
Expand Down
Expand Up @@ -264,12 +264,6 @@ public long readEntryTerm( long logIndex ) throws IOException
return resultTerm;
}

@Override
public boolean entryExists( long logIndex ) throws IOException
{
return appendIndex.get() >= logIndex;
}

@Override
public void init() throws Throwable
{
Expand Down
Expand Up @@ -21,7 +21,6 @@

import java.io.IOException;

import org.neo4j.coreedge.raft.replication.ReplicatedContent;
import org.neo4j.cursor.IOCursor;

public interface ReadableRaftLog
Expand Down Expand Up @@ -52,17 +51,9 @@ public interface ReadableRaftLog
*/
long readEntryTerm( long logIndex ) throws IOException;

/**
* Tells if a entry exists in the log at the supplied index.
*
* @param logIndex The index of the log entry.
* @return True if the entry exists, otherwise false.
*/
boolean entryExists( long logIndex ) throws IOException;

/**
* Returns an {@link IOCursor} of {@link RaftLogEntry}s from the specified index until the end of the log
* @param fromIndex The log index at which the cursor should be positioned
*/
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException;
IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException;
}
Expand Up @@ -39,7 +39,7 @@ public AppendLogEntry( long index, RaftLogEntry entry )
@Override
public void applyTo( RaftLog raftLog ) throws IOException
{
if ( raftLog.entryExists( index ) )
if ( index <= raftLog.appendIndex() )
{
throw new IllegalStateException( "Attempted to append over an existing entry at index " + index );
}
Expand Down
Expand Up @@ -44,9 +44,10 @@ public BatchAppendLogEntries( long baseIndex, int offset, RaftLogEntry[] entries
@Override
public void applyTo( RaftLog raftLog ) throws IOException
{
if ( raftLog.entryExists( baseIndex + offset ) )
long lastIndex = baseIndex + offset;
if ( lastIndex <= raftLog.appendIndex() )
{
throw new IllegalStateException( "Attempted to append over an existing entry starting at index " + baseIndex + offset );
throw new IllegalStateException( "Attempted to append over an existing entry starting at index " + lastIndex );
}

for ( int i = offset; i < entries.length; i++ )
Expand Down
Expand Up @@ -399,7 +399,7 @@ private void sendSingle( long logIndex, LeaderContext leaderContext ) throws IOE
}

RaftLogEntry[] logEntries;
if ( raftLog.entryExists( logIndex ) )
if ( logIndex <= raftLog.appendIndex() )
{
logEntries = new RaftLogEntry[]{ raftLog.readLogEntry( logIndex ) };
}
Expand Down
Expand Up @@ -545,12 +545,6 @@ public long readEntryTerm( long logIndex ) throws IOException
return -1;
}

@Override
public boolean entryExists( long logIndex )
{
return false;
}

@Override
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException
{
Expand Down
Expand Up @@ -97,7 +97,7 @@ public void shouldReadBackInCachedEntry() throws Throwable
long entryIndex = raftLog.append( new RaftLogEntry( term, content ) );

// Then
assertTrue( raftLog.entryExists( entryIndex ) );
assertEquals( entryIndex, raftLog.appendIndex() );
assertEquals( content, raftLog.readLogEntry( entryIndex ).content() );
assertEquals( term, raftLog.readEntryTerm( entryIndex ) );
}
Expand All @@ -118,12 +118,10 @@ public void shouldReadBackNonCachedEntry() throws Exception

// Then
// entry 1 should be there
assertTrue( raftLog.entryExists( entryIndex1 ) );
assertEquals( content1, raftLog.readLogEntry( entryIndex1 ).content() );
assertEquals( term, raftLog.readEntryTerm( entryIndex1 ) );

// entry 2 should be there also
assertTrue( raftLog.entryExists( entryIndex2 ) );
assertEquals( content2, raftLog.readLogEntry( entryIndex2 ).content() );
assertEquals( term, raftLog.readEntryTerm( entryIndex2 ) );
}
Expand Down
Expand Up @@ -20,7 +20,6 @@
package org.neo4j.coreedge.raft.log;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;

import java.io.File;
Expand Down Expand Up @@ -143,7 +142,6 @@ public void shouldBeAbleToRecoverToLatestStateAfterRotation() throws Throwable
// Then
assertEquals( indexToCommit, log.commitIndex() );
assertEquals( indexToCommit, log.appendIndex() );
assertTrue( log.entryExists( indexToCommit ) );
assertEquals( term, log.readEntryTerm( indexToCommit ) );
}
}
Expand Up @@ -74,7 +74,6 @@ public void shouldDiscardEntryIfEntryChannelFails() throws Exception
verifyCurrentLogAndNewLogLoadedFromFileSystem( log, fileSystem, l -> {
assertThat( l.appendIndex(), is( -1L ) );
assertThat( l.commitIndex(), is( -1L ) );
assertThat( l.entryExists( 0 ), is( false ) );
} );
}

Expand Down Expand Up @@ -105,7 +104,6 @@ public void shouldDiscardEntryIfContentChannelFails() throws Exception
verifyCurrentLogAndNewLogLoadedFromFileSystem( log, fileSystem, l -> {
assertThat( l.appendIndex(), is( -1L ) );
assertThat( l.commitIndex(), is( -1L ) );
assertThat( l.entryExists( 0 ), is( false ) );
} );
}

Expand Down
Expand Up @@ -42,7 +42,6 @@ public void shouldReportCorrectDefaultValuesOnEmptyLog() throws Exception
// then
assertThat( log.appendIndex(), is( -1L ) );
assertThat( log.commitIndex(), is( -1L ) );
assertThat( log.entryExists( 0 ), is( false ) );
assertThat( log.readEntryTerm( 0 ), is( -1L ) );
assertThat( log.readEntryTerm( -1 ), is( -1L ) );
}
Expand Down Expand Up @@ -76,7 +75,6 @@ public void shouldAppendDataAndNotCommitImmediately() throws Exception

assertThat( log.appendIndex(), is( 0L ) );
assertThat( log.commitIndex(), is( -1L ) );
assertThat( log.entryExists( 0 ), is( true ) );
assertThat( log.readLogEntry( 0 ), equalTo( logEntry ) );
}

Expand All @@ -89,7 +87,6 @@ public void shouldNotCommitWhenNoAppendedData() throws Exception

assertThat( log.appendIndex(), is( -1L ) );
assertThat( log.commitIndex(), is( -1L ) );
assertThat( log.entryExists( 0 ), is( false ) );
}

@Test
Expand All @@ -106,7 +103,6 @@ public void shouldCommitOutOfOrderAppend() throws Exception

assertThat( log.appendIndex(), is( 0L ) );
assertThat( log.commitIndex(), is( 0L ) );
assertThat( log.entryExists( 0 ), is( true ) );
}

@Test
Expand All @@ -121,14 +117,10 @@ public void shouldTruncatePreviouslyAppendedEntries() throws Exception
log.append( logEntryB );

assertThat( log.appendIndex(), is( 1L ) );
assertThat( log.entryExists( 0 ), is( true ) );
assertThat( log.entryExists( 1 ), is( true ) );

log.truncate( 1 );

assertThat( log.appendIndex(), is( 0L ) );
assertThat( log.entryExists( 0 ), is( true ) );
assertThat( log.entryExists( 1 ), is( false ) );
}

@Test
Expand All @@ -155,10 +147,6 @@ public void shouldReplacePreviouslyAppendedEntries() throws Exception
assertThat( log.readLogEntry( 0 ), equalTo( logEntryA ) );
assertThat( log.readLogEntry( 1 ), equalTo( logEntryD ) );
assertThat( log.readLogEntry( 2 ), equalTo( logEntryE ) );
assertThat( log.entryExists( 0 ), is( true ) );
assertThat( log.entryExists( 1 ), is( true ) );
assertThat( log.entryExists( 2 ), is( true ) );
assertThat( log.entryExists( 3 ), is( false ) );
}

@Test
Expand Down Expand Up @@ -231,8 +219,7 @@ public void shouldCommitAndThenTruncateSubsequentEntry() throws Exception
log.truncate( toTruncate );

// then
assertThat( log.entryExists( toCommit ), is( true ) );
assertThat( log.entryExists( toTruncate ), is( false ) );
assertThat( log.appendIndex(), is( toCommit ) );
assertThat( log.readEntryTerm( toCommit ), is( 0L ) );
}

Expand All @@ -250,8 +237,7 @@ public void shouldTruncateAndThenCommitPreviousEntry() throws Exception
log.commit( toCommit );

// then
assertThat( log.entryExists( toCommit ), is( true ) );
assertThat( log.entryExists( toTruncate ), is( false ) );
assertThat( log.appendIndex(), is( toCommit ) );
assertThat( log.readEntryTerm( toCommit ), is( 0L ) );
}

Expand All @@ -274,9 +260,7 @@ public void shouldCommitAfterTruncatingAndAppending() throws Exception
log.commit( toCommit );

// then
assertThat( log.entryExists( toCommit ), is( true ) );
assertThat( log.entryExists( lastAppended ), is( true ) );
assertThat( log.entryExists( toTruncate ), is( true ) ); // index is "reused"
assertThat( log.appendIndex(), is( lastAppended ) );
assertThat( log.readEntryTerm( toCommit ), is( 0L ) );
assertThat( log.readEntryTerm( lastAppended ), is( 2L ) );
}
Expand All @@ -296,9 +280,7 @@ public void shouldCommitAfterAppendingAndTruncating() throws Exception
log.commit( toCommit );

// then
assertThat( log.entryExists( toCommit ), is( true ) );
assertThat( log.entryExists( lastAppended ), is( false ) );
assertThat( log.entryExists( toTruncate ), is( false ) );
assertThat( log.appendIndex(), is( toCommit ) );
assertThat( log.readEntryTerm( toCommit ), is( 0L ) );
}

Expand All @@ -324,7 +306,7 @@ public void shouldNotAllowTruncationAtLastCommit() throws Exception
}

// then
assertThat( log.entryExists( toCommit ), is( true ) );
assertThat( log.appendIndex(), is( toCommit ) );
}

@Test
Expand All @@ -350,7 +332,6 @@ public void shouldNotAllowTruncationBeforeLastCommit() throws Exception
}

// then
assertThat( log.entryExists( toCommit ), is( true ) );
assertThat( log.entryExists( toTryToTruncate ), is( true ) );
assertThat( log.appendIndex(), is( toCommit ) );
}
}
Expand Up @@ -55,7 +55,6 @@ public void shouldAppendDataAndNotCommitImmediately() throws Exception
verifyCurrentLogAndNewLogLoadedFromFileSystem( log, fileSystem, myLog -> {
assertThat( myLog.appendIndex(), is( 0L ) );
assertThat( myLog.commitIndex(), is( -1L ) );
assertThat( myLog.entryExists( 0 ), is( true ) );
assertThat( myLog.readLogEntry( 0 ), equalTo( logEntry ) );
} );
}
Expand All @@ -73,7 +72,6 @@ public void shouldAppendAndCommit() throws Exception
verifyCurrentLogAndNewLogLoadedFromFileSystem( log, fileSystem, myLog -> {
assertThat( myLog.appendIndex(), is( 0L ) );
assertThat( myLog.commitIndex(), is( 0L ) );
assertThat( myLog.entryExists( 0 ), is( true ) );
} );
}

Expand All @@ -93,14 +91,8 @@ public void shouldAppendAfterReloadingFromFileSystem() throws Exception
log.append( logEntryB );

assertThat( log.appendIndex(), is( 1L ) );

assertThat( log.entryExists( 0 ), is( true ) );
assertThat( log.readLogEntry( 0 ), is( logEntryA ) );

assertThat( log.entryExists( 1 ), is( true ) );
assertThat( log.readLogEntry( 1 ), is( logEntryB ) );

assertThat( log.entryExists( 2 ), is( false ) );
}

@Test
Expand All @@ -118,8 +110,6 @@ public void shouldTruncatePreviouslyAppendedEntries() throws Exception

verifyCurrentLogAndNewLogLoadedFromFileSystem( log, fileSystem, myLog -> {
assertThat( myLog.appendIndex(), is( 0L ) );
assertThat( myLog.entryExists( 0 ), is( true ) );
assertThat( myLog.entryExists( 1 ), is( false ) );
} );
}

Expand Down Expand Up @@ -149,10 +139,6 @@ public void shouldReplacePreviouslyAppendedEntries() throws Exception
assertThat( myLog.readLogEntry( 0 ), equalTo( logEntryA ) );
assertThat( myLog.readLogEntry( 1 ), equalTo( logEntryD ) );
assertThat( myLog.readLogEntry( 2 ), equalTo( logEntryE ) );
assertThat( myLog.entryExists( 0 ), is( true ) );
assertThat( myLog.entryExists( 1 ), is( true ) );
assertThat( myLog.entryExists( 2 ), is( true ) );
assertThat( myLog.entryExists( 3 ), is( false ) );
} );
}

Expand Down
Expand Up @@ -97,14 +97,6 @@ public long readEntryTerm( long logIndex ) throws IOException
return term;
}

@Override
public boolean entryExists( long logIndex ) throws IOException
{
boolean exists = expected.entryExists( logIndex );
assertEquals( exists, other.entryExists( logIndex ) );
return exists;
}

@Override
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException
{
Expand Down Expand Up @@ -144,7 +136,6 @@ private void verifyDirectLookupBackwards( InMemoryRaftLog expected, RaftLog othe

private void directAssertions( InMemoryRaftLog expected, RaftLog other, long logIndex ) throws IOException
{
assertEquals( expected.entryExists( logIndex ), other.entryExists( logIndex ) );
assertEquals( expected.readEntryTerm( logIndex ), other.readEntryTerm( logIndex ) );
assertEquals( expected.readLogEntry( logIndex ), other.readLogEntry( logIndex ) );
}
Expand Down
Expand Up @@ -90,11 +90,6 @@ public long appendIndex()
return 0;
}

@Override public boolean entryExists( long logIndex )
{
return false;
}

@Override
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException
{
Expand Down
Expand Up @@ -184,11 +184,6 @@ public long appendIndex()
return 0;
}

@Override public boolean entryExists( long logIndex )
{
return false;
}

@Override
public IOCursor<RaftLogEntry> getEntryCursor( long fromIndex ) throws IOException
{
Expand Down

0 comments on commit d22631e

Please sign in to comment.