dlmarion commented on code in PR #3677: URL: https://github.com/apache/accumulo/pull/3677#discussion_r1290544626
########## test/src/main/java/org/apache/accumulo/test/functional/HalfClosedTabletIT.java: ########## @@ -0,0 +1,219 @@ +/* + * 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 + * + * https://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.accumulo.test.functional; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; + +import java.util.EnumSet; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.BatchWriter; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.clientImpl.AccumuloServerException; +import org.apache.accumulo.core.clientImpl.ClientContext; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.data.Mutation; +import org.apache.accumulo.core.data.TableId; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; +import org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType; +import org.apache.accumulo.core.metadata.schema.TabletsMetadata; +import org.apache.accumulo.core.util.UtilWaitThread; +import org.apache.accumulo.harness.MiniClusterConfigurationCallback; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl; +import org.apache.accumulo.server.ServerContext; +import org.apache.accumulo.server.conf.store.TablePropKey; +import org.apache.accumulo.test.util.Wait; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Text; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import com.google.common.collect.Sets; + +// This IT tests the cases seen in https://github.com/apache/accumulo/issues/3674 +// where a failing minor compaction causes a Tablet.initiateClose to leave the +// Tablet in a half-closed state. The half-closed Tablet cannot be unloaded and +// the TabletServer cannot be shutdown normally. Because the minor compaction has +// been failing the Tablet needs to be recovered when it's ultimately re-assigned. +public class HalfClosedTabletIT extends SharedMiniClusterBase { + + public static class HalfClosedTabletITConfiguration implements MiniClusterConfigurationCallback { + + @Override + public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration coreSite) { + cfg.setNumTservers(1); + } + + } + + @BeforeAll + public static void startup() throws Exception { + SharedMiniClusterBase.startMiniClusterWithConfig(new HalfClosedTabletITConfiguration()); + } + + @AfterAll + public static void shutdown() throws Exception { + SharedMiniClusterBase.stopMiniCluster(); + } + + @Test + public void testSplitWithInvalidContext() throws Exception { + + // In this scenario the table has been mis-configured with an invalid context name. + // The minor compaction task is failing because classes like the volume chooser or + // user iterators cannot be loaded. The user calls Tablet.split which calls initiateClose. + // This test ensures that the Tablet can still be unloaded normally by taking if offline + // after the split call with an invalid context. The context property is removed after the + // split call below to get the minor compaction task to succeed on a subsequent run. Because + // the minor compaction task backs off when retrying, this could take some time. + + String tableName = getUniqueNames(1)[0]; + + try (final var client = Accumulo.newClient().from(getClientProps()).build()) { + final var tops = client.tableOperations(); + tops.create(tableName); + TableId tableId = TableId.of(tops.tableIdMap().get(tableName)); + try (final var bw = client.createBatchWriter(tableName)) { + final var m1 = new Mutation("a"); + final var m2 = new Mutation("b"); + m1.put(new Text("cf"), new Text(), new Value()); + m2.put(new Text("cf"), new Text(), new Value()); + bw.addMutation(m1); + bw.addMutation(m2); + } + + setInvalidClassLoaderContextPropertyWithoutValidation(getCluster().getServerContext(), + tableId); + + Thread.sleep(500); + + // This should fail to split, but not leave the tablets in a state where they can't + // be unloaded + assertThrows(AccumuloServerException.class, + () -> tops.addSplits(tableName, Sets.newTreeSet(List.of(new Text("b"))))); + + removeInvalidClassLoaderContextPropertyWithoutValidation(getCluster().getServerContext(), + tableId); Review Comment: I made these changes in 9f84ed4, however 2/3 of these new tests no longer work. I believe that is because I merged 3678 into 2.1 and into this branch. The tests that are failing are setting an invalid context, and when ClassLoaderUtil.getClassLoader is called, the context is not valid, and the non-context aware classloader is returned instead of the code returning null or throwing an error. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
