[cassandra] branch cassandra-3.0 updated: Race in CompactionExecutorTest

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jonmeredith pushed a commit to branch cassandra-3.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/cassandra-3.0 by this push:
     new 071ecb5  Race in CompactionExecutorTest
071ecb5 is described below

commit 071ecb52465837c90520423c2bc80eb886936953
Author: Jon Meredith <jo...@apache.org>
AuthorDate: Thu Jan 6 11:12:21 2022 -0700

    Race in CompactionExecutorTest
    
    patch by Jon Meredith; reviewed by Josh McKenzie for CASSANDRA-17239
---
 CHANGES.txt                                        |  1 +
 .../db/compaction/CompactionExecutorTest.java      | 32 +++++++++++++---------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 085e1ff..6ed2033 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -22,6 +22,7 @@
  * Use JMX to validate nodetool --jobs parameter (CASSANDRA-16104)
  * Handle properly UnsatisfiedLinkError in NativeLibrary#getProcessID() (CASSANDRA-16578)
  * Remove mutation data from error log message (CASSANDRA-16817)
+ * Race in CompactionExecutorTest (CASSANDRA-17239)
 Merged from 2.2:
  * Add python2 location to RPMs (CASSANDRA-16822)
 
diff --git a/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java
index 9b07da9..b1f29b3 100644
--- a/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java
+++ b/test/unit/org/apache/cassandra/db/compaction/CompactionExecutorTest.java
@@ -18,13 +18,13 @@
 
 package org.apache.cassandra.db.compaction;
 
-import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.apache.cassandra.concurrent.DebuggableThreadPoolExecutor;
+import org.apache.cassandra.utils.concurrent.SimpleCondition;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -32,8 +32,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)
         {
@@ -42,6 +46,7 @@ public class CompactionExecutorTest
                 t = DebuggableThreadPoolExecutor.extractThrowable(r);
             }
             testTaskThrowable = t;
+            afterExecuteCompleted.signalAll();
         }
         @Override
         protected void beforeExecute(Thread t, Runnable r)
@@ -54,6 +59,8 @@ public class CompactionExecutorTest
     public void setup()
     {
         executor = new TestTaskExecutor();
+        testTaskThrowable = null;
+        afterExecuteCompleted = new SimpleCondition();
     }
 
     @After
@@ -63,16 +70,19 @@ public class CompactionExecutorTest
         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");
     }
@@ -80,13 +90,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");
     }
@@ -94,13 +102,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");
     }

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org