[accumulo] branch 2.1 updated: run minor compaction earlier in tablet close (#3685)

[kt...@apache.org]

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

kturner pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/2.1 by this push:
     new 92af41860e run minor compaction earlier in tablet close (#3685)
92af41860e is described below

commit 92af41860e29f448f9262b9d4d16406cafa31045
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Aug 11 14:13:05 2023 -0400

    run minor compaction earlier in tablet close (#3685)
    
    Closing a tablet does two minor compactions.  This commit moves the
    first minor compaction to run earlier.  The goal of this change is to
    avoid leaving tablets in a half closed state when minor compaction are
    failing.
---
 .../org/apache/accumulo/tserver/tablet/Tablet.java | 75 ++++++++++++++++------
 1 file changed, 55 insertions(+), 20 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
index cfc36719e2..d0d71bf722 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
@@ -902,7 +902,62 @@ public class Tablet extends TabletBase {
     log.trace("initiateClose(saveState={}) {}", saveState, getExtent());
 
     MinorCompactionTask mct = null;
+    if (saveState) {
+      try {
+        synchronized (this) {
+          // Wait for any running minor compaction before trying to start another. This is done for
+          // the case where the current in memory map has a lot of data. So wait for the running
+          // minor compaction and then start compacting the current in memory map before closing.
+          getTabletMemory().waitForMinC();
+        }
+        mct = createMinorCompactionTask(getFlushID(), MinorCompactionReason.CLOSE);
+      } catch (NoNodeException e) {
+        throw new IllegalStateException("Exception on " + extent + " during prep for MinC", e);
+      }
+    }
+
+    if (mct != null) {
+      // Do an initial minor compaction that flushes any data in memory before marking that tablet
+      // as closed. Another minor compaction will be done once the tablet is marked as closed. There
+      // are two goals for this initial minor compaction.
+      //
+      // 1. Make the 2nd minor compaction that occurs after closing faster because it has less
+      // data. That is important because after the tablet is closed it can not be read or written
+      // to, so hopefully the 2nd compaction has little if any data because of this minor compaction
+      // that occurred before close.
+      //
+      // 2. Its possible a minor compaction may hang because of bad config or DFS problems. Taking
+      // this action before close can be less disruptive if it does hang. Also in the case where
+      // there is a bug that causes minor compaction to fail it will leave the tablet in a bad
+      // state. If that happens here before starting to close then it could leave the tablet in a
+      // more usable state than a failure that happens after the tablet starts to close.
+      //
+      // If 'mct' was null it means either a minor compaction was running, there was no data to
+      // minor compact, or the flush id was updating. In the case of flush id was updating, ideally
+      // this code would wait for flush id updates and then minor compact if needed, but that can
+      // not be done without setting the close state to closing to prevent flush id updates from
+      // starting. So if there is a flush id update going on it could cause no minor compaction
+      // here. There will still be a minor compaction after close.
+      //
+      // Its important to run the following minor compaction outside of any sync blocks as this
+      // could needlessly block scans. The resources needed for the minor compaction have already
+      // been reserved in a sync block.
+      mct.run();
+    }
+
     synchronized (this) {
+
+      if (saveState) {
+        // Wait for any running minc to finish before we start shutting things down in the tablet.
+        // It is possible that this function was unable to initiate a minor compaction above and
+        // something else did because of race conditions (because everything above happens before
+        // marking anything closed so normal actions could still start minor compactions). If
+        // something did start lets wait on it before marking things closed.
+        getTabletMemory().waitForMinC();
+      }
+
+      // This check is intentionally done later in the method because the check and change of the
+      // closeState variable need to be atomic, so both are done in the same sync block.
       if (isClosed() || isClosing()) {
         String msg = "Tablet " + getExtent() + " already " + closeState;
         throw new IllegalStateException(msg);
@@ -933,27 +988,7 @@ public class Tablet extends TabletBase {
       // calling this.wait() releases the lock, ensure things are as expected when the lock is
       // obtained again
       Preconditions.checkState(closeState == CloseState.CLOSING);
-
-      if (!saveState || getTabletMemory().getMemTable().getNumEntries() == 0) {
-        return;
-      }
-
-      getTabletMemory().waitForMinC();
-
-      // calling this.wait() in waitForMinC() releases the lock, ensure things are as expected when
-      // the lock is obtained again
-      Preconditions.checkState(closeState == CloseState.CLOSING);
-
-      try {
-        mct = prepareForMinC(getFlushID(), MinorCompactionReason.CLOSE);
-      } catch (NoNodeException e) {
-        throw new RuntimeException("Exception on " + extent + " during prep for MinC", e);
-      }
     }
-
-    // do minor compaction outside of synch block so that tablet can be read and written to while
-    // compaction runs
-    mct.run();
   }
 
   private boolean closeCompleting = false;