HBASE-26434 Do compact when all L0 files are expired (#3830)

Signed-off-by: Duo Zhang <zhangduo@apache.org>
apache · Feb 15, 2022 · c8ccbde · c8ccbde
1 parent 3f315d7
commit c8ccbde
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 14 deletions.
diff --git a/...rc/main/java/org/apache/hadoop/hbase/regionserver/compactions/StripeCompactionPolicy.java b/...rc/main/java/org/apache/hadoop/hbase/regionserver/compactions/StripeCompactionPolicy.java
@@ -129,7 +129,8 @@ public StripeCompactionRequest selectCompaction(StripeInformationProvider si,
     List<HStoreFile> l0Files = si.getLevel0Files();
 
     // See if we need to make new stripes.
-    boolean shouldCompactL0 = this.config.getLevel0MinFiles() <= l0Files.size();
+    boolean shouldCompactL0 =
+        this.config.getLevel0MinFiles() <= l0Files.size() || allL0FilesExpired(si);
     if (stripeCount == 0) {
       if (!shouldCompactL0) {
         return null; // nothing to do.
@@ -167,7 +168,7 @@ public boolean needsCompactions(StripeInformationProvider si, List<HStoreFile> f
     return filesCompacting.isEmpty()
         && (StoreUtils.hasReferences(si.getStorefiles())
           || (si.getLevel0Files().size() >= this.config.getLevel0MinFiles())
-          || needsSingleStripeCompaction(si) || hasExpiredStripes(si));
+          || needsSingleStripeCompaction(si) || hasExpiredStripes(si) || allL0FilesExpired(si));
   }
 
   @Override
@@ -368,7 +369,25 @@ private StripeCompactionRequest selectExpiredMergeCompaction(
     return result;
   }
 
-  private boolean isStripeExpired(ImmutableList<HStoreFile> storeFiles) {
+  protected boolean hasExpiredStripes(StripeInformationProvider si) {
+    // Find if exists a stripe where all files have expired, if any.
+    ArrayList<ImmutableList<HStoreFile>> stripes = si.getStripes();
+    for (ImmutableList<HStoreFile> stripe : stripes) {
+      if (allFilesExpired(stripe)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  protected boolean allL0FilesExpired(StripeInformationProvider si) {
+    return allFilesExpired(si.getLevel0Files());
+  }
+
+  private boolean allFilesExpired(final List<HStoreFile> storeFiles) {
+    if (storeFiles == null || storeFiles.isEmpty()) {
+      return false;
+    }
     long cfTtl = this.storeConfigInfo.getStoreFileTtl();
     if (cfTtl == Long.MAX_VALUE) {
       return false; // minversion might be set, cannot delete old files
@@ -384,17 +403,6 @@ private boolean isStripeExpired(ImmutableList<HStoreFile> storeFiles) {
     return true;
   }
 
-  protected boolean hasExpiredStripes(StripeInformationProvider si) {
-    // Find if exists a stripe where all files have expired, if any.
-    ArrayList<ImmutableList<HStoreFile>> stripes = si.getStripes();
-    for (ImmutableList<HStoreFile> stripe : stripes) {
-      if (isStripeExpired(stripe)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   private static long getTotalKvCount(final Collection<HStoreFile> candidates) {
     long totalSize = 0;
     for (HStoreFile storeFile : candidates) {

diff --git a/...est/java/org/apache/hadoop/hbase/regionserver/compactions/TestStripeCompactionPolicy.java b/...est/java/org/apache/hadoop/hbase/regionserver/compactions/TestStripeCompactionPolicy.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.regionserver.compactions;
 
 import static org.apache.hadoop.hbase.regionserver.StripeStoreConfig.MAX_FILES_KEY;
+import static org.apache.hadoop.hbase.regionserver.StripeStoreConfig.MIN_FILES_KEY;
 import static org.apache.hadoop.hbase.regionserver.StripeStoreFileManager.OPEN_KEY;
 import static org.apache.hadoop.hbase.regionserver.compactions.CompactionConfiguration.HBASE_HSTORE_COMPACTION_MAX_SIZE_KEY;
 import static org.junit.Assert.assertEquals;
@@ -39,13 +40,15 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.OptionalLong;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.Cell;
@@ -88,6 +91,7 @@
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 import org.mockito.ArgumentMatcher;
+
 import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableList;
 import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
 
@@ -536,6 +540,44 @@ public void testSingleStripeDropDeletes() throws Exception {
       true);
   }
 
+  @Test
+  public void testCheckExpiredL0Compaction() throws Exception {
+    Configuration conf = HBaseConfiguration.create();
+    int minL0 = 100;
+    conf.setInt(StripeStoreConfig.MIN_FILES_L0_KEY, minL0);
+    conf.setInt(MIN_FILES_KEY, 4);
+
+    ManualEnvironmentEdge edge = new ManualEnvironmentEdge();
+    long now = defaultTtl + 2;
+    edge.setValue(now);
+    EnvironmentEdgeManager.injectEdge(edge);
+    HStoreFile expiredFile = createFile(10), notExpiredFile = createFile(10);
+    when(expiredFile.getReader().getMaxTimestamp()).thenReturn(now - defaultTtl - 1);
+    when(notExpiredFile.getReader().getMaxTimestamp()).thenReturn(now - defaultTtl + 1);
+    List<HStoreFile> expired = Lists.newArrayList(expiredFile, expiredFile);
+    List<HStoreFile> mixed = Lists.newArrayList(expiredFile, notExpiredFile);
+
+    StripeCompactionPolicy policy =
+        createPolicy(conf, defaultSplitSize, defaultSplitCount, defaultInitialCount, true);
+    // Merge expired if there are eligible stripes.
+    StripeCompactionPolicy.StripeInformationProvider si =
+        createStripesWithFiles(null, new ArrayList<>(), mixed);
+    assertFalse(policy.needsCompactions(si, al()));
+
+    List<HStoreFile> largeMixed = new ArrayList<>();
+    for (int i = 0; i < minL0 - 1; i++) {
+      largeMixed.add(i % 2 == 0 ? notExpiredFile : expiredFile);
+    }
+    si = createStripesWithFiles(null, new ArrayList<>(), largeMixed);
+    assertFalse(policy.needsCompactions(si, al()));
+
+    si = createStripesWithFiles(null, new ArrayList<>(), expired);
+    assertFalse(policy.needsSingleStripeCompaction(si));
+    assertFalse(policy.hasExpiredStripes(si));
+    assertTrue(policy.allL0FilesExpired(si));
+    assertTrue(policy.needsCompactions(si, al()));
+  }
+
   /********* HELPER METHODS ************/
   private static StripeCompactionPolicy createPolicy(
       Configuration conf) throws Exception {