Repository: hbase Updated Branches: refs/heads/branch-1 335b8a8e1 -> e65004aee
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/e65004ae Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e65004ae Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e65004ae Branch: refs/heads/branch-1 Commit: e65004aeea09e751624729a896409eef57f19a1e Parents: 335b8a8 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:17:13 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/e65004ae/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/e65004ae/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/e65004ae/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"); + } + } + } +}
