Skip to content

Commit

Permalink
Add missing latch await call result verification
Browse files Browse the repository at this point in the history
Add missing result checks for latch.await(some time) calls.
  • Loading branch information
MishaDemianenko committed Oct 19, 2015
1 parent c6c3a7b commit 8587b53
Show file tree
Hide file tree
Showing 13 changed files with 35 additions and 46 deletions.
Expand Up @@ -38,6 +38,7 @@
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.neo4j.bolt.runtime.integration.SessionMatchers.failedWith; import static org.neo4j.bolt.runtime.integration.SessionMatchers.failedWith;
import static org.neo4j.bolt.runtime.integration.SessionMatchers.streamContaining; import static org.neo4j.bolt.runtime.integration.SessionMatchers.streamContaining;
import static org.neo4j.bolt.runtime.integration.SessionMatchers.success; import static org.neo4j.bolt.runtime.integration.SessionMatchers.success;
Expand Down Expand Up @@ -273,7 +274,7 @@ public void completed( Object attachment )
} ); } );


// Then // Then
pullAllCallbackCalled.await( 30, TimeUnit.SECONDS ); assertTrue( pullAllCallbackCalled.await( 30, TimeUnit.SECONDS ) );
final Neo4jError err = error.get(); final Neo4jError err = error.get();
assertThat( err.status(), equalTo( (Status)Status.General.UnknownFailure ) ); assertThat( err.status(), equalTo( (Status)Status.General.UnknownFailure ) );
assertThat( err.message(), containsString( "Ooopsies!" ) ); assertThat( err.message(), containsString( "Ooopsies!" ) );
Expand Down
Expand Up @@ -34,6 +34,7 @@


import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.SECONDS;


import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -137,7 +138,7 @@ public void shouldBlockAccessDuringFlipAndThenDelegateToCorrectContext() throws
triggerFinishFlip, triggerExternalAccess ) ); triggerFinishFlip, triggerExternalAccess ) );


// And I wait until the flipping thread is in the middle of "the flip" // And I wait until the flipping thread is in the middle of "the flip"
triggerExternalAccess.await( 10, SECONDS ); assertTrue( triggerExternalAccess.await( 10, SECONDS ) );


// And another thread comes along and drops the index // And another thread comes along and drops the index
Future<Void> dropIndexFuture = dropIndexThread.executeDontWait( dropTheIndex( flippable ) ); Future<Void> dropIndexFuture = dropIndexThread.executeDontWait( dropTheIndex( flippable ) );
Expand Down Expand Up @@ -189,7 +190,7 @@ public Void doWork( Void state ) throws FlipFailedKernelException
public Void call() public Void call()
{ {
triggerExternalAccess.countDown(); triggerExternalAccess.countDown();
awaitLatch( triggerFinishFlip ); assertTrue( awaitLatch( triggerFinishFlip ) );
return null; return null;
} }
}, null ); }, null );
Expand Down
Expand Up @@ -97,11 +97,11 @@ public static <T> T awaitFuture( Future<T> future )
} }
} }


