You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by dl...@apache.org on 2024/02/06 17:02:45 UTC

(accumulo) branch elasticity updated: Host tablets needing recovery (#4229)

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

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


The following commit(s) were added to refs/heads/elasticity by this push:
     new 878e4edfee Host tablets needing recovery (#4229)
878e4edfee is described below

commit 878e4edfeeb68622c2b13348220c521f1f36c091
Author: Dave Marion <dl...@apache.org>
AuthorDate: Tue Feb 6 12:02:41 2024 -0500

    Host tablets needing recovery (#4229)
    
    Modified the TabletManagementIterator to return NEEDS_RECOVERY
    when the tablet has wals and is not being deleted. Modified
    TabletGoalState to return the goal of HOSTED when the tablet
    has wals and tablet availability is UNHOSTED or ONDEMAND. Re-ordered
    the TabletGroupWatcher such that volume replacement is always
    performed. In the case that bad state is not returned, but recovery is,
    then recovery will be performed.
    
    Closes #3663
---
 .../core/manager/state/TabletManagement.java       |  7 ++++-
 .../server/manager/state/TabletGoalState.java      | 10 +++++++
 .../manager/state/TabletManagementIterator.java    | 12 +++++++-
 .../accumulo/manager/TabletGroupWatcher.java       | 35 ++++++++++++++--------
 .../functional/TabletManagementIteratorIT.java     | 18 +++++++++--
 5 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/core/src/main/java/org/apache/accumulo/core/manager/state/TabletManagement.java b/core/src/main/java/org/apache/accumulo/core/manager/state/TabletManagement.java
index 94c41af7ff..04d956dd8c 100644
--- a/core/src/main/java/org/apache/accumulo/core/manager/state/TabletManagement.java
+++ b/core/src/main/java/org/apache/accumulo/core/manager/state/TabletManagement.java
@@ -53,7 +53,12 @@ public class TabletManagement {
   private static final Text EMPTY = new Text("");
 
   public static enum ManagementAction {
-    BAD_STATE, NEEDS_COMPACTING, NEEDS_LOCATION_UPDATE, NEEDS_SPLITTING, NEEDS_VOLUME_REPLACEMENT;
+    BAD_STATE,
+    NEEDS_COMPACTING,
+    NEEDS_LOCATION_UPDATE,
+    NEEDS_RECOVERY,
+    NEEDS_SPLITTING,
+    NEEDS_VOLUME_REPLACEMENT;
   }
 
   public static void addActions(final SortedMap<Key,Value> decodedRow,
diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletGoalState.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletGoalState.java
index 17779a4a7f..e4a30df8b4 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletGoalState.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletGoalState.java
@@ -18,6 +18,7 @@
  */
 package org.apache.accumulo.server.manager.state;
 
+import org.apache.accumulo.core.client.admin.TabletAvailability;
 import org.apache.accumulo.core.data.TabletId;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.dataImpl.TabletIdImpl;
@@ -84,6 +85,15 @@ public enum TabletGoalState {
         return TabletGoalState.UNASSIGNED;
       }
 
+      // When the tablet has wals and it will not be hosted normally, then cause it to
+      // be hosted so that recovery can occur. When tablet availability is ONDEMAND or
+      // UNHOSTED, then this tablet will eventually become unhosted after recovery occurs.
+      // This could cause a little bit of churn on the cluster w/r/t balancing, but it's
+      // necessary.
+      if (!tm.getLogs().isEmpty() && tm.getTabletAvailability() != TabletAvailability.HOSTED) {
+        return TabletGoalState.HOSTED;
+      }
+
       if (!params.isTableOnline(tm.getTableId())) {
         return UNASSIGNED;
       }
diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java
index ae0027c200..b339dd9e72 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java
@@ -55,6 +55,7 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Se
 import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn;
 import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily;
 import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.core.metadata.schema.TabletOperationType;
 import org.apache.accumulo.core.spi.balancer.SimpleLoadBalancer;
 import org.apache.accumulo.core.spi.balancer.TabletBalancer;
 import org.apache.accumulo.core.spi.compaction.CompactionKind;
@@ -248,13 +249,22 @@ public class TabletManagementIterator extends SkippingIterator {
     if (tm.isFutureAndCurrentLocationSet()) {
       // no need to check everything, we are in a known state where we want to return everything.
       reasonsToReturnThisTablet.add(ManagementAction.BAD_STATE);
-      return;
+    }
+
+    if (!tm.getLogs().isEmpty() && (tm.getOperationId() == null
+        || tm.getOperationId().getType() != TabletOperationType.DELETING)) {
+      reasonsToReturnThisTablet.add(ManagementAction.NEEDS_RECOVERY);
     }
 
     if (VolumeUtil.needsVolumeReplacement(tabletMgmtParams.getVolumeReplacements(), tm)) {
       reasonsToReturnThisTablet.add(ManagementAction.NEEDS_VOLUME_REPLACEMENT);
     }
 
+    if (!reasonsToReturnThisTablet.isEmpty()) {
+      // If volume replacement or recovery is needed, then return early.
+      return;
+    }
+
     if (shouldReturnDueToLocation(tm)) {
       reasonsToReturnThisTablet.add(ManagementAction.NEEDS_LOCATION_UPDATE);
     }
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
index b467d49551..b59c75d974 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
@@ -401,13 +401,6 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
         continue;
       }
 
-      final Set<ManagementAction> actions = mti.getActions();
-      if (actions.contains(ManagementAction.BAD_STATE) && tm.isFutureAndCurrentLocationSet()) {
-        throw new BadLocationStateException(
-            tm.getExtent() + " is both assigned and hosted, which should never happen: " + this,
-            tm.getExtent().toMetaRow());
-      }
-
       final TableId tableId = tm.getTableId();
       // ignore entries for tables that do not exist in zookeeper
       if (manager.getTableManager().getTableState(tableId) == null) {
@@ -460,6 +453,13 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
       final TabletGoalState goal =
           TabletGoalState.compute(tm, state, manager.tabletBalancer, tableMgmtParams);
 
+      final Set<ManagementAction> actions = mti.getActions();
+
+      if (actions.contains(ManagementAction.NEEDS_RECOVERY) && goal != TabletGoalState.HOSTED) {
+        LOG.warn("Tablet has wals, but goal is not hosted. Tablet: {}, goal:{}", tm.getExtent(),
+            goal);
+      }
+
       if (actions.contains(ManagementAction.NEEDS_VOLUME_REPLACEMENT)) {
         tableMgmtStats.totalVolumeReplacements++;
         if (state == TabletState.UNASSIGNED || state == TabletState.SUSPENDED) {
@@ -488,6 +488,12 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
         }
       }
 
+      if (actions.contains(ManagementAction.BAD_STATE) && tm.isFutureAndCurrentLocationSet()) {
+        throw new BadLocationStateException(
+            tm.getExtent() + " is both assigned and hosted, which should never happen: " + this,
+            tm.getExtent().toMetaRow());
+      }
+
       final Location location = tm.getLocation();
       Location current = null;
       Location future = null;
@@ -512,8 +518,7 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
             state, goal, actions);
       }
 
-      if (actions.contains(ManagementAction.NEEDS_SPLITTING)
-          && !actions.contains(ManagementAction.NEEDS_VOLUME_REPLACEMENT)) {
+      if (actions.contains(ManagementAction.NEEDS_SPLITTING)) {
         LOG.debug("{} may need splitting.", tm.getExtent());
         if (manager.getSplitter().isSplittable(tm)) {
           if (manager.getSplitter().addSplitStarting(tm.getExtent())) {
@@ -528,8 +533,7 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
         // sendSplitRequest(mergeStats.getMergeInfo(), state, tm);
       }
 
-      if (actions.contains(ManagementAction.NEEDS_COMPACTING)
-          && !actions.contains(ManagementAction.NEEDS_VOLUME_REPLACEMENT)) {
+      if (actions.contains(ManagementAction.NEEDS_COMPACTING)) {
         var jobs = compactionGenerator.generateJobs(tm,
             TabletManagementIterator.determineCompactionKinds(actions));
         LOG.debug("{} may need compacting adding {} jobs", tm.getExtent(), jobs.size());
@@ -542,14 +546,19 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
       // entries from the queue because we see nothing here for that case. After a full
       // metadata scan could remove any tablets that were not updated during the scan.
 
-      if (actions.contains(ManagementAction.NEEDS_LOCATION_UPDATE)) {
+      if (actions.contains(ManagementAction.NEEDS_LOCATION_UPDATE)
+          || actions.contains(ManagementAction.NEEDS_RECOVERY)) {
 
         if (tm.getLocation() != null) {
           filteredServersToShutdown.remove(tm.getLocation().getServerInstance());
         }
 
         if (goal == TabletGoalState.HOSTED) {
-          if ((state != TabletState.HOSTED && !tm.getLogs().isEmpty())
+
+          // RecoveryManager.recoverLogs will return false when all of the logs
+          // have been sorted so that recovery can occur. Delay the hosting of
+          // the Tablet until the sorting is finished.
+          if ((state != TabletState.HOSTED && actions.contains(ManagementAction.NEEDS_RECOVERY))
               && manager.recoveryManager.recoverLogs(tm.getExtent(), tm.getLogs())) {
             LOG.debug("Not hosting {} as it needs recovery, logs: {}", tm.getExtent(),
                 tm.getLogs().size());
diff --git a/test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java b/test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java
index 8d4a354a2d..ff6f4d34cb 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/TabletManagementIteratorIT.java
@@ -110,7 +110,7 @@ public class TabletManagementIteratorIT extends AccumuloClusterHarness {
 
     try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) {
 
-      String[] tables = getUniqueNames(8);
+      String[] tables = getUniqueNames(9);
       final String t1 = tables[0];
       final String t2 = tables[1];
       final String t3 = tables[2];
@@ -119,6 +119,7 @@ public class TabletManagementIteratorIT extends AccumuloClusterHarness {
       final String metaCopy2 = tables[5];
       final String metaCopy3 = tables[6];
       final String metaCopy4 = tables[7];
+      final String metaCopy5 = tables[8];
 
       // create some metadata
       createTable(client, t1, true);
@@ -152,6 +153,7 @@ public class TabletManagementIteratorIT extends AccumuloClusterHarness {
       copyTable(client, metaCopy1, metaCopy2);
       copyTable(client, metaCopy1, metaCopy3);
       copyTable(client, metaCopy1, metaCopy4);
+      copyTable(client, metaCopy1, metaCopy5);
 
       // t1 is unassigned, setting to always will generate a change to host tablets
       setTabletAvailability(client, metaCopy1, t1, TabletAvailability.HOSTED.name());
@@ -177,6 +179,18 @@ public class TabletManagementIteratorIT extends AccumuloClusterHarness {
       assertEquals(1, findTabletsNeedingAttention(client, metaCopy2, tabletMgmtParams),
           "Only 1 of 2 tablets in table t1 should be returned");
 
+      // Test the recovery cases
+      createLogEntry(client, metaCopy5, t1);
+      setTabletAvailability(client, metaCopy5, t1, TabletAvailability.UNHOSTED.name());
+      assertEquals(1, findTabletsNeedingAttention(client, metaCopy5, tabletMgmtParams),
+          "Only 1 of 2 tablets in table t1 should be returned");
+      setTabletAvailability(client, metaCopy5, t1, TabletAvailability.ONDEMAND.name());
+      assertEquals(1, findTabletsNeedingAttention(client, metaCopy5, tabletMgmtParams),
+          "Only 1 of 2 tablets in table t1 should be returned");
+      setTabletAvailability(client, metaCopy5, t1, TabletAvailability.HOSTED.name());
+      assertEquals(2, findTabletsNeedingAttention(client, metaCopy5, tabletMgmtParams),
+          "2 tablets in table t1 should be returned");
+
       // Remove location and set merge operation id on both tablets
       // These tablets should not need attention as they have no WALs
       setTabletAvailability(client, metaCopy4, t4, TabletAvailability.HOSTED.name());
@@ -225,7 +239,7 @@ public class TabletManagementIteratorIT extends AccumuloClusterHarness {
           "Should have one tablet that needs a volume replacement");
 
       // clean up
-      dropTables(client, t1, t2, t3, t4, metaCopy1, metaCopy2, metaCopy3, metaCopy4);
+      dropTables(client, t1, t2, t3, t4, metaCopy1, metaCopy2, metaCopy3, metaCopy4, metaCopy5);
     }
   }