[hbase] branch branch-2.3 updated: HBASE-24940: runCatalogJanitor() API should return -1 to indicate already running status

[vj...@apache.org]

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

vjasani pushed a commit to branch branch-2.3
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2.3 by this push:
     new dc79b26  HBASE-24940: runCatalogJanitor() API should return -1 to indicate already running status
dc79b26 is described below

commit dc79b267618c3c05ae0cb26c6be925abb415bc5e
Author: Mohammad Arshad <ar...@apache.org>
AuthorDate: Thu Sep 3 15:46:10 2020 +0530

    HBASE-24940: runCatalogJanitor() API should return -1 to indicate already running status
    
    Closes #2331
    
    Signed-off-by: Viraj Jasani <vj...@apache.org>
---
 .../java/org/apache/hadoop/hbase/client/Admin.java |  2 +-
 .../apache/hadoop/hbase/master/CatalogJanitor.java |  4 +++-
 .../hadoop/hbase/master/TestCatalogJanitor.java    | 25 ++++++++++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
index d34f56f..75c71a3 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -1333,7 +1333,7 @@ public interface Admin extends Abortable, Closeable {
   /**
    * Ask for a scan of the catalog table.
    *
-   * @return the number of entries cleaned
+   * @return the number of entries cleaned. Returns -1 if previous run is in progress.
    * @throws IOException if a remote or network exception occurs
    * @deprecated Since 2.0.0. Will be removed in 3.0.0. Use {@link #runCatalogJanitor()}}
    * instead.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
index af413f3..33ba382 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
@@ -160,6 +160,7 @@ public class CatalogJanitor extends ScheduledChore {
    * Run janitorial scan of catalog <code>hbase:meta</code> table looking for
    * garbage to collect.
    * @return How many items gc'd whether for merge or split.
+   *   Returns -1 if previous scan is in progress.
    */
   @VisibleForTesting
   public int scan() throws IOException {
@@ -167,7 +168,8 @@ public class CatalogJanitor extends ScheduledChore {
     try {
       if (!alreadyRunning.compareAndSet(false, true)) {
         LOG.debug("CatalogJanitor already running");
-        return gcs;
+        // -1 indicates previous scan is in progress
+        return -1;
       }
       this.lastReport = scanForReport();
       if (!this.lastReport.isEmpty()) {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
index 97a0f6f..5486817 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
@@ -25,6 +25,8 @@ import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.spy;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Objects;
@@ -555,6 +557,29 @@ public class TestCatalogJanitor {
     assertArchiveEqualToOriginal(storeFiles, archivedStoreFiles, fs, true);
   }
 
+  @Test
+  public void testAlreadyRunningStatus() throws Exception {
+    int numberOfThreads = 2;
+    List<Integer> gcValues = new ArrayList<>();
+    Thread[] threads = new Thread[numberOfThreads];
+    for (int i = 0; i < numberOfThreads; i++) {
+      threads[i] = new Thread(() -> {
+        try {
+          gcValues.add(janitor.scan());
+        } catch (IOException e) {
+          throw new RuntimeException(e);
+        }
+      });
+    }
+    for (int i = 0; i < numberOfThreads; i++) {
+      threads[i].start();
+    }
+    for (int i = 0; i < numberOfThreads; i++) {
+      threads[i].join();
+    }
+    assertTrue("One janitor.scan() call should have returned -1", gcValues.contains(-1));
+  }
+
   private FileStatus[] addMockStoreFiles(int count, MasterServices services, Path storedir)
       throws IOException {
     // get the existing store files