diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/ProcedureStoreTracker.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/ProcedureStoreTracker.java index 1930f8ac679..25c94272f08 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/ProcedureStoreTracker.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/ProcedureStoreTracker.java @@ -27,7 +27,8 @@ import java.util.function.BiFunction; import java.util.stream.LongStream; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.yetus.audience.InterfaceStability; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos; @@ -38,8 +39,10 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.ProcedureProtos; * deleted/completed to avoid the deserialization step on restart */ @InterfaceAudience.Private -@InterfaceStability.Evolving public class ProcedureStoreTracker { + + private static final Logger LOG = LoggerFactory.getLogger(ProcedureStoreTracker.class); + // Key is procedure id corresponding to first bit of the bitmap. private final TreeMap map = new TreeMap<>(); @@ -153,8 +156,10 @@ public class ProcedureStoreTracker { private BitSetNode delete(BitSetNode node, long procId) { node = lookupClosestNode(node, procId); - assert node != null : "expected node to delete procId=" + procId; - assert node.contains(procId) : "expected procId=" + procId + " in the node"; + if (node == null || !node.contains(procId)) { + LOG.warn("The BitSetNode for procId={} does not exist, maybe a double deletion?", procId); + return node; + } node.delete(procId); if (!keepDeletes && node.isEmpty()) { // TODO: RESET if (map.size() == 1) diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureRollbackAIOOB.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureRollbackAIOOB.java new file mode 100644 index 00000000000..fc12924244e --- /dev/null +++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureRollbackAIOOB.java @@ -0,0 +1,117 @@ +/** + * 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.procedure2; + +import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseCommonTestingUtility; +import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility.NoopProcedure; +import org.apache.hadoop.hbase.procedure2.store.wal.WALProcedureStore; +import org.apache.hadoop.hbase.testclassification.MasterTests; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; + +/** + * Testcase for HBASE-20973 + */ +@Category({ MasterTests.class, MediumTests.class }) +public class TestProcedureRollbackAIOOB { + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestProcedureRollbackAIOOB.class); + + private static final HBaseCommonTestingUtility UTIL = new HBaseCommonTestingUtility(); + + public static final class ParentProcedure extends NoopProcedure { + + private final CountDownLatch latch = new CountDownLatch(1); + + private boolean scheduled; + + @Override + protected Procedure[] execute(Void env) + throws ProcedureYieldException, ProcedureSuspendedException, InterruptedException { + latch.await(); + if (scheduled) { + return null; + } + scheduled = true; + return new Procedure[] { new SubProcedure() }; + } + } + + public static final class SubProcedure extends NoopProcedure { + + @Override + protected Procedure[] execute(Void env) + throws ProcedureYieldException, ProcedureSuspendedException, InterruptedException { + setFailure("Inject error", new RuntimeException("Inject error")); + return null; + } + } + + private WALProcedureStore procStore; + + private ProcedureExecutor procExec; + + @Rule + public final TestName name = new TestName(); + + @Before + public void setUp() throws IOException { + procStore = ProcedureTestingUtility.createWalStore(UTIL.getConfiguration(), + UTIL.getDataTestDir(name.getMethodName())); + procStore.start(2); + procExec = new ProcedureExecutor(UTIL.getConfiguration(), null, procStore); + ProcedureTestingUtility.initAndStartWorkers(procExec, 2, true); + } + + @After + public void tearDown() { + procExec.stop(); + procStore.stop(false); + } + + @AfterClass + public static void tearDownAfterClass() throws IOException { + UTIL.cleanupTestDir(); + } + + @Test + public void testArrayIndexOutOfBounds() { + ParentProcedure proc = new ParentProcedure(); + long procId = procExec.submitProcedure(proc); + long noopProcId = -1L; + // make sure that the sub procedure will have a new BitSetNode + for (int i = 0; i < Long.SIZE - 2; i++) { + noopProcId = procExec.submitProcedure(new NoopProcedure<>()); + } + final long lastNoopProcId = noopProcId; + UTIL.waitFor(30000, () -> procExec.isFinished(lastNoopProcId)); + proc.latch.countDown(); + UTIL.waitFor(10000, () -> procExec.isFinished(procId)); + } +}