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

[vjasani]

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

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


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

commit 76db1e1791cfa2a6685580af254a8180363a8dba
Author: Mohammad Arshad <ars...@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
    
    Co-authored-by: Viraj Jasani <vjas...@apache.org>
    
    Signed-off-by: Viraj Jasani <vjas...@apache.org>
---
 .../java/org/apache/hadoop/hbase/client/Admin.java |  2 +-
 .../apache/hadoop/hbase/master/CatalogJanitor.java |  4 ++-
 .../hadoop/hbase/master/TestCatalogJanitor.java    | 32 ++++++++++++++++++++++
 3 files changed, 36 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 4896df9..c32b391 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
@@ -779,7 +779,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
    */
   int runCatalogScan() throws IOException;
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 549c38f..911248c 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
@@ -238,7 +238,9 @@ public class CatalogJanitor extends ScheduledChore {
   int scan() throws IOException {
     try {
       if (!alreadyRunning.compareAndSet(false, true)) {
-        return 0;
+        LOG.debug("CatalogJanitor already running");
+        // -1 indicates previous scan is in progress
+        return -1;
       }
       Triple<Integer, Map<HRegionInfo, Result>, Map<HRegionInfo, Result>> 
scanTriple =
         getMergedRegionsAndSplitParents();
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 9416620..397d5a8 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
@@ -26,6 +26,7 @@ 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.SortedMap;
@@ -1068,6 +1069,37 @@ public class TestCatalogJanitor {
     janitor.cancel(true);
   }
 
+  @Test
+  public void testAlreadyRunningStatus() throws Exception {
+    int numberOfThreads = 2;
+    final List<Integer> gcValues = new ArrayList<>();
+    Thread[] threads = new Thread[numberOfThreads];
+    HBaseTestingUtility hBaseTestingUtility = new HBaseTestingUtility();
+    
hBaseTestingUtility.getConfiguration().setInt("hbase.client.retries.number", 5);
+    Server server = new MockServer(hBaseTestingUtility);
+    MasterServices services = new MockMasterServices(server);
+    final CatalogJanitor catalogJanitor = new CatalogJanitor(server, services);
+    for (int i = 0; i < numberOfThreads; i++) {
+      threads[i] = new Thread(new Runnable() {
+        @Override
+        public void run() {
+          try {
+            gcValues.add(catalogJanitor.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