diff --git a/CHANGES.txt b/CHANGES.txt index 0fd51ec50f..c84e6436ba 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -45,6 +45,7 @@ Merged from 3.0: * Catch UnsatisfiedLinkError in WindowsTimer (CASSANDRA-16085) * Avoid removing batch when it's not created during view replication (CASSANDRA-16175) * Make the addition of regular column to COMPACT tables throw an InvalidRequestException (CASSANDRA-14564) + * Race in CompactionExecutorTest (CASSANDRA-17239) 4.0.1 * Tolerate missing DNS entry when completing a host replacement (CASSANDRA-16873) diff --git a/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java index ab3d9e5c1e..cca29975dc 100644 --- a/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java @@ -18,7 +18,6 @@ package org.apache.cassandra.db.compaction; -import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import org.junit.After; @@ -27,6 +26,7 @@ import org.junit.Before; import org.junit.Test; import org.apache.cassandra.concurrent.DebuggableThreadPoolExecutor; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.utils.concurrent.SimpleCondition; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -34,8 +34,12 @@ import static org.junit.Assert.assertNotNull; public class CompactionExecutorTest { static Throwable testTaskThrowable = null; + static SimpleCondition afterExecuteCompleted = null; private static class TestTaskExecutor extends CompactionManager.CompactionExecutor { + // afterExecute runs immediately after the task completes, but it may + // race with the main thread checking the result, so make the main thread wait + // with a simple condition @Override public void afterExecute(Runnable r, Throwable t) { @@ -44,6 +48,7 @@ public class CompactionExecutorTest t = DebuggableThreadPoolExecutor.extractThrowable(r); } testTaskThrowable = t; + afterExecuteCompleted.signalAll(); } @Override protected void beforeExecute(Thread t, Runnable r) @@ -57,6 +62,8 @@ public class CompactionExecutorTest { DatabaseDescriptor.daemonInitialization(); executor = new TestTaskExecutor(); + testTaskThrowable = null; + afterExecuteCompleted = new SimpleCondition(); } @After @@ -66,16 +73,19 @@ public class CompactionExecutorTest Assert.assertTrue(executor.awaitTermination(1, TimeUnit.MINUTES)); } + void awaitExecution() throws Exception + { + assert afterExecuteCompleted.await(10, TimeUnit.SECONDS) : "afterExecute failed to complete"; + } + @Test public void testFailedRunnable() throws Exception { - testTaskThrowable = null; - Future tt = executor.submitIfRunning( + executor.submitIfRunning( () -> { assert false : "testFailedRunnable"; } , "compactionExecutorTest"); - while (!tt.isDone()) - Thread.sleep(10); + awaitExecution(); assertNotNull(testTaskThrowable); assertEquals(testTaskThrowable.getMessage(), "testFailedRunnable"); } @@ -83,13 +93,11 @@ public class CompactionExecutorTest @Test public void testFailedCallable() throws Exception { - testTaskThrowable = null; - Future tt = executor.submitIfRunning( + executor.submitIfRunning( () -> { assert false : "testFailedCallable"; return 1; } , "compactionExecutorTest"); - while (!tt.isDone()) - Thread.sleep(10); + awaitExecution(); assertNotNull(testTaskThrowable); assertEquals(testTaskThrowable.getMessage(), "testFailedCallable"); } @@ -97,13 +105,11 @@ public class CompactionExecutorTest @Test public void testExceptionRunnable() throws Exception { - testTaskThrowable = null; - Future tt = executor.submitIfRunning( + executor.submitIfRunning( () -> { throw new RuntimeException("testExceptionRunnable"); } , "compactionExecutorTest"); - while (!tt.isDone()) - Thread.sleep(10); + awaitExecution(); assertNotNull(testTaskThrowable); assertEquals(testTaskThrowable.getMessage(), "testExceptionRunnable"); }