kevinrr888 commented on code in PR #6040: URL: https://github.com/apache/accumulo/pull/6040#discussion_r2728673963
########## test/src/main/java/org/apache/accumulo/test/functional/IteratorConflictsIT.java: ########## @@ -0,0 +1,816 @@ +/* + * 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 java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.BufferedReader; +import java.io.InputStreamReader; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Consumer; + +import org.apache.accumulo.core.client.Accumulo; +import org.apache.accumulo.core.client.AccumuloClient; +import org.apache.accumulo.core.client.AccumuloException; +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.Scanner; +import org.apache.accumulo.core.client.admin.CloneConfiguration; +import org.apache.accumulo.core.client.admin.NamespaceOperations; +import org.apache.accumulo.core.client.admin.NewTableConfiguration; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.iterators.IteratorUtil; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.hadoop.fs.Path; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.layout.PatternLayout; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +/** + * Tests that iterator conflicts are detected and cause exceptions. Iterators can be added multiple + * ways. This test ensures that: + * <p> + * - {@link Scanner#addScanIterator(IteratorSetting)} + * <p> + * - {@link TableOperations#setProperty(String, String, String)} + * <p> + * - {@link TableOperations#modifyProperties(String, Consumer)} + * <p> + * - {@link NewTableConfiguration#attachIterator(IteratorSetting, EnumSet)} + * <p> + * - {@link TableOperations#attachIterator(String, IteratorSetting, EnumSet)} + * <p> + * - {@link NamespaceOperations#attachIterator(String, IteratorSetting, EnumSet)} + * <p> + * - {@link NamespaceOperations#setProperty(String, String, String)} + * <p> + * - {@link NamespaceOperations#modifyProperties(String, Consumer)} + * <p> + * - {@link CloneConfiguration.Builder#setPropertiesToSet(Map)} + * <p> + * All fail when conflicts arise from: + * <p> + * - Iterators attached directly to a table + * <p> + * - Iterators attached to a namespace, inherited by a table + * <p> + * - Default table iterators, but should not fail if {@link NewTableConfiguration#withoutDefaults()} + * is specified + * <p> + * - Adding the exact iterator already present should not fail + */ +public class IteratorConflictsIT extends SharedMiniClusterBase { + private static TableOperations tops; + private static NamespaceOperations nops; + private static AccumuloClient client; + // doesn't matter what iterator is used here + private static final String iterClass = SlowIterator.class.getName(); + private static final IteratorSetting iter1 = new IteratorSetting(99, "iter1name", iterClass); + private static final String iter1Key = Property.TABLE_ITERATOR_PREFIX + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + iter1.getName(); + private static final String iter1Val = "99," + iterClass; + private static final IteratorSetting iter1PrioConflict = + new IteratorSetting(99, "othername", iterClass); + private static final IteratorSetting iter1NameConflict = + new IteratorSetting(101, iter1.getName(), iterClass); + private static final String iter1PrioConflictKey = Property.TABLE_ITERATOR_PREFIX + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + ".othername"; + private static final String iter1PrioConflictVal = "99," + iterClass; + private static final String iter1NameConflictKey = Property.TABLE_ITERATOR_PREFIX + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + iter1.getName(); + private static final String iter1NameConflictVal = "101," + iterClass; + private static final IteratorSetting defaultIterPrioConflict = + new IteratorSetting(20, "bar", iterClass); + private static final IteratorSetting defaultIterNameConflict = + new IteratorSetting(101, "vers", iterClass); + private static final IteratorSetting defaultTableIter = + IteratorConfigUtil.getInitialTableIteratorSettings().keySet().iterator().next(); + private static final String defaultIterPrioConflictKey = Property.TABLE_ITERATOR_PREFIX + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + ".foo"; + private static final String defaultIterPrioConflictVal = + defaultTableIter.getPriority() + "," + iterClass; + private static final String defaultIterNameConflictKey = Property.TABLE_ITERATOR_PREFIX + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + defaultTableIter.getName(); + private static final String defaultIterNameConflictVal = "99," + iterClass; + private static final String defaultIterKey = Property.TABLE_ITERATOR_PREFIX.getKey() + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + defaultTableIter.getName(); + private static final String defaultIterVal = + defaultTableIter.getPriority() + "," + defaultTableIter.getIteratorClass(); + private static final String defaultIterOptKey = Property.TABLE_ITERATOR_PREFIX.getKey() + + IteratorUtil.IteratorScope.scan.name().toLowerCase() + "." + defaultTableIter.getName() + + ".opt." + defaultTableIter.getOptions().entrySet().iterator().next().getKey(); + private static final String defaultIterOptVal = + defaultTableIter.getOptions().entrySet().iterator().next().getValue(); + + private static final LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false); + private static final Configuration loggerConfig = loggerContext.getConfiguration(); + private static final TestAppender appender = new TestAppender(); + private static final String datePattern = getDatePattern(); + private static final DateTimeFormatter dateTimeFormatter = + DateTimeFormatter.ofPattern(datePattern); + + public static class TestAppender extends AbstractAppender { + // CopyOnWriteArrayList for thread safety, even while iterating + private final List<LogEvent> events = new CopyOnWriteArrayList<>(); + + public TestAppender() { + super("TestAppender", null, PatternLayout.createDefaultLayout(), false, null); + } + + @Override + public void append(LogEvent event) { + events.add(event.toImmutable()); + } + + public List<LogEvent> events() { + return events; + } + } + + @BeforeAll + public static void startup() throws Exception { + SharedMiniClusterBase.startMiniCluster(); + client = Accumulo.newClient().from(getClientProps()).build(); + tops = client.tableOperations(); + nops = client.namespaceOperations(); + appender.start(); + loggerConfig.getRootLogger().addAppender(appender, Level.WARN, null); + loggerContext.updateLoggers(); + } + + @AfterAll + public static void shutdown() throws Exception { + client.close(); + SharedMiniClusterBase.stopMiniCluster(); + loggerConfig.getRootLogger().removeAppender(appender.getName()); + appender.stop(); + loggerContext.updateLoggers(); + } + + @Test + public void testTableIterConflict() throws Throwable { + final String[] tableNames = getUniqueNames(13); + String table1 = tableNames[0]; + String table2 = tableNames[1]; + String table3 = tableNames[2]; + String table4 = tableNames[3]; + String ns5 = tableNames[4]; + String table5 = ns5 + "." + tableNames[5]; + String ns6 = tableNames[6]; + String table6 = ns6 + "." + tableNames[7]; + String ns7 = tableNames[8]; + String table7 = ns7 + "." + tableNames[9]; + String table8 = tableNames[10]; + for (String ns : List.of(ns5, ns6, ns7)) { + nops.create(ns); + } + for (String table : List.of(table1, table2, table3, table4, table5, table6, table7, table8)) { + tops.create(table); + } + + // testing Scanner.addScanIterator + try (var scanner1 = client.createScanner(table1); var scanner2 = client.createScanner(table1)) { + testTableIterConflict(table1, RuntimeException.class, () -> { + scanner1.addScanIterator(iter1PrioConflict); + scanner1.iterator().hasNext(); + }, () -> { + scanner2.addScanIterator(iter1NameConflict); + scanner2.iterator().hasNext(); + }, false); + } + + // testing TableOperations.setProperty + testTableIterConflict(table2, AccumuloException.class, + () -> tops.setProperty(table2, iter1PrioConflictKey, iter1PrioConflictVal), + () -> tops.setProperty(table2, iter1NameConflictKey, iter1NameConflictVal), false); + + // testing TableOperations.modifyProperties + testTableIterConflict(table3, AccumuloException.class, + () -> tops.modifyProperties(table3, + props -> props.put(iter1PrioConflictKey, iter1PrioConflictVal)), + () -> tops.modifyProperties(table3, + props -> props.put(iter1NameConflictKey, iter1NameConflictVal)), + false); + + // NewTableConfiguration.attachIterator is not applicable for this test + // Attaching the iterator to the table requires the table to exist, but testing + // NewTableConfiguration.attachIterator requires that the table does not exist + + // testing TableOperations.attachIterator + testTableIterConflict(table4, AccumuloException.class, + () -> tops.attachIterator(table4, iter1PrioConflict), + () -> tops.attachIterator(table4, iter1NameConflict), true); + + // testing NamespaceOperations.attachIterator + testTableIterConflict(table5, AccumuloException.class, + () -> nops.attachIterator(ns5, iter1PrioConflict), + () -> nops.attachIterator(ns5, iter1NameConflict), false); + + // testing NamespaceOperations.setProperty + testTableIterConflict(table6, AccumuloException.class, + () -> nops.setProperty(ns6, iter1PrioConflictKey, iter1PrioConflictVal), + () -> nops.setProperty(ns6, iter1NameConflictKey, iter1NameConflictVal), false); + + // testing NamespaceOperations.modifyProperties + testTableIterConflict(table7, AccumuloException.class, + () -> nops.modifyProperties(ns7, + props -> props.put(iter1PrioConflictKey, iter1PrioConflictVal)), + () -> nops.modifyProperties(ns7, + props -> props.put(iter1NameConflictKey, iter1NameConflictVal)), + false); + + // testing CloneConfiguration.Builder.setPropertiesToSet + String table9 = tableNames[11]; + String table10 = tableNames[12]; + testTableIterConflict(table8, AccumuloException.class, + () -> tops.clone(table8, table9, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1PrioConflictKey, iter1PrioConflictVal)).build()), + () -> tops.clone(table8, table10, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1NameConflictKey, iter1NameConflictVal)).build()), + false); + } + + private <T extends Exception> void testTableIterConflict(String table, Class<T> exceptionClass, + Executable iterPrioConflictExec, Executable iterNameConflictExec, boolean shouldThrow) + throws Throwable { + tops.attachIterator(table, iter1); + if (shouldThrow) { + var e = assertThrows(exceptionClass, iterPrioConflictExec); + assertTrue(e.toString().contains("iterator priority conflict")); + e = assertThrows(exceptionClass, iterNameConflictExec); + assertTrue(e.toString().contains("iterator name conflict")); + } else { + assertTrue(logsContain(List.of("iterator priority conflict"), iterPrioConflictExec)); + assertTrue(logsContain(List.of("iterator name conflict"), iterNameConflictExec)); + } + } + + @Test + public void testNamespaceIterConflict() throws Throwable { + final String[] names = getUniqueNames(28); + + // testing Scanner.addScanIterator + String ns1 = names[0]; + nops.create(ns1); + String table1 = ns1 + "." + names[1]; + tops.create(table1); + try (var scanner1 = client.createScanner(table1); var scanner2 = client.createScanner(table1)) { + testNamespaceIterConflict(ns1, RuntimeException.class, () -> { + scanner1.addScanIterator(iter1PrioConflict); + scanner1.iterator().hasNext(); + }, () -> { + scanner2.addScanIterator(iter1NameConflict); + scanner2.iterator().hasNext(); + }, false); + } + + // testing TableOperations.setProperty + String ns2 = names[2]; + nops.create(ns2); + String table2 = ns2 + "." + names[3]; + tops.create(table2); + testNamespaceIterConflict(ns2, AccumuloException.class, + () -> tops.setProperty(table2, iter1PrioConflictKey, iter1PrioConflictVal), + () -> tops.setProperty(table2, iter1NameConflictKey, iter1NameConflictVal), false); + + // testing TableOperations.modifyProperties + String ns3 = names[4]; + nops.create(ns3); + String table3 = ns3 + "." + names[5]; + tops.create(table3); + testNamespaceIterConflict(ns3, AccumuloException.class, + () -> tops.modifyProperties(table3, + props -> props.put(iter1PrioConflictKey, iter1PrioConflictVal)), + () -> tops.modifyProperties(table3, + props -> props.put(iter1NameConflictKey, iter1NameConflictVal)), + false); + + // testing NewTableConfiguration.attachIterator + String ns4 = names[6]; + nops.create(ns4); + String table4 = ns4 + "." + names[7]; + String table5 = ns4 + "." + names[8]; + testNamespaceIterConflict(ns4, AccumuloException.class, + () -> tops.create(table4, new NewTableConfiguration().attachIterator(iter1PrioConflict)), + () -> tops.create(table5, new NewTableConfiguration().attachIterator(iter1NameConflict)), + false); + + // testing TableOperations.attachIterator + String ns5 = names[9]; + nops.create(ns5); + String table6 = ns5 + "." + names[10]; + tops.create(table6); + testNamespaceIterConflict(ns5, AccumuloException.class, + () -> tops.attachIterator(table6, iter1PrioConflict), + () -> tops.attachIterator(table6, iter1NameConflict), true); + + // testing NamespaceOperations.attachIterator + String ns6 = names[11]; + nops.create(ns6); + testNamespaceIterConflict(ns6, AccumuloException.class, + () -> nops.attachIterator(ns6, iter1PrioConflict), + () -> nops.attachIterator(ns6, iter1NameConflict), true); + + // testing NamespaceOperations.setProperty + String ns7 = names[12]; + nops.create(ns7); + testNamespaceIterConflict(ns7, AccumuloException.class, + () -> nops.setProperty(ns7, iter1PrioConflictKey, iter1PrioConflictVal), + () -> nops.setProperty(ns7, iter1NameConflictKey, iter1NameConflictVal), false); + + // testing NamespaceOperations.modifyProperties + String ns8 = names[13]; + nops.create(ns8); + testNamespaceIterConflict(ns8, AccumuloException.class, + () -> nops.modifyProperties(ns8, + props -> props.put(iter1PrioConflictKey, iter1PrioConflictVal)), + () -> nops.modifyProperties(ns8, + props -> props.put(iter1NameConflictKey, iter1NameConflictVal)), + false); + + // testing CloneConfiguration.Builder.setPropertiesToSet + // testing same src and dst namespace: should conflict + String dstAndSrcNamespace1 = names[14]; + nops.create(dstAndSrcNamespace1); + String src1 = dstAndSrcNamespace1 + "." + names[15]; + tops.create(src1); + String dst1 = dstAndSrcNamespace1 + "." + names[16]; + String dst2 = dstAndSrcNamespace1 + "." + names[17]; + testNamespaceIterConflict(dstAndSrcNamespace1, AccumuloException.class, + () -> tops.clone(src1, dst1, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1PrioConflictKey, iter1PrioConflictVal)).build()), + () -> tops.clone(src1, dst2, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1NameConflictKey, iter1NameConflictVal)).build()), + false); + // testing attached to src namespace, different dst namespace: should not conflict + String srcNamespace2 = names[18]; + nops.create(srcNamespace2); + nops.attachIterator(srcNamespace2, iter1); + String src2 = srcNamespace2 + "." + names[19]; + tops.create(src2); + String dstNamespace2 = names[20]; + nops.create(dstNamespace2); + String dst3 = dstNamespace2 + "." + names[21]; + String dst4 = dstNamespace2 + "." + names[22]; + // should NOT throw + tops.clone(src2, dst3, CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1PrioConflictKey, iter1PrioConflictVal)).build()); + // should NOT throw + tops.clone(src2, dst4, CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1NameConflictKey, iter1NameConflictVal)).build()); + // testing attached to dst namespace, different src namespace: should conflict + String srcNamespace3 = names[23]; + nops.create(srcNamespace3); + String src3 = srcNamespace3 + "." + names[24]; + tops.create(src3); + String dstNamespace3 = names[25]; + nops.create(dstNamespace3); + String dst5 = dstNamespace3 + "." + names[26]; + String dst6 = dstNamespace3 + "." + names[27]; + testNamespaceIterConflict(dstNamespace3, AccumuloException.class, + () -> tops.clone(src3, dst5, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1PrioConflictKey, iter1PrioConflictVal)).build()), + () -> tops.clone(src3, dst6, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(iter1NameConflictKey, iter1NameConflictVal)).build()), + false); + } + + private <T extends Exception> void testNamespaceIterConflict(String ns, Class<T> exceptionClass, + Executable iterPrioConflictExec, Executable iterNameConflictExec, boolean shouldThrow) + throws Throwable { + nops.attachIterator(ns, iter1); + + if (shouldThrow) { + var e = assertThrows(exceptionClass, iterPrioConflictExec); + assertTrue(e.toString().contains("iterator priority conflict")); + e = assertThrows(exceptionClass, iterNameConflictExec); + assertTrue(e.toString().contains("iterator name conflict")); Review Comment: Resolved from https://github.com/kevinrr888/accumulo/pull/3 -- 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]
