HBASE-24940: runCatalogJanitor() API should return -1 to indicate already running status

Closes #2331

Signed-off-by: Viraj Jasani <vjasani@apache.org>
This commit is contained in:
Mohammad Arshad 2020-09-03 15:46:10 +05:30 committed by Viraj Jasani
parent 4dc08144f3
commit a352706700
No known key found for this signature in database
GPG Key ID: B3D6C0B41C8ADFD5
3 changed files with 29 additions and 2 deletions

View File

@ -903,7 +903,7 @@ public interface Admin extends Abortable, Closeable {
/** /**
* Ask for a scan of the catalog table. * 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 * @throws IOException if a remote or network exception occurs
*/ */
int runCatalogJanitor() throws IOException; int runCatalogJanitor() throws IOException;

View File

@ -162,6 +162,7 @@ public class CatalogJanitor extends ScheduledChore {
* Run janitorial scan of catalog <code>hbase:meta</code> table looking for * Run janitorial scan of catalog <code>hbase:meta</code> table looking for
* garbage to collect. * garbage to collect.
* @return How many items gc'd whether for merge or split. * @return How many items gc'd whether for merge or split.
* Returns -1 if previous scan is in progress.
*/ */
@VisibleForTesting @VisibleForTesting
public int scan() throws IOException { public int scan() throws IOException {
@ -169,7 +170,8 @@ public class CatalogJanitor extends ScheduledChore {
try { try {
if (!alreadyRunning.compareAndSet(false, true)) { if (!alreadyRunning.compareAndSet(false, true)) {
LOG.debug("CatalogJanitor already running"); LOG.debug("CatalogJanitor already running");
return gcs; // -1 indicates previous scan is in progress
return -1;
} }
this.lastReport = scanForReport(); this.lastReport = scanForReport();
if (!this.lastReport.isEmpty()) { if (!this.lastReport.isEmpty()) {

View File

@ -25,6 +25,8 @@ import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy; import static org.mockito.Mockito.spy;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.NavigableMap; import java.util.NavigableMap;
import java.util.Objects; import java.util.Objects;
@ -551,6 +553,29 @@ public class TestCatalogJanitor {
assertArchiveEqualToOriginal(storeFiles, archivedStoreFiles, fs, true); 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) private FileStatus[] addMockStoreFiles(int count, MasterServices services, Path storedir)
throws IOException { throws IOException {
// get the existing store files // get the existing store files