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

dlmarion pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/2.1 by this push:
     new 5715f06fc8 Fix ExternalCompaction_1_IT.testPartialCompaction failure 
(#5054)
5715f06fc8 is described below

commit 5715f06fc8eba911c89ffa86ee12dff1a64127aa
Author: Dave Marion <dlmar...@apache.org>
AuthorDate: Thu Nov 14 08:01:08 2024 -0500

    Fix ExternalCompaction_1_IT.testPartialCompaction failure (#5054)
    
    The test was timing out because it never started running compactions
    created by the test method. Instead, the compactor process was
    running compactions created by the previous test method because
    the previous test created a table with a lot of files, started a
    user compaction, then cancelled the user compaction. The recent
    changes in #5026 caused a bunch of system compactions to be
    generated for the table. The two test methods share the same
    compaction queue, so the compactor was busy running the system
    compactions.
    
    To fix this issue I backported a property added in #3955 that
    makes the compactor cancel check method time configurable and
    I deleted the table in the test method that created a lot of files.
    
    Closes #5052
---
 core/src/main/java/org/apache/accumulo/core/conf/Property.java   | 6 ++++++
 .../src/main/java/org/apache/accumulo/compactor/Compactor.java   | 5 ++---
 .../accumulo/test/compaction/ExternalCompactionTestUtils.java    | 1 +
 .../apache/accumulo/test/compaction/ExternalCompaction_1_IT.java | 9 +++++++++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java 
b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index 87a3ceb12d..0fa037366c 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -1466,6 +1466,12 @@ public enum Property {
   @Experimental
   COMPACTOR_PREFIX("compactor.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo 
compactor server.", "2.1.0"),
+  COMPACTOR_CANCEL_CHECK_INTERVAL("compactor.cancel.check.interval", "5m",
+      PropertyType.TIMEDURATION,
+      "Interval at which Compactors will check to see if the currently 
executing compaction"
+          + " should be cancelled. This checks for situations like was the 
tablet deleted (split "
+          + " and merge do this), was the table deleted, was a user compaction 
canceled, etc.",
+      "2.1.4"),
   @Experimental
   COMPACTOR_MIN_JOB_WAIT_TIME("compactor.wait.time.job.min", "1s", 
PropertyType.TIMEDURATION,
       "The minimum amount of time to wait between checks for the next 
compaction job, backing off"
diff --git 
a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java 
b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
index c97d696a4a..e03032326f 100644
--- 
a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
+++ 
b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
@@ -19,7 +19,6 @@
 package org.apache.accumulo.compactor;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.MINUTES;
 import static 
org.apache.accumulo.core.util.UtilWaitThread.sleepUninterruptibly;
 
 import java.io.IOException;
@@ -154,7 +153,6 @@ public class Compactor extends AbstractServer implements 
MetricsProducer, Compac
 
   private static final Logger LOG = LoggerFactory.getLogger(Compactor.class);
   private static final long TIME_BETWEEN_GC_CHECKS = 5000;
-  private static final long TIME_BETWEEN_CANCEL_CHECKS = MINUTES.toMillis(5);
 
   private static final long TEN_MEGABYTES = 10485760;
 
@@ -701,7 +699,8 @@ public class Compactor extends AbstractServer implements 
MetricsProducer, Compac
     var schedExecutor = ThreadPools.getServerThreadPools()
         .createGeneralScheduledExecutorService(getConfiguration());
     startGCLogger(schedExecutor);
-    startCancelChecker(schedExecutor, TIME_BETWEEN_CANCEL_CHECKS);
+    startCancelChecker(schedExecutor,
+        
getConfiguration().getTimeInMillis(Property.COMPACTOR_CANCEL_CHECK_INTERVAL));
 
     LOG.info("Compactor started, waiting for work");
     try {
diff --git 
a/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompactionTestUtils.java
 
b/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompactionTestUtils.java
index 2d37f7518c..cf33f1db0b 100644
--- 
a/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompactionTestUtils.java
+++ 
b/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompactionTestUtils.java
@@ -233,6 +233,7 @@ public class ExternalCompactionTestUtils {
     
cfg.setProperty(Property.COMPACTION_COORDINATOR_DEAD_COMPACTOR_CHECK_INTERVAL, 
"5s");
     
cfg.setProperty(Property.COMPACTION_COORDINATOR_TSERVER_COMPACTION_CHECK_INTERVAL,
 "3s");
     cfg.setProperty(Property.COMPACTION_COORDINATOR_THRIFTCLIENT_PORTSEARCH, 
"true");
+    cfg.setProperty(Property.COMPACTOR_CANCEL_CHECK_INTERVAL, "5s");
     cfg.setProperty(Property.COMPACTOR_PORTSEARCH, "true");
     cfg.setProperty(Property.GENERAL_THREADPOOL_SIZE, "10");
     cfg.setProperty(Property.MANAGER_FATE_THREADPOOL_SIZE, "10");
diff --git 
a/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_1_IT.java
 
b/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_1_IT.java
index fd50fd199e..ba6c0167b3 100644
--- 
a/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_1_IT.java
+++ 
b/test/src/main/java/org/apache/accumulo/test/compaction/ExternalCompaction_1_IT.java
@@ -619,6 +619,15 @@ public class ExternalCompaction_1_IT extends 
SharedMiniClusterBase {
 
       client.tableOperations().cancelCompaction(table1);
       t.join();
+
+      // This test created a lot of files (bw.flush in a loop) which will
+      // cause system compactions to be started for this table because
+      // it will be over the max number of files for the tablets. Without
+      // deleting the table, this test method will end and another may start
+      // using the same compaction queue, causing the next test method to
+      // possibly time out.
+      client.tableOperations().delete(table1);
+
     }
   }
 

Reply via email to