Repository: hbase
Updated Branches:
  refs/heads/branch-1.4 21152acbe -> 7446b8eaf


HBASE-19996 Some nonce procs might not be cleaned up (follow up HBASE-19756)

Signed-off-by: tedyu <yuzhih...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/7446b8ea
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/7446b8ea
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/7446b8ea

Branch: refs/heads/branch-1.4
Commit: 7446b8eaf21a2ef57a9352e2000c5b192b0512f9
Parents: 21152ac
Author: Thiruvel Thirumoolan <thiru...@oath.com>
Authored: Tue Feb 13 17:38:16 2018 -0800
Committer: tedyu <yuzhih...@gmail.com>
Committed: Wed Feb 14 09:16:44 2018 -0800

----------------------------------------------------------------------
 .../hbase/procedure2/ProcedureExecutor.java     |  30 +++--
 .../client/TestNonceProcCleanerOnFailure.java   | 101 --------------
 .../hbase/procedure/TestFailedProcCleanup.java  | 135 +++++++++++++++++++
 3 files changed, 157 insertions(+), 109 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/7446b8ea/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
----------------------------------------------------------------------
diff --git 
a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
 
b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
index f04a409..6e517aa 100644
--- 
a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
+++ 
b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
@@ -50,6 +50,7 @@ import 
org.apache.hadoop.hbase.procedure2.store.ProcedureStore.ProcedureIterator
 import org.apache.hadoop.hbase.procedure2.util.StringUtils;
 import org.apache.hadoop.hbase.procedure2.util.TimeoutBlockingQueue;
 import 
org.apache.hadoop.hbase.procedure2.util.TimeoutBlockingQueue.TimeoutRetriever;
+import org.apache.hadoop.hbase.protobuf.generated.ErrorHandlingProtos;
 import 