public static void awaitLatch( CountDownLatch latch ) public static boolean awaitLatch( CountDownLatch latch )
{ {
try try
{ {
latch.await( 10, SECONDS ); return latch.await( 10, SECONDS );
} }
catch ( InterruptedException e ) catch ( InterruptedException e )
{ {
Expand Down
Expand Up @@ -322,8 +322,7 @@ public void testDeadlockDetection() throws InterruptedException
executor.execute( readerLockNode1 ); executor.execute( readerLockNode1 );


// Deadlock should occur // Deadlock should occur
deadLockDetector.await( 1000, TimeUnit.MILLISECONDS ); Assert.assertTrue( "Deadlock was detected as expected.", deadLockDetector.await( 1000, TimeUnit.MILLISECONDS ) );
Assert.assertTrue( "Deadlock was detected as expected.", true );
} }


@Test( timeout = 1000 ) @Test( timeout = 1000 )
Expand Down
27 changes: 10 additions & 17 deletions community/kernel/src/test/java/org/neo4j/test/DoubleLatch.java
Expand Up @@ -22,9 +22,11 @@
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;


import static org.junit.Assert.assertTrue;

public class DoubleLatch public class DoubleLatch
{ {
private static final int FIVE_MINUTES = 5 * 60 * 1000; private static final int FIVE_MINUTES = 5;
private final CountDownLatch startSignal; private final CountDownLatch startSignal;
private final CountDownLatch finishSignal; private final CountDownLatch finishSignal;
private final int numberOfContestants; private final int numberOfContestants;
Expand Down Expand Up @@ -75,24 +77,15 @@ public void awaitFinish()


public static void awaitLatch( CountDownLatch latch ) public static void awaitLatch( CountDownLatch latch )
{ {
long deadline = System.currentTimeMillis() + FIVE_MINUTES; try
long remaining; {

assertTrue( "Latch specified waiting time elapsed.", latch.await( FIVE_MINUTES, TimeUnit.MINUTES ) );
while( ( remaining = deadline - System.currentTimeMillis() ) >= 0 ) }
catch ( InterruptedException e )
{ {
try Thread.interrupted();
{ throw new RuntimeException( "Thread interrupted while waiting on latch", e );
latch.await( remaining, TimeUnit.MILLISECONDS );
return;
}
catch ( InterruptedException e )
{
Thread.interrupted();
new RuntimeException( "Thread interrupted while waiting on latch", e).printStackTrace();
}
Thread.yield();
} }
throw new RuntimeException( "Failed to acquire latch" );
} }


@Override @Override
Expand Down
Expand Up @@ -65,7 +65,7 @@ public void shouldReturnCorrectStatusCodeOnDeadlock() throws Exception
otherThread.execute( writeToFirstAndSecond() ); otherThread.execute( writeToFirstAndSecond() );


// and I wait for those locks to be pending // and I wait for those locks to be pending
secondNodeLocked.await(10, TimeUnit.SECONDS); assertTrue( secondNodeLocked.await( 10, TimeUnit.SECONDS ) );
Thread.sleep( 1000 ); Thread.sleep( 1000 );


// and I then try and lock node:Second in the first transaction // and I then try and lock node:Second in the first transaction
Expand Down
Expand Up @@ -88,7 +88,7 @@ public void shouldSendAMessageFromAClientWhichIsReceivedByAServer() throws Excep


// then // then


latch.await( 5, TimeUnit.SECONDS ); assertTrue( latch.await( 5, TimeUnit.SECONDS ) );


assertTrue( "server1 should have processed the message", server1.processedMessage() ); assertTrue( "server1 should have processed the message", server1.processedMessage() );
assertTrue( "server2 should have processed the message", server2.processedMessage() ); assertTrue( "server2 should have processed the message", server2.processedMessage() );
Expand Down
Expand Up @@ -193,10 +193,7 @@ public void leftCluster( InstanceId instanceId, URI member )
slave.shutdown(); slave.shutdown();


// Make sure that the slave has left, because shutdown() may return before the master knows // Make sure that the slave has left, because shutdown() may return before the master knows
if (!slaveLeftLatch.await(60, TimeUnit.SECONDS)) assertTrue( "Timeout waiting for slave to leave", slaveLeftLatch.await( 60, TimeUnit.SECONDS ) );
{
throw new IllegalStateException( "Timeout waiting for slave to leave" );
}


long nodeId; long nodeId;
try ( Transaction tx = master.beginTx() ) try ( Transaction tx = master.beginTx() )
Expand Down
Expand Up @@ -48,6 +48,7 @@
import org.neo4j.test.TargetDirectory; import org.neo4j.test.TargetDirectory;


import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;


/** /**
* This test case ensures that updates in HA are first written out to the log * This test case ensures that updates in HA are first written out to the log
Expand Down Expand Up @@ -120,10 +121,7 @@ public void leftCluster( InstanceId instanceId, URI member )


dbToKill.shutdown(); dbToKill.shutdown();


if ( !latch1.await( 60, TimeUnit.SECONDS ) ) assertTrue( "Timeout waiting for instance to leave cluster", latch1.await( 60, TimeUnit.SECONDS ) );
{
throw new IllegalStateException( "Timeout waiting for instance to leave cluster" );
}


addNode( master ); // this will be pulled by tne next start up, applied addNode( master ); // this will be pulled by tne next start up, applied
// but not written to log. // but not written to log.
Expand All @@ -147,10 +145,7 @@ public void failed( InstanceId server )


runInOtherJvmToGetExitCode( targetDirectory.getAbsolutePath(), "" + toKill ); runInOtherJvmToGetExitCode( targetDirectory.getAbsolutePath(), "" + toKill );


if ( !latch2.await( 60, TimeUnit.SECONDS ) ) assertTrue( "Timeout waiting for instance to fail", latch2.await( 60, TimeUnit.SECONDS ) );
{
throw new IllegalStateException( "Timeout waiting for instance to fail" );
}


// This is to allow other instances to mark the dead instance as failed, otherwise on startup it will be denied. // This is to allow other instances to mark the dead instance as failed, otherwise on startup it will be denied.
// TODO This is to demonstrate shortcomings in our design. Fix this, you ugly, ugly hacker // TODO This is to demonstrate shortcomings in our design. Fix this, you ugly, ugly hacker
Expand Down
Expand Up @@ -43,6 +43,7 @@
import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;


import static org.junit.Assert.assertTrue;
import static org.neo4j.kernel.impl.ha.ClusterManager.allSeesAllAsAvailable; import static org.neo4j.kernel.impl.ha.ClusterManager.allSeesAllAsAvailable;
import static org.neo4j.kernel.impl.ha.ClusterManager.fromXml; import static org.neo4j.kernel.impl.ha.ClusterManager.fromXml;


Expand All @@ -69,7 +70,7 @@ public void testMasterElectionAfterMasterRecoversInSlaveOnlyCluster() throws Thr


final ClusterManager.RepairKit repairKit = cluster.fail( master ); final ClusterManager.RepairKit repairKit = cluster.fail( master );


masterFailedLatch.await( 60, TimeUnit.SECONDS ); assertTrue( masterFailedLatch.await( 60, TimeUnit.SECONDS ) );


repairKit.repair(); repairKit.repair();


Expand Down
Expand Up @@ -65,6 +65,7 @@


import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.SECONDS;


import static org.junit.Assert.assertTrue;
import static org.neo4j.cluster.protocol.cluster.ClusterConfiguration.COORDINATOR; import static org.neo4j.cluster.protocol.cluster.ClusterConfiguration.COORDINATOR;
import static org.neo4j.function.Predicates.not; import static org.neo4j.function.Predicates.not;
import static org.neo4j.kernel.impl.ha.ClusterManager.allSeesAllAsAvailable; import static org.neo4j.kernel.impl.ha.ClusterManager.allSeesAllAsAvailable;
Expand Down Expand Up @@ -155,11 +156,11 @@ else if ( instanceIdOf( slave2 ).equals( server ) )


// fail slave1 and await master to spot the failure // fail slave1 and await master to spot the failure
RepairKit slave1RepairKit = cluster.fail( slave1 ); RepairKit slave1RepairKit = cluster.fail( slave1 );
slave1Left.await(60, SECONDS); assertTrue( slave1Left.await( 60, SECONDS ) );


// fail slave2 and await master to spot the failure // fail slave2 and await master to spot the failure
RepairKit slave2RepairKit = cluster.fail( slave2 ); RepairKit slave2RepairKit = cluster.fail( slave2 );
slave2Left.await(60, SECONDS); assertTrue( slave2Left.await( 60, SECONDS ) );


// master loses quorum and goes to PENDING, cluster is unavailable // master loses quorum and goes to PENDING, cluster is unavailable
cluster.await( not( masterAvailable() ) ); cluster.await( not( masterAvailable() ) );
Expand Down Expand Up @@ -199,8 +200,9 @@ else if ( instanceIdOf( newSlave2 ).equals( unavailableId ) )


// attempt to perform transactions on both slaves throws, election is triggered // attempt to perform transactions on both slaves throws, election is triggered
attemptTransactions( newSlave1, newSlave2 ); attemptTransactions( newSlave1, newSlave2 );
slave1Unavailable.await( 60, TimeUnit.SECONDS ); // set a timeout in case the instance does not have stale epoch // set a timeout in case the instance does not have stale epoch
slave2Unavailable.await( 60, TimeUnit.SECONDS ); assertTrue( slave1Unavailable.await( 60, TimeUnit.SECONDS ) );
assertTrue( slave2Unavailable.await( 60, TimeUnit.SECONDS ) );


// THEN: done with election, cluster feels good and able to serve transactions // THEN: done with election, cluster feels good and able to serve transactions
cluster.info( "Waiting for cluster to stabilize" ); cluster.info( "Waiting for cluster to stabilize" );
Expand Down Expand Up @@ -245,7 +247,7 @@ public void enteredCluster( ClusterConfiguration clusterConfiguration )
clusterClient.life.start(); clusterClient.life.start();


// Then // Then
latch.await( 20, SECONDS ); assertTrue( latch.await( 20, SECONDS ) );
assertEquals( new InstanceId( 2 ), coordinatorIdWhenReJoined.get() ); assertEquals( new InstanceId( 2 ), coordinatorIdWhenReJoined.get() );
} }


Expand Down
Expand Up @@ -101,7 +101,7 @@ public void receive( Payload value )
proc.destroy(); proc.destroy();
proc = null; proc = null;


newMasterAvailableLatch.await( 60, SECONDS ); assertTrue( newMasterAvailableLatch.await( 60, SECONDS ) );


assertTrue( dbWithId2.isMaster() ); assertTrue( dbWithId2.isMaster() );
assertTrue( !dbWithId3.isMaster() ); assertTrue( !dbWithId3.isMaster() );
Expand Down
Expand Up @@ -252,7 +252,7 @@ public void run()


t.run(); t.run();


latch.await( 1000, TimeUnit.MILLISECONDS ); assertTrue( latch.await( 1000, TimeUnit.MILLISECONDS ) );


t.join(); t.join();
} }
Expand Down

0 comments on commit 8587b53

Please sign in to comment.