Merge pull request #14807 from cdapio/bugfix/CDAP-20206

[CDAP-20206] Fix flaky CDAPLogAppenderTest
cdapio · Dec 20, 2022 · 83fa515 · 83fa515
2 parents 1da99a4 + c9582e4
commit 83fa515
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 6 deletions.
diff --git a/cdap-watchdog/src/main/java/io/cdap/cdap/logging/appender/system/CDAPLogAppender.java b/cdap-watchdog/src/main/java/io/cdap/cdap/logging/appender/system/CDAPLogAppender.java
@@ -66,10 +66,22 @@ public class CDAPLogAppender extends AppenderBase<ILoggingEvent> implements Flus
   private int fileRetentionDurationDays;
   private int fileCleanupBatchSize;
 
-  public CDAPLogAppender() {
+  private final boolean disableLogCleanerForTest;
+
+  /**
+   * Unit tests do not need to use a log cleaner because JUnit automatically cleans up temporary directories.
+   * LogCleaner is disabled in tests to avoid deleting the directory required by another test when run in parallel.
+   */
+  @VisibleForTesting
+  CDAPLogAppender(boolean disableLogCleanerForTest) {
+    this.disableLogCleanerForTest = disableLogCleanerForTest;
     setName(getClass().getName());
   }
 
+  public CDAPLogAppender() {
+    this(false);
+  }
+
   /**
    * Sets the permissions for directory created by this appender. This is called by the logback framework.
    */
@@ -146,7 +158,7 @@ public void start() {
                                           syncIntervalBytes,
                                           new FileMetaDataWriter(context.getTransactionRunner()),
                                           context.getLocationFactory());
-      if (context.getInstanceId() == 0) {
+      if (context.getInstanceId() == 0 && !disableLogCleanerForTest) {
         scheduledExecutorService =
           Executors.newSingleThreadScheduledExecutor(Threads.createDaemonThreadFactory("log-clean-up"));
         FileMetadataCleaner fileMetadataCleaner = new FileMetadataCleaner(context.getTransactionRunner());
@@ -235,7 +247,9 @@ public void stop() {
       if (logFileManager != null) {
         logFileManager.close();
       }
-      scheduledExecutorService.shutdownNow();
+      if (scheduledExecutorService != null) {
+        scheduledExecutorService.shutdownNow();
+      }
     } finally {
       super.stop();
     }

diff --git a/cdap-watchdog/src/test/java/io/cdap/cdap/logging/appender/system/CDAPLogAppenderTest.java b/cdap-watchdog/src/test/java/io/cdap/cdap/logging/appender/system/CDAPLogAppenderTest.java
@@ -125,7 +125,7 @@ public static void cleanUp() {
   @Test
   public void testCDAPLogAppender() {
     int syncInterval = 1024 * 1024;
-    CDAPLogAppender cdapLogAppender = new CDAPLogAppender();
+    CDAPLogAppender cdapLogAppender = new CDAPLogAppender(true);
 
     cdapLogAppender.setSyncIntervalBytes(syncInterval);
     cdapLogAppender.setMaxFileLifetimeMs(TimeUnit.DAYS.toMillis(1));
@@ -190,7 +190,7 @@ public void testCDAPLogAppender() {
   public void testCDAPLogAppenderRotation() throws Exception {
     int syncInterval = 1024 * 1024;
     FileMetaDataReader fileMetaDataReader = injector.getInstance(FileMetaDataReader.class);
-    CDAPLogAppender cdapLogAppender = new CDAPLogAppender();
+    CDAPLogAppender cdapLogAppender = new CDAPLogAppender(true);
     AppenderContext context = new LocalAppenderContext(injector.getInstance(TransactionRunner.class),
                                                        injector.getInstance(LocationFactory.class),
                                                        new NoOpMetricsCollectionService());
@@ -261,7 +261,7 @@ public void testCDAPLogAppenderRotation() throws Exception {
   public void testCDAPLogAppenderSizeBasedRotation() throws Exception {
     int syncInterval = 1024 * 1024;
     FileMetaDataReader fileMetaDataReader = injector.getInstance(FileMetaDataReader.class);
-    CDAPLogAppender cdapLogAppender = new CDAPLogAppender();
+    CDAPLogAppender cdapLogAppender = new CDAPLogAppender(true);
     AppenderContext context = new LocalAppenderContext(injector.getInstance(TransactionRunner.class),
                                                        injector.getInstance(LocationFactory.class),
                                                        new NoOpMetricsCollectionService());