org.apache.hadoop.hbase.protobuf.generated.ProcedureProtos.ProcedureState;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
@@ -178,17 +179,19 @@ public class ProcedureExecutor<TEnvironment> {
         // TODO: Select TTL based on Procedure type
         if ((procInfo.hasClientAckTime() && (now - 
procInfo.getClientAckTime()) >= evictAckTtl) ||
             (now - procInfo.getLastUpdate()) >= evictTtl) {
-          if (isDebugEnabled) {
-            LOG.debug("Evict completed procedure: " + procInfo);
+          // Failed Procedures aren't persisted in WAL.
+          if (!(procInfo instanceof FailedProcedureInfo)) {
+            store.delete(entry.getKey());
           }
+          it.remove();
+
           NonceKey nonceKey = procInfo.getNonceKey();
-          // Nonce procedures aren't persisted in WAL.
-          if (nonceKey == null) {
-            store.delete(entry.getKey());
-          } else {
+          if (nonceKey != null) {
             nonceKeysToProcIdsMap.remove(nonceKey);
           }
-          it.remove();
+          if (isDebugEnabled) {
+            LOG.debug("Evict completed procedure: " + procInfo);
+          }
         }
       }
     }
@@ -696,7 +699,7 @@ public class ProcedureExecutor<TEnvironment> {
     if (procId == null || completed.containsKey(procId)) return;
 
     final long currentTime = EnvironmentEdgeManager.currentTime();
-    final ProcedureInfo result = new ProcedureInfo(
+    final ProcedureInfo result = new FailedProcedureInfo(
       procId.longValue(),
       procName,
       procOwner != null ? procOwner.getShortName() : null,
@@ -710,6 +713,17 @@ public class ProcedureExecutor<TEnvironment> {
     completed.putIfAbsent(procId, result);
   }
 
+  public static class FailedProcedureInfo extends ProcedureInfo {
+
+    public FailedProcedureInfo(long procId, String procName, String procOwner,
+        ProcedureState procState, long parentId, NonceKey nonceKey,
+        ErrorHandlingProtos.ForeignExceptionMessage exception, long 
lastUpdate, long startTime,
+        byte[] result) {
+      super(procId, procName, procOwner, procState, parentId, nonceKey, 
exception, lastUpdate,
+          startTime, result);
+    }
+  }
+
   // ==========================================================================
   //  Submit/Abort Procedure
   // ==========================================================================

http://git-wip-us.apache.org/repos/asf/hbase/blob/7446b8ea/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestNonceProcCleanerOnFailure.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestNonceProcCleanerOnFailure.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestNonceProcCleanerOnFailure.java
deleted file mode 100644
index bbf96b0..0000000
--- 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestNonceProcCleanerOnFailure.java
+++ /dev/null
@@ -1,101 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with this
- * work for additional information regarding copyright ownership. The ASF
- * licenses this file to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.hadoop.hbase.client;
-
-import static org.junit.Assert.fail;
-
-import java.io.IOException;
-import java.util.List;
-
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.HBaseTestingUtility;
-import org.apache.hadoop.hbase.HRegionInfo;
-import org.apache.hadoop.hbase.HTableDescriptor;
-import org.apache.hadoop.hbase.ProcedureInfo;
-import org.apache.hadoop.hbase.TableName;
-import org.apache.hadoop.hbase.coprocessor.BaseMasterObserver;
-import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
-import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
-import org.apache.hadoop.hbase.coprocessor.ObserverContext;
-import org.apache.hadoop.hbase.protobuf.generated.ProcedureProtos;
-import org.apache.hadoop.hbase.security.AccessDeniedException;
-import org.apache.hadoop.hbase.testclassification.MediumTests;
-import org.apache.hadoop.hbase.util.Bytes;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-
-/**
- * Check if CompletedProcedureCleaner cleans up failed nonce procedures.
- */
-@Category(MediumTests.class)
-public class TestNonceProcCleanerOnFailure {
-  private static final Log LOG = 
LogFactory.getLog(TestNonceProcCleanerOnFailure.class);
-
-  protected static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
-  private static final TableName TABLE = TableName.valueOf("test");
-  private static final byte[] FAMILY = Bytes.toBytesBinary("f");
-  private static final int evictionDelay = 10 * 1000;
-
-  @BeforeClass
-  public static void setUpBeforeClass() throws Exception {
-    Configuration conf = TEST_UTIL.getConfiguration();
-    conf.setInt("hbase.procedure.cleaner.evict.ttl", evictionDelay);
-    conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, 
CreateFailObserver.class.getName());
-    TEST_UTIL.startMiniCluster(3);
-  }
-
-  @AfterClass
-  public static void tearDownAfterClass() throws Exception {
-    TEST_UTIL.cleanupTestDir();
-    TEST_UTIL.cleanupDataTestDirOnTestFS();
-    TEST_UTIL.shutdownMiniCluster();
-  }
-
-  @Test
-  public void testFailCreateTable() throws Exception {
-    try {
-      TEST_UTIL.createTable(TABLE, FAMILY);
-    } catch (AccessDeniedException e) {
-      LOG.debug("Ignoring exception: ", e);
-      Thread.sleep(evictionDelay * 3);
-    }
-    List<ProcedureInfo> procedureInfos =
-        
TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor().listProcedures();
-    for (ProcedureInfo procedureInfo : procedureInfos) {
-      if (procedureInfo.getProcName().equals("CreateTableProcedure")
-          && procedureInfo.getProcState() == 
ProcedureProtos.ProcedureState.ROLLEDBACK) {
-        fail("Found procedure " + procedureInfo + " that hasn't been cleaned 
up");
-      }
-    }
-  }
-
-  public static class CreateFailObserver extends BaseMasterObserver {
-
-    @Override
-    public void preCreateTable(ObserverContext<MasterCoprocessorEnvironment> 
ctx,
-        HTableDescriptor desc, HRegionInfo[] regions) throws IOException {
-
-      if (desc.getTableName().equals(TABLE)) {
-        throw new AccessDeniedException("Don't allow creation of table");
-      }
-    }
-  }
-}

http://git-wip-us.apache.org/repos/asf/hbase/blob/7446b8ea/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestFailedProcCleanup.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestFailedProcCleanup.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestFailedProcCleanup.java
new file mode 100644
index 0000000..e10b74e
--- /dev/null
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestFailedProcCleanup.java
@@ -0,0 +1,135 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.hadoop.hbase.procedure;
+
+import static 
org.apache.hadoop.hbase.coprocessor.CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.ProcedureInfo;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.coprocessor.BaseMasterObserver;
+import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.protobuf.generated.ProcedureProtos;
+import org.apache.hadoop.hbase.security.AccessDeniedException;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Check if CompletedProcedureCleaner cleans up failed nonce procedures.
+ */
+@Category(MediumTests.class)
+public class TestFailedProcCleanup {
+  private static final Log LOG = 
LogFactory.getLog(TestFailedProcCleanup.class);
+
+  protected static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+  private static Configuration conf;
+  private static final TableName TABLE = TableName.valueOf("test");
+  private static final byte[] FAMILY = Bytes.toBytesBinary("f");
+  private static final int evictionDelay = 10 * 1000;
+
+  @BeforeClass
+  public static void setUpBeforeClass() {
+    conf = TEST_UTIL.getConfiguration();
+    conf.setInt("hbase.procedure.cleaner.evict.ttl", evictionDelay);
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    TEST_UTIL.cleanupTestDir();
+    TEST_UTIL.cleanupDataTestDirOnTestFS();
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testFailCreateTable() throws Exception {
+    conf.set(MASTER_COPROCESSOR_CONF_KEY, CreateFailObserver.class.getName());
+    TEST_UTIL.startMiniCluster(3);
+    try {
+      TEST_UTIL.createTable(TABLE, FAMILY);
+    } catch (AccessDeniedException e) {
+      LOG.debug("Ignoring exception: ", e);
+      Thread.sleep(evictionDelay * 3);
+    }
+    List<ProcedureInfo> procedureInfos =
+        
TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor().listProcedures();
+    for (ProcedureInfo procedureInfo : procedureInfos) {
+      if (procedureInfo.getProcName().equals("CreateTableProcedure")
+          && procedureInfo.getProcState() == 
ProcedureProtos.ProcedureState.ROLLEDBACK) {
+        fail("Found procedure " + procedureInfo + " that hasn't been cleaned 
up");
+      }
+    }
+  }
+
+  @Test
+  public void testFailCreateTableHandler() throws Exception {
+    conf.set(MASTER_COPROCESSOR_CONF_KEY, 
CreateFailObserverHandler.class.getName());
+    TEST_UTIL.startMiniCluster(3);
+    try {
+      TEST_UTIL.createTable(TABLE, FAMILY);
+    } catch (AccessDeniedException e) {
+      LOG.debug("Ignoring exception: ", e);
+      Thread.sleep(evictionDelay * 3);
+    }
+    List<ProcedureInfo> procedureInfos =
+        
TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor().listProcedures();
+    for (ProcedureInfo procedureInfo : procedureInfos) {
+      if (procedureInfo.getProcName().equals("CreateTableProcedure")
+          && procedureInfo.getProcState() == 
ProcedureProtos.ProcedureState.ROLLEDBACK) {
+        fail("Found procedure " + procedureInfo + " that hasn't been cleaned 
up");
+      }
+    }
+  }
+
+  public static class CreateFailObserver extends BaseMasterObserver {
+
+    @Override
+    public void preCreateTable(ObserverContext<MasterCoprocessorEnvironment> 
ctx,
+        HTableDescriptor desc, HRegionInfo[] regions) throws IOException {
+
+      if (desc.getTableName().equals(TABLE)) {
+        throw new AccessDeniedException("Don't allow creation of table");
+      }
+    }
+  }
+
+  public static class CreateFailObserverHandler extends BaseMasterObserver {
+
+    @Override
+    public void preCreateTableHandler(
+        final ObserverContext<MasterCoprocessorEnvironment> ctx,
+        HTableDescriptor desc, HRegionInfo[] regions) throws IOException {
+
+      if (desc.getTableName().equals(TABLE)) {
+        throw new AccessDeniedException("Don't allow creation of table");
+      }
+    }
+  }
+}

Reply via email to