HBASE-15113 Procedure v2 - Speedup eviction of sys operation results

hbase-common/src/main/java/org/apache/hadoop/hbase/ProcedureInfo.java

@@ -203,7 +203,7 @@ public class ProcedureInfo implements Cloneable {
 
   @InterfaceAudience.Private
   public boolean hasClientAckTime() {
-    return clientAckTime > 0;
+    return clientAckTime != -1;
   }
 
   @InterfaceAudience.Private

hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java

@@ -190,6 +190,19 @@ public abstract class Procedure<TEnvironment> implements Comparable<Procedure> {
     return false;
   }
 
+  /**
+   * By default, the executor will keep the procedure result around util
+   * the eviction TTL is expired. The client can cut down the waiting time
+   * by requesting that the result is removed from the executor.
+   * In case of system started procedure, we can force the executor to auto-ack.
+   * @param env the environment passed to the ProcedureExecutor
+   * @return true if the executor should wait the client ack for the result.
+   *         Defaults to return true.
+   */
+  protected boolean shouldWaitClientAck(final TEnvironment env) {
+    return true;
+  }
+
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();

hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java

@@ -165,22 +165,23 @@ public class ProcedureExecutor<TEnvironment> {
       final long evictTtl = conf.getInt(EVICT_TTL_CONF_KEY, DEFAULT_EVICT_TTL);
       final long evictAckTtl = conf.getInt(EVICT_ACKED_TTL_CONF_KEY, DEFAULT_ACKED_EVICT_TTL);
 
-      long now = EnvironmentEdgeManager.currentTime();
-      Iterator<Map.Entry<Long, ProcedureInfo>> it = completed.entrySet().iterator();
+      final long now = EnvironmentEdgeManager.currentTime();
+      final Iterator<Map.Entry<Long, ProcedureInfo>> it = completed.entrySet().iterator();
+      final boolean isDebugEnabled = LOG.isDebugEnabled();
       while (it.hasNext() && store.isRunning()) {
-        Map.Entry<Long, ProcedureInfo> entry = it.next();
-        ProcedureInfo result = entry.getValue();
+        final Map.Entry<Long, ProcedureInfo> entry = it.next();
+        final ProcedureInfo procInfo = entry.getValue();
 
         // TODO: Select TTL based on Procedure type
-        if ((result.hasClientAckTime() && (now - result.getClientAckTime()) >= evictAckTtl) ||
-            (now - result.getLastUpdate()) >= evictTtl) {
-          if (LOG.isDebugEnabled()) {
-            LOG.debug("Evict completed procedure " + entry.getKey());
+        if ((procInfo.hasClientAckTime() && (now - procInfo.getClientAckTime()) >= evictAckTtl) ||
+            (now - procInfo.getLastUpdate()) >= evictTtl) {
+          if (isDebugEnabled) {
+            LOG.debug("Evict completed procedure: " + procInfo);
           }
           store.delete(entry.getKey());
           it.remove();
 
-          NonceKey nonceKey = result.getNonceKey();
+          NonceKey nonceKey = procInfo.getNonceKey();
           if (nonceKey != null) {
             nonceKeysToProcIdsMap.remove(nonceKey);
           }
@@ -1272,7 +1273,12 @@ public class ProcedureExecutor<TEnvironment> {
     }
 
     // update the executor internal state maps
-    completed.put(proc.getProcId(), Procedure.createProcedureInfo(proc, proc.getNonceKey()));
+    ProcedureInfo procInfo = Procedure.createProcedureInfo(proc, proc.getNonceKey());
+    if (!proc.shouldWaitClientAck(getEnvironment())) {
+      procInfo.setClientAckTime(0);
+    }
+
+    completed.put(procInfo.getProcId(), procInfo);
     rollbackStack.remove(proc.getProcId());
     procedures.remove(proc.getProcId());
 

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateNamespaceProcedure.java

@@ -194,16 +194,18 @@ public class CreateNamespaceProcedure
     sb.append(")");
   }
 
+  private boolean isBootstrapNamespace() {
+    return nsDescriptor.equals(NamespaceDescriptor.DEFAULT_NAMESPACE) ||
+        nsDescriptor.equals(NamespaceDescriptor.SYSTEM_NAMESPACE);
+  }
+
   @Override
   protected boolean acquireLock(final MasterProcedureEnv env) {
     if (!env.getMasterServices().isInitialized()) {
       // Namespace manager might not be ready if master is not fully initialized,
       // return false to reject user namespace creation; return true for default
       // and system namespace creation (this is part of master initialization).
-      boolean isBootstrapNs = nsDescriptor.equals(NamespaceDescriptor.DEFAULT_NAMESPACE) ||
-        nsDescriptor.equals(NamespaceDescriptor.SYSTEM_NAMESPACE);
-
-      if (!isBootstrapNs && env.waitInitialized(this)) {
+      if (!isBootstrapNamespace() && env.waitInitialized(this)) {
         return false;
       }
     }
@@ -364,4 +366,11 @@ public class CreateNamespaceProcedure
     }
     return traceEnabled;
   }
+
+  @Override
+  protected boolean shouldWaitClientAck(MasterProcedureEnv env) {
+    // hbase and default namespaces are created on bootstrap internally by the system
+    // the client does not know about this procedures.
+    return !isBootstrapNamespace();
+  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java

@@ -448,4 +448,11 @@ public class CreateTableProcedure
       final TableName tableName) throws IOException {
     env.getMasterServices().getTableDescriptors().get(tableName);
   }
+
+  @Override
+  protected boolean shouldWaitClientAck(MasterProcedureEnv env) {
+    // system tables are created on bootstrap internally by the system
+    // the client does not know about this procedures.
+    return !getTableName().isSystemTable();
+  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java

@@ -767,4 +767,11 @@ implements ServerProcedureInterface {
   protected boolean isYieldBeforeExecuteFromState(MasterProcedureEnv env, ServerCrashState state) {
     return true;
   }
+
+  @Override
+  protected boolean shouldWaitClientAck(MasterProcedureEnv env) {
+    // The operation is triggered internally on the server
+    // the client does not know about this procedure.
+    return false;
+  }
 }