diff --git a/core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java b/core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java index 9aacc7ca18f..c35aca8c0f2 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java +++ b/core/src/main/java/org/apache/accumulo/core/client/admin/NewTableConfiguration.java @@ -198,30 +198,31 @@ public Map getProperties() { var initTableProps = IteratorConfigUtil.getInitialTableProperties(); // check the properties for conflicts with default iterators var defaultIterSettings = IteratorConfigUtil.getInitialTableIteratorSettings(); - // if a default prop already exists, don't want to consider that a conflict - var noDefaultsPropMap = new HashMap<>(propertyMap); - noDefaultsPropMap.entrySet().removeIf(entry -> initTableProps.get(entry.getKey()) != null - && initTableProps.get(entry.getKey()).equals(entry.getValue())); - defaultIterSettings.forEach((setting, scopes) -> { + for (var defaultIterSetting : defaultIterSettings.entrySet()) { + var setting = defaultIterSetting.getKey(); + var scopes = defaultIterSetting.getValue(); try { - TableOperationsHelper.checkIteratorConflicts(noDefaultsPropMap, setting, scopes); + TableOperationsHelper.checkIteratorConflicts(propertyMap, setting, scopes); } catch (AccumuloException e) { - throw new IllegalStateException(String.format( + throw new IllegalArgumentException(String.format( "conflict with default table iterator: scopes: %s setting: %s", scopes, setting), e); } - }); + } // check the properties for conflicts with default properties (non-iterator) var nonIterDefaults = IteratorConfigUtil.getInitialTableProperties(); nonIterDefaults.keySet().removeAll(IteratorConfigUtil.getInitialTableIterators().keySet()); - nonIterDefaults.forEach((dk, dv) -> { + for (var nonIterDefault : nonIterDefaults.entrySet()) { + var dk = nonIterDefault.getKey(); + var dv = nonIterDefault.getValue(); var valInPropMap = propertyMap.get(dk); - Preconditions.checkState(valInPropMap == null || valInPropMap.equals(dv), String.format( + Preconditions.checkArgument(valInPropMap == null || valInPropMap.equals(dv), String.format( "conflict for property %s : %s (default val) != %s (set val)", dk, dv, valInPropMap)); - }); + } propertyMap.putAll(initTableProps); } + return Collections.unmodifiableMap(propertyMap); } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsHelper.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsHelper.java index d05e16afc6c..9df25d3251a 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsHelper.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsHelper.java @@ -32,6 +32,7 @@ import org.apache.accumulo.core.client.admin.NamespaceOperations; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; public abstract class NamespaceOperationsHelper implements NamespaceOperations { @@ -146,45 +147,8 @@ public void checkIteratorConflicts(String namespace, IteratorSetting setting, if (!exists(namespace)) { throw new NamespaceNotFoundException(null, namespace, null); } - for (IteratorScope scope : scopes) { - String scopeStr = - String.format("%s%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase()); - String nameStr = String.format("%s.%s", scopeStr, setting.getName()); - String optStr = String.format("%s.opt.", nameStr); - Map optionConflicts = new TreeMap<>(); - for (Entry property : this.getProperties(namespace)) { - if (property.getKey().startsWith(scopeStr)) { - if (property.getKey().equals(nameStr)) { - throw new AccumuloException(new IllegalArgumentException("iterator name conflict for " - + setting.getName() + ": " + property.getKey() + "=" + property.getValue())); - } - if (property.getKey().startsWith(optStr)) { - optionConflicts.put(property.getKey(), property.getValue()); - } - if (property.getKey().contains(".opt.")) { - continue; - } - String[] parts = property.getValue().split(","); - if (parts.length != 2) { - throw new AccumuloException("Bad value for existing iterator setting: " - + property.getKey() + "=" + property.getValue()); - } - try { - if (Integer.parseInt(parts[0]) == setting.getPriority()) { - throw new AccumuloException(new IllegalArgumentException( - "iterator priority conflict: " + property.getKey() + "=" + property.getValue())); - } - } catch (NumberFormatException e) { - throw new AccumuloException("Bad value for existing iterator setting: " - + property.getKey() + "=" + property.getValue()); - } - } - } - if (!optionConflicts.isEmpty()) { - throw new AccumuloException(new IllegalArgumentException( - "iterator options conflict for " + setting.getName() + ": " + optionConflicts)); - } - } + var props = this.getNamespaceProperties(namespace); + IteratorConfigUtil.checkIteratorConflicts(props, setting, scopes, true); } @Override diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java index 451496ed9c5..6c7b870a180 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/NamespaceOperationsImpl.java @@ -57,6 +57,7 @@ import org.apache.accumulo.core.data.constraints.Constraint; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.manager.thrift.FateOperation; import org.apache.accumulo.core.rpc.clients.ThriftClientTypes; import org.apache.accumulo.core.trace.TraceUtil; @@ -186,6 +187,8 @@ public void setProperty(final String namespace, final String property, final Str checkArgument(value != null, "value is null"); try { + IteratorConfigUtil.checkIteratorConflicts(tableOps, this, namespace, property, value); + ThriftClientTypes.MANAGER.executeVoidTableCommand(context, client -> client.setNamespaceProperty(TraceUtil.traceInfo(), context.rpcCreds(), namespace, property, value)); @@ -215,6 +218,11 @@ private Map tryToModifyProperties(final String namespace, // from here on the code is assured to always be dealing with the same map. vProperties.setProperties(Map.copyOf(vProperties.getProperties())); + for (var property : vProperties.getProperties().entrySet()) { + IteratorConfigUtil.checkIteratorConflicts(tableOps, this, namespace, property.getKey(), + property.getValue()); + } + try { // Send to server ThriftClientTypes.MANAGER.executeVoidTableCommand(context, @@ -374,6 +382,8 @@ public void attachIterator(String namespace, IteratorSetting setting, throws AccumuloSecurityException, AccumuloException, NamespaceNotFoundException { // testClassLoad validates the namespace name testClassLoad(namespace, setting.getIteratorClass(), SortedKeyValueIterator.class.getName()); + IteratorConfigUtil.checkIteratorConflictsWithTablesInNamespace(tableOps, namespace, setting, + scopes); super.attachIterator(namespace, setting, scopes); } diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java index 030bb958806..6cf7e5e3d78 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsHelper.java @@ -35,6 +35,7 @@ import org.apache.accumulo.core.client.admin.TableOperations; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; public abstract class TableOperationsHelper implements TableOperations { @@ -139,45 +140,7 @@ public static void checkIteratorConflicts(Map props, IteratorSett EnumSet scopes) throws AccumuloException { checkArgument(setting != null, "setting is null"); checkArgument(scopes != null, "scopes is null"); - for (IteratorScope scope : scopes) { - String scopeStr = - String.format("%s%s", Property.TABLE_ITERATOR_PREFIX, scope.name().toLowerCase()); - String nameStr = String.format("%s.%s", scopeStr, setting.getName()); - String optStr = String.format("%s.opt.", nameStr); - Map optionConflicts = new TreeMap<>(); - for (Entry property : props.entrySet()) { - if (property.getKey().startsWith(scopeStr)) { - if (property.getKey().equals(nameStr)) { - throw new AccumuloException(new IllegalArgumentException("iterator name conflict for " - + setting.getName() + ": " + property.getKey() + "=" + property.getValue())); - } - if (property.getKey().startsWith(optStr)) { - optionConflicts.put(property.getKey(), property.getValue()); - } - if (property.getKey().contains(".opt.")) { - continue; - } - String[] parts = property.getValue().split(","); - if (parts.length != 2) { - throw new AccumuloException("Bad value for existing iterator setting: " - + property.getKey() + "=" + property.getValue()); - } - try { - if (Integer.parseInt(parts[0]) == setting.getPriority()) { - throw new AccumuloException(new IllegalArgumentException( - "iterator priority conflict: " + property.getKey() + "=" + property.getValue())); - } - } catch (NumberFormatException e) { - throw new AccumuloException("Bad value for existing iterator setting: " - + property.getKey() + "=" + property.getValue()); - } - } - } - if (!optionConflicts.isEmpty()) { - throw new AccumuloException(new IllegalArgumentException( - "iterator options conflict for " + setting.getName() + ": " + optionConflicts)); - } - } + IteratorConfigUtil.checkIteratorConflicts(props, setting, scopes, true); } @Override diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java index 17727b2b8f1..ac7fe53f8ea 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java @@ -116,6 +116,7 @@ import org.apache.accumulo.core.dataImpl.thrift.TSummaryRequest; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.manager.state.tables.TableState; import org.apache.accumulo.core.manager.thrift.FateOperation; import org.apache.accumulo.core.manager.thrift.FateService; @@ -1008,10 +1009,13 @@ public void setProperty(final String tableName, final String property, final Str checkArgument(value != null, "value is null"); try { + IteratorConfigUtil.checkIteratorConflicts(Map.copyOf(this.getConfiguration(tableName)), + property, value); + setPropertyNoChecks(tableName, property, value); checkLocalityGroups(tableName, property); - } catch (TableNotFoundException e) { + } catch (TableNotFoundException | IllegalArgumentException e) { throw new AccumuloException(e); } } @@ -1022,6 +1026,13 @@ private Map tryToModifyProperties(String tableName, final TVersionedProperties vProperties = ThriftClientTypes.CLIENT.execute(context, client -> client .getVersionedTableProperties(TraceUtil.traceInfo(), context.rpcCreds(), tableName)); + final Map configBeforeMut; + try { + configBeforeMut = getConfiguration(tableName); + } catch (TableNotFoundException e) { + throw new AccumuloException(e); + } + mapMutator.accept(vProperties.getProperties()); // A reference to the map was passed to the user, maybe they still have the reference and are @@ -1030,6 +1041,11 @@ private Map tryToModifyProperties(String tableName, // from here on the code is assured to always be dealing with the same map. vProperties.setProperties(Map.copyOf(vProperties.getProperties())); + for (var property : vProperties.getProperties().entrySet()) { + IteratorConfigUtil.checkIteratorConflicts(configBeforeMut, property.getKey(), + property.getValue()); + } + try { // Send to server ThriftClientTypes.MANAGER.executeVoid(context, diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java index 59224deb9b9..1ac3adcdf60 100644 --- a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java @@ -29,11 +29,20 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.TreeMap; +import java.util.stream.Collectors; import org.apache.accumulo.core.classloader.ClassLoaderUtil; +import org.apache.accumulo.core.client.AccumuloException; +import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.client.NamespaceNotFoundException; +import org.apache.accumulo.core.client.TableNotFoundException; +import org.apache.accumulo.core.client.admin.TableOperations; +import org.apache.accumulo.core.clientImpl.Namespace; +import org.apache.accumulo.core.clientImpl.NamespaceOperationsHelper; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; @@ -57,6 +66,9 @@ public class IteratorConfigUtil { public static final Comparator ITER_INFO_COMPARATOR = Comparator.comparingInt(IterInfo::getPriority); + private static final String WARNING_MSG = + ". Iterator was set as requested, but may lead to non-deterministic behavior."; + /** * Fetch the correct configuration key prefix for the given scope. Throws an * IllegalArgumentException if no property exists for the given scope. @@ -122,24 +134,15 @@ public static List parseIterConf(IteratorScope scope, List i Map> allOptions, AccumuloConfiguration conf) { Map properties = conf.getAllPropertiesWithPrefix(getProperty(scope)); ArrayList iterators = new ArrayList<>(iters); - final Property scopeProperty = getProperty(scope); - final String scopePropertyKey = scopeProperty.getKey(); for (Entry entry : properties.entrySet()) { - String suffix = entry.getKey().substring(scopePropertyKey.length()); - String[] suffixSplit = suffix.split("\\.", 3); - - if (suffixSplit.length == 1) { - String[] sa = entry.getValue().split(","); - int prio = Integer.parseInt(sa[0]); - String className = sa[1]; - iterators.add(new IterInfo(prio, className, suffixSplit[0])); - } else if (suffixSplit.length == 3 && suffixSplit[1].equals("opt")) { - String iterName = suffixSplit[0]; - String optName = suffixSplit[2]; - allOptions.computeIfAbsent(iterName, k -> new HashMap<>()).put(optName, entry.getValue()); + var iterProp = IteratorProperty.parse(entry.getKey(), entry.getValue()); + if (iterProp.isOption()) { + allOptions.computeIfAbsent(iterProp.getName(), k -> new HashMap<>()) + .put(iterProp.getOptionKey(), iterProp.getOptionValue()); } else { - throw new IllegalArgumentException("Invalid iterator format: " + entry.getKey()); + iterators + .add(new IterInfo(iterProp.getPriority(), iterProp.getClassName(), iterProp.getName())); } } @@ -273,4 +276,191 @@ private static Class> loadClass(boolean useAcc log.trace("Iterator class {} loaded from classpath", iterInfo.className); return clazz; } + + public static void checkIteratorConflicts(Map props, String property, String value) + throws AccumuloException { + if (Objects.equals(props.get(property), value)) { + // setting a property that already exists (i.e., no change) + return; + } + + var iterProp = IteratorProperty.parse(property, value); + if (iterProp != null && !iterProp.isOption()) { + // given a single property, the only way for the property to be equivalent to an existing + // iterator is if the existing iterator has no options (opts are set as separate props) + checkIteratorConflicts(props, iterProp.toSetting(), EnumSet.of(iterProp.getScope()), false); + } + } + + public static void checkIteratorConflicts(TableOperations tableOps, NamespaceOperationsHelper noh, + String namespace, String property, String value) + throws AccumuloException, AccumuloSecurityException, NamespaceNotFoundException { + var props = noh.getNamespaceProperties(namespace); + if (Objects.equals(props.get(property), value)) { + // setting a property that already exists (i.e., no change) + return; + } + + var iterProp = IteratorProperty.parse(property, value); + if (iterProp != null && !iterProp.isOption()) { + // given a single property, the only way for the property to be equivalent to an existing + // iterator is if the existing iterator has no options (opts are set as separate props) + checkIteratorConflicts(props, iterProp.toSetting(), EnumSet.of(iterProp.getScope()), false); + } + + // checking for conflicts for the tables in the namespace + checkIteratorConflictsWithTablesInNamespace(tableOps, namespace, property, value); + } + + public static void checkIteratorConflictsWithTablesInNamespace(TableOperations tableOps, + String namespace, IteratorSetting is, EnumSet scopes) + throws AccumuloException { + Set tablesInNamespace; + if (namespace.equals(Namespace.DEFAULT.name())) { + tablesInNamespace = tableOps.list().stream().filter(t -> !t.contains(Namespace.SEPARATOR)) + .collect(Collectors.toSet()); + } else { + tablesInNamespace = tableOps.list().stream() + .filter(t -> t.startsWith(namespace + Namespace.SEPARATOR)).collect(Collectors.toSet()); + } + try { + for (var table : tablesInNamespace) { + checkIteratorConflicts(tableOps.getTableProperties(table), is, scopes, false); + } + } catch (TableNotFoundException e) { + throw new AccumuloException(e); + } + } + + public static void checkIteratorConflictsWithTablesInNamespace(TableOperations tableOps, + String namespace, String property, String value) throws AccumuloException { + Set tablesInNamespace; + if (namespace.equals(Namespace.DEFAULT.name())) { + tablesInNamespace = tableOps.list().stream().filter(t -> !t.contains(Namespace.SEPARATOR)) + .collect(Collectors.toSet()); + } else { + tablesInNamespace = tableOps.list().stream() + .filter(t -> t.startsWith(namespace + Namespace.SEPARATOR)).collect(Collectors.toSet()); + } + try { + for (var table : tablesInNamespace) { + checkIteratorConflicts(tableOps.getTableProperties(table), property, value); + } + } catch (TableNotFoundException e) { + throw new AccumuloException(e); + } + } + + public static void checkIteratorConflicts(IteratorSetting iterToCheck, + EnumSet iterScopesToCheck, + Map> existingIters, boolean shouldThrow) + throws AccumuloException { + // The reason for the 'shouldThrow' var is to prevent newly added 2.x checks from breaking + // existing user code. Just log the problem and proceed. Major version > 2 will always throw + for (var scope : iterScopesToCheck) { + var existingItersForScope = existingIters.get(scope); + if (existingItersForScope == null) { + continue; + } + for (var existingIter : existingItersForScope) { + // not a conflict if exactly the same + if (iterToCheck.equals(existingIter)) { + continue; + } + if (iterToCheck.getName().equals(existingIter.getName())) { + String msg = + String.format("iterator name conflict at %s scope. %s conflicts with existing %s", + scope, iterToCheck, existingIter); + if (shouldThrow) { + throw new AccumuloException(new IllegalArgumentException(msg)); + } else { + log.warn(msg + WARNING_MSG); + } + } + if (iterToCheck.getPriority() == existingIter.getPriority()) { + String msg = + String.format("iterator priority conflict at %s scope. %s conflicts with existing %s", + scope, iterToCheck, existingIter); + if (shouldThrow) { + throw new AccumuloException(new IllegalArgumentException(msg)); + } else { + log.warn(msg + WARNING_MSG); + } + } + } + } + } + + public static void checkIteratorConflicts(Map props, IteratorSetting iterToCheck, + EnumSet iterScopesToCheck, boolean shouldThrow) throws AccumuloException { + // parse the props map + Map> iteratorSettings = new HashMap<>(); + Map> existingIters = new HashMap<>(); + + for (var prop : props.entrySet()) { + var iterProp = IteratorProperty.parse(prop.getKey(), prop.getValue()); + if (iterProp != null && !iterProp.isOption() + && iterScopesToCheck.contains(iterProp.getScope())) { + var iterSetting = iterProp.toSetting(); + iteratorSettings.computeIfAbsent(iterProp.getScope(), s -> new HashMap<>()) + .put(iterProp.getName(), iterSetting); + existingIters.computeIfAbsent(iterProp.getScope(), s -> new ArrayList<>()).add(iterSetting); + } + } + + // check for conflicts + // any iterator option property not part of an existing iterator is an option conflict + for (var prop : props.entrySet()) { + var iterProp = IteratorProperty.parse(prop.getKey(), prop.getValue()); + if (iterProp != null && iterProp.isOption() + && iterScopesToCheck.contains(iterProp.getScope())) { + var iterSetting = + iteratorSettings.getOrDefault(iterProp.getScope(), Map.of()).get(iterProp.getName()); + if (iterSetting == null) { + String msg = String.format("iterator options conflict for %s : %s=%s", + iterToCheck.getName(), prop.getKey(), prop.getValue()); + if (shouldThrow) { + throw new AccumuloException(new IllegalArgumentException(msg)); + } else { + log.warn(msg + WARNING_MSG); + } + } else { + iterSetting.addOption(iterProp.getOptionKey(), iterProp.getOptionValue()); + } + } + } + // check if the given iterator conflicts with any existing iterators + checkIteratorConflicts(iterToCheck, iterScopesToCheck, existingIters, shouldThrow); + } + + /** + * Returns a new map of all the iterator props contained in the given map + */ + public static Map gatherIteratorProps(Map props) { + Map iterProps = new HashMap<>(); + for (var e : props.entrySet()) { + var iterProp = IteratorProperty.parse(e.getKey(), e.getValue()); + if (iterProp != null) { + iterProps.put(e.getKey(), e.getValue()); + } + } + return iterProps; + } + + /** + * returns a map of the options associated with the given iterator property key. Options of the + * iterator are obtained by searching the given map + */ + public static Map gatherIterOpts(IteratorProperty nameProp, + Map map) { + Map opts = new HashMap<>(); + for (var e : map.entrySet()) { + var iterProp = IteratorProperty.parse(e.getKey(), e.getValue()); + if (iterProp != null && iterProp.isOption() && nameProp.getName().equals(iterProp.getName()) + && nameProp.getScope().equals(iterProp.getScope())) { + opts.put(iterProp.getOptionKey(), iterProp.getOptionValue()); + } + } + return opts; + } } diff --git a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorProperty.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorProperty.java new file mode 100644 index 00000000000..d9812fb3084 --- /dev/null +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorProperty.java @@ -0,0 +1,138 @@ +/* + * 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.core.iteratorsImpl; + +import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.conf.Property; +import org.apache.accumulo.core.iterators.IteratorUtil; + +import com.google.common.base.Preconditions; + +/** + * Utility for parsing a single iterator key/value property. + */ +public class IteratorProperty { + + private final IteratorUtil.IteratorScope scope; + private final String name; + + private final int priority; + private final String className; + + private final String optionKey; + private final String optionValue; + + private IteratorProperty(String iterName, IteratorUtil.IteratorScope scope, int priority, + String className) { + this.name = iterName; + this.scope = scope; + this.priority = priority; + this.className = className; + this.optionKey = null; + this.optionValue = null; + } + + private IteratorProperty(String iterName, IteratorUtil.IteratorScope scope, String optionName, + String optionValue) { + this.name = iterName; + this.scope = scope; + this.priority = -1; + this.className = null; + this.optionKey = optionName; + this.optionValue = optionValue; + } + + public boolean isOption() { + return optionKey != null; + } + + public String getClassName() { + Preconditions.checkState(!isOption()); + return className; + } + + public String getName() { + return name; + } + + public String getOptionKey() { + Preconditions.checkState(isOption()); + return optionKey; + } + + public String getOptionValue() { + Preconditions.checkState(isOption()); + return optionValue; + } + + public int getPriority() { + Preconditions.checkState(!isOption()); + return priority; + } + + public IteratorUtil.IteratorScope getScope() { + return scope; + } + + /** + * Creates an initial iterator setting without options. + * + * @throws IllegalStateException if {@link #isOption()} returns true + */ + public IteratorSetting toSetting() { + Preconditions.checkState(!isOption()); + return new IteratorSetting(getPriority(), getName(), getClassName()); + } + + private static void check(boolean b, String property, String value) { + if (!b) { + throw new IllegalArgumentException("Illegal iterator property: " + property + "=" + value); + } + } + + /** + * Parses an iterator key value property. + * + * @return parsed iterator property or null if the property does not start with the iterator + * property prefix. + * @throws RuntimeException if the iterator property is malformed. Will actually throw a few + * different subclasses of this exception. + */ + public static IteratorProperty parse(String property, String value) { + if (!property.startsWith(Property.TABLE_ITERATOR_PREFIX.getKey())) { + return null; + } + + String[] iterPropParts = property.split("\\."); + check(iterPropParts.length == 4 || iterPropParts.length == 6, property, value); + IteratorUtil.IteratorScope scope = IteratorUtil.IteratorScope.valueOf(iterPropParts[2]); + String iterName = iterPropParts[3]; + + if (iterPropParts.length == 4) { + String[] valTokens = value.split(","); + check(valTokens.length == 2, property, value); + return new IteratorProperty(iterName, scope, Integer.parseInt(valTokens[0]), valTokens[1]); + } else if (iterPropParts.length == 6) { + check(iterPropParts[4].equals("opt"), property, value); + return new IteratorProperty(iterName, scope, iterPropParts[5], value); + } else { + throw new IllegalArgumentException("Illegal iterator property: " + property + "=" + value); + } + } +} diff --git a/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java b/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java index d0699e8a1d7..97917015719 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java +++ b/server/base/src/main/java/org/apache/accumulo/server/conf/TableConfiguration.java @@ -25,6 +25,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; import org.apache.accumulo.core.classloader.ClassLoaderUtil; @@ -116,6 +117,8 @@ public static class ParsedIteratorConfig { private final List tableIters; private final Map> tableOpts; private final String context; + private final Set uniqueNames; + private final Set uniquePriorities; private ParsedIteratorConfig(List ii, Map> opts, String context) { @@ -123,6 +126,10 @@ private ParsedIteratorConfig(List ii, Map> o tableOpts = opts.entrySet().stream() .collect(Collectors.toUnmodifiableMap(Entry::getKey, e -> Map.copyOf(e.getValue()))); this.context = context; + uniqueNames = + tableIters.stream().map(IterInfo::getIterName).collect(Collectors.toUnmodifiableSet()); + uniquePriorities = + tableIters.stream().map(IterInfo::getPriority).collect(Collectors.toUnmodifiableSet()); } public List getIterInfo() { @@ -136,6 +143,14 @@ public Map> getOpts() { public String getServiceEnv() { return context; } + + public Set getUniqueNames() { + return uniqueNames; + } + + public Set getUniquePriorities() { + return uniquePriorities; + } } public ParsedIteratorConfig getParsedIteratorConfig(IteratorScope scope) { diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java index 57e3858a2da..aa053b01b72 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/FateServiceHandler.java @@ -44,6 +44,7 @@ import java.util.Set; import java.util.stream.Collectors; +import org.apache.accumulo.core.client.AccumuloException; import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.NamespaceNotFoundException; import org.apache.accumulo.core.client.TableNotFoundException; @@ -63,6 +64,8 @@ import org.apache.accumulo.core.data.NamespaceId; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.fate.ReadOnlyTStore.TStatus; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; +import org.apache.accumulo.core.iteratorsImpl.IteratorProperty; import org.apache.accumulo.core.manager.state.tables.TableState; import org.apache.accumulo.core.manager.thrift.FateOperation; import org.apache.accumulo.core.manager.thrift.FateService; @@ -90,6 +93,7 @@ import org.apache.accumulo.manager.tableOps.tableExport.ExportTable; import org.apache.accumulo.manager.tableOps.tableImport.ImportTable; import org.apache.accumulo.server.client.ClientServiceHandler; +import org.apache.accumulo.server.conf.store.TablePropKey; import org.apache.accumulo.server.manager.state.MergeInfo; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.fs.FSDataOutputStream; @@ -216,16 +220,12 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe throw new ThriftSecurityException(c.getPrincipal(), SecurityErrorCode.PERMISSION_DENIED); } + var namespaceIterProps = IteratorConfigUtil + .gatherIteratorProps(manager.getContext().getNamespaceConfiguration(namespaceId) + .getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX)); for (Map.Entry entry : options.entrySet()) { - if (!Property.isValidProperty(entry.getKey(), entry.getValue())) { - String errorMessage = "Property or value not valid "; - if (!Property.isValidTablePropertyKey(entry.getKey())) { - errorMessage = "Invalid Table Property "; - } - throw new ThriftTableOperationException(null, tableName, tableOp, - TableOperationExceptionType.OTHER, - errorMessage + entry.getKey() + "=" + entry.getValue()); - } + validateTableProperty(entry.getKey(), entry.getValue(), options, namespaceIterProps, + tableName, tableOp); } goalMessage += "Create table " + tableName + " " + initialTableState + " with " + splitCount @@ -322,23 +322,34 @@ public void executeFateOperation(TInfo tinfo, TCredentials c, long opid, FateOpe Map propertiesToSet = new HashMap<>(); Set propertiesToExclude = new HashSet<>(); + // dest table will have the dest namespace props + src table props: need to check provided + // options to set for conflicts with this + var iterProps = new HashMap<>(manager.getContext().getNamespaceConfiguration(namespaceId) + .getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX)); + // get only the source table props, not the merged view + var srcTableProps = manager.getContext().getPropStore() + .get(TablePropKey.of(manager.getContext(), srcTableId)).asMap(); + for (Entry entry : options.entrySet()) { if (entry.getKey().startsWith(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX)) { propertiesToExclude.add( entry.getKey().substring(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX.length())); - continue; } + } - if (!Property.isValidProperty(entry.getKey(), entry.getValue())) { - String errorMessage = "Property or value not valid "; - if (!Property.isValidTablePropertyKey(entry.getKey())) { - errorMessage = "Invalid Table Property "; - } - throw new ThriftTableOperationException(null, tableName, tableOp, - TableOperationExceptionType.OTHER, - errorMessage + entry.getKey() + "=" + entry.getValue()); + // these props will not be cloned + srcTableProps.keySet().removeAll(propertiesToExclude); + // merge src table props into dest namespace props + iterProps.putAll(srcTableProps); + + for (Entry entry : options.entrySet()) { + if (entry.getKey().startsWith(TableOperationsImpl.PROPERTY_EXCLUDE_PREFIX)) { + continue; } + validateTableProperty(entry.getKey(), entry.getValue(), options, + IteratorConfigUtil.gatherIteratorProps(iterProps), tableName, tableOp); + propertiesToSet.put(entry.getKey(), entry.getValue()); } @@ -844,6 +855,35 @@ private void writeSplitsToFile(Path splitsPath, final List arguments } } + private void validateTableProperty(String propKey, String propVal, Map propMap, + Map config, String tableName, TableOperation tableOp) + throws ThriftTableOperationException { + // validating property as valid table property + if (!Property.isValidProperty(propKey, propVal)) { + String errorMessage = "Property or value not valid "; + if (!Property.isValidTablePropertyKey(propKey)) { + errorMessage = "Invalid Table Property "; + } + throw new ThriftTableOperationException(null, tableName, tableOp, + TableOperationExceptionType.OTHER, errorMessage + propKey + "=" + propVal); + } + + // validating property does not create an iterator conflict with those in the config + var iterProp = IteratorProperty.parse(propKey, propVal); + if (iterProp != null && !iterProp.isOption()) { + Map opts = IteratorConfigUtil.gatherIterOpts(iterProp, propMap); + try { + var is = iterProp.toSetting(); + is.addOptions(opts); + IteratorConfigUtil.checkIteratorConflicts(config, is, EnumSet.of(iterProp.getScope()), + false); + } catch (AccumuloException e) { + throw new ThriftTableOperationException(null, tableName, tableOp, + TableOperationExceptionType.OTHER, e.getMessage()); + } + } + } + /** * Creates a temporary directory for the given FaTE operation (deleting any existing, to avoid * issues in case of server retry). diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java index 13c75157c63..f999feff0c3 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java @@ -20,12 +20,15 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import org.apache.accumulo.core.client.AccumuloException; +import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; @@ -243,6 +246,32 @@ private SortedKeyValueIterator createIterator() } else { // Scan time iterator options were set, so need to merge those with pre-parsed table // iterator options. + + // First ensure the set iterators do not conflict with the existing table iterators. + List picIteratorSettings = null; + for (var scanParamIterInfo : scanParams.getSsiList()) { + // Quick check for a potential iterator conflict (does not consider iterator scope). + // This avoids the more expensive check method call most of the time. + if (pic.getUniqueNames().contains(scanParamIterInfo.getIterName()) + || pic.getUniquePriorities().contains(scanParamIterInfo.getPriority())) { + if (picIteratorSettings == null) { + picIteratorSettings = new ArrayList<>(pic.getIterInfo().size()); + for (var picIterInfo : pic.getIterInfo()) { + picIteratorSettings.add( + getIteratorSetting(picIterInfo, pic.getOpts().get(picIterInfo.getIterName()))); + } + } + try { + IteratorConfigUtil.checkIteratorConflicts( + getIteratorSetting(scanParamIterInfo, + scanParams.getSsio().get(scanParamIterInfo.getIterName())), + EnumSet.of(IteratorScope.scan), Map.of(IteratorScope.scan, picIteratorSettings), + false); + } catch (AccumuloException e) { + throw new IllegalArgumentException(e); + } + } + } iterOpts = new HashMap<>(pic.getOpts().size() + scanParams.getSsio().size()); iterInfos = new ArrayList<>(pic.getIterInfo().size() + scanParams.getSsiList().size()); IteratorConfigUtil.mergeIteratorConfig(iterInfos, iterOpts, pic.getIterInfo(), @@ -272,6 +301,18 @@ private SortedKeyValueIterator createIterator() } } + private IteratorSetting getIteratorSetting(IterInfo iterInfo, Map iterOpts) { + IteratorSetting setting; + if (iterOpts != null) { + setting = new IteratorSetting(iterInfo.getPriority(), iterInfo.getIterName(), + iterInfo.getClassName(), iterOpts); + } else { + setting = new IteratorSetting(iterInfo.getPriority(), iterInfo.getIterName(), + iterInfo.getClassName()); + } + return setting; + } + private void returnIterators() { if (memIters != null) { log.trace("Returning mem iterators for {}, scanId:{}, fid:{}", tablet.getExtent(), diff --git a/test/pom.xml b/test/pom.xml index 4430f6d929e..30f5cf3d3c9 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -182,6 +182,14 @@ org.apache.logging.log4j log4j-1.2-api + + org.apache.logging.log4j + log4j-api + + + org.apache.logging.log4j + log4j-core + org.apache.thrift libthrift diff --git a/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java b/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java index bf044eb3857..4c99ab36605 100644 --- a/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java +++ b/test/src/main/java/org/apache/accumulo/test/NewTableConfigurationIT.java @@ -534,12 +534,12 @@ public void testConflictsWithDefaults() throws Exception { */ // add an iterator with same priority as the default iterator var iterator1 = new IteratorSetting(20, "foo", "foo.bar"); - var exception = assertThrows(IllegalStateException.class, () -> client.tableOperations() + var exception = assertThrows(IllegalArgumentException.class, () -> client.tableOperations() .create(table, new NewTableConfiguration().attachIterator(iterator1))); assertTrue(exception.getMessage().contains("conflict with default table iterator")); // add an iterator with same name as the default iterator var iterator2 = new IteratorSetting(10, "vers", "foo.bar"); - exception = assertThrows(IllegalStateException.class, () -> client.tableOperations() + exception = assertThrows(IllegalArgumentException.class, () -> client.tableOperations() .create(table, new NewTableConfiguration().attachIterator(iterator2))); assertTrue(exception.getMessage().contains("conflict with default table iterator")); // try to attach the exact default iterators, should not present a conflict as they are @@ -561,7 +561,7 @@ public void testConflictsWithDefaults() throws Exception { for (IteratorScope iterScope : IteratorScope.values()) { props.put(Property.TABLE_ITERATOR_PREFIX + iterScope.name() + ".foo", "20,foo.bar"); } - exception = assertThrows(IllegalStateException.class, () -> client.tableOperations() + exception = assertThrows(IllegalArgumentException.class, () -> client.tableOperations() .create(table2, new NewTableConfiguration().setProperties(props))); assertTrue(exception.getMessage().contains("conflict with default table iterator")); props.clear(); @@ -569,7 +569,7 @@ public void testConflictsWithDefaults() throws Exception { for (IteratorScope iterScope : IteratorScope.values()) { props.put(Property.TABLE_ITERATOR_PREFIX + iterScope.name() + ".vers", "10,foo.bar"); } - exception = assertThrows(IllegalStateException.class, () -> client.tableOperations() + exception = assertThrows(IllegalArgumentException.class, () -> client.tableOperations() .create(table2, new NewTableConfiguration().setProperties(props))); assertTrue(exception.getMessage().contains("conflict with default table iterator")); props.clear(); @@ -583,7 +583,7 @@ public void testConflictsWithDefaults() throws Exception { */ // setting a value different from default should throw props.put(Property.TABLE_CONSTRAINT_PREFIX + "1", "foo"); - exception = assertThrows(IllegalStateException.class, () -> client.tableOperations() + exception = assertThrows(IllegalArgumentException.class, () -> client.tableOperations() .create(table3, new NewTableConfiguration().setProperties(props))); assertTrue(exception.getMessage() .contains("conflict for property " + Property.TABLE_CONSTRAINT_PREFIX + "1")); diff --git a/test/src/main/java/org/apache/accumulo/test/functional/IteratorConflictsIT.java b/test/src/main/java/org/apache/accumulo/test/functional/IteratorConflictsIT.java new file mode 100644 index 00000000000..0cc56b2237d --- /dev/null +++ b/test/src/main/java/org/apache/accumulo/test/functional/IteratorConflictsIT.java @@ -0,0 +1,815 @@ +/* + * 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.assertEquals; +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.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +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.IteratorScope; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; +import org.apache.accumulo.harness.SharedMiniClusterBase; +import org.apache.accumulo.test.util.Wait; +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: + *

+ * - {@link Scanner#addScanIterator(IteratorSetting)} + *

+ * - {@link TableOperations#setProperty(String, String, String)} + *

+ * - {@link TableOperations#modifyProperties(String, Consumer)} + *

+ * - {@link NewTableConfiguration#attachIterator(IteratorSetting, EnumSet)} + *

+ * - {@link TableOperations#attachIterator(String, IteratorSetting, EnumSet)} + *

+ * - {@link NamespaceOperations#attachIterator(String, IteratorSetting, EnumSet)} + *

+ * - {@link NamespaceOperations#setProperty(String, String, String)} + *

+ * - {@link NamespaceOperations#modifyProperties(String, Consumer)} + *

+ * - {@link CloneConfiguration.Builder#setPropertiesToSet(Map)} + *

+ * All fail when conflicts arise from: + *

+ * - Iterators attached directly to a table + *

+ * - Iterators attached to a namespace, inherited by a table + *

+ * - Default table iterators, but should not fail if {@link NewTableConfiguration#withoutDefaults()} + * is specified + *

+ * - 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 + + 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 + IteratorScope.scan.name().toLowerCase() + ".othername"; + private static final String iter1PrioConflictVal = "99," + iterClass; + private static final String iter1NameConflictKey = Property.TABLE_ITERATOR_PREFIX + + 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 + IteratorScope.scan.name().toLowerCase() + ".foo"; + private static final String defaultIterPrioConflictVal = + defaultTableIter.getPriority() + "," + iterClass; + private static final String defaultIterNameConflictKey = Property.TABLE_ITERATOR_PREFIX + + IteratorScope.scan.name().toLowerCase() + "." + defaultTableIter.getName(); + private static final String defaultIterNameConflictVal = "99," + iterClass; + private static final String defaultIterKey = Property.TABLE_ITERATOR_PREFIX.getKey() + + 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() + + 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 events = new CopyOnWriteArrayList<>(); + + public TestAppender() { + super("TestAppender", null, PatternLayout.createDefaultLayout(), false, null); + } + + @Override + public void append(LogEvent event) { + events.add(event.toImmutable()); + } + + public List 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 { + loggerConfig.getRootLogger().removeAppender(appender.getName()); + appender.stop(); + loggerContext.updateLoggers(); + client.close(); + SharedMiniClusterBase.stopMiniCluster(); + } + + @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, new NewTableConfiguration().attachIterator(iter1)); + } + + // 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 void testTableIterConflict(String table, Class exceptionClass, + Executable iterPrioConflictExec, Executable iterNameConflictExec, boolean shouldThrow) + throws Throwable { + 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")); + assertEquals(Set.of(iter1.getName(), "vers"), tops.listIterators(table).keySet()); + for (var scope : IteratorScope.values()) { + assertEquals(iter1, tops.getIteratorSetting(table, iter1.getName(), scope)); + } + } 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 void testNamespaceIterConflict(String ns, Class exceptionClass, + Executable iterPrioConflictExec, Executable iterNameConflictExec, boolean shouldThrow) + throws Throwable { + nops.attachIterator(ns, iter1); + Wait.waitFor(() -> nops.listIterators(ns).containsKey(iter1.getName())); + + 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")); + assertEquals(Set.of(iter1.getName()), nops.listIterators(ns).keySet()); + for (var scope : IteratorScope.values()) { + assertEquals(iter1, nops.getIteratorSetting(ns, iter1.getName(), scope)); + } + } else { + assertTrue(logsContain(List.of("iterator priority conflict"), iterPrioConflictExec)); + assertTrue(logsContain(List.of("iterator name conflict"), iterNameConflictExec)); + } + } + + @Test + public void testDefaultIterConflict() throws Throwable { + final String[] names = getUniqueNames(29); + + // testing Scanner.addScanIterator + String defaultsTable1 = names[0]; + tops.create(defaultsTable1); + String noDefaultsTable1 = names[1]; + tops.create(noDefaultsTable1, new NewTableConfiguration().withoutDefaults()); + try (var defaultsScanner1 = client.createScanner(defaultsTable1); + var noDefaultsScanner1 = client.createScanner(noDefaultsTable1); + var defaultsScanner2 = client.createScanner(defaultsTable1); + var noDefaultsScanner2 = client.createScanner(noDefaultsTable1)) { + testDefaultIterConflict(RuntimeException.class, () -> { + defaultsScanner1.addScanIterator(defaultIterPrioConflict); + defaultsScanner1.iterator().hasNext(); + }, () -> { + defaultsScanner2.addScanIterator(defaultIterNameConflict); + defaultsScanner2.iterator().hasNext(); + }, () -> { + noDefaultsScanner1.addScanIterator(defaultIterPrioConflict); + noDefaultsScanner1.iterator().hasNext(); + }, () -> { + noDefaultsScanner2.addScanIterator(defaultIterNameConflict); + noDefaultsScanner2.iterator().hasNext(); + }, false); + } + + // testing TableOperations.setProperty + String defaultsTable2 = names[2]; + tops.create(defaultsTable2); + String noDefaultsTable2 = names[3]; + tops.create(noDefaultsTable2, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> tops.setProperty(defaultsTable2, defaultIterPrioConflictKey, + defaultIterPrioConflictVal), + () -> tops.setProperty(defaultsTable2, defaultIterNameConflictKey, + defaultIterNameConflictVal), + () -> tops.setProperty(noDefaultsTable2, defaultIterPrioConflictKey, + defaultIterPrioConflictVal), + () -> tops.setProperty(noDefaultsTable2, defaultIterNameConflictKey, + defaultIterNameConflictVal), + false); + + // testing TableOperations.modifyProperties + String defaultsTable3 = names[4]; + tops.create(defaultsTable3); + String noDefaultsTable3 = names[5]; + tops.create(noDefaultsTable3, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> tops.modifyProperties(defaultsTable3, + props -> props.put(defaultIterPrioConflictKey, defaultIterPrioConflictVal)), + () -> tops.modifyProperties(defaultsTable3, + props -> props.put(defaultIterNameConflictKey, defaultIterNameConflictVal)), + () -> tops.modifyProperties(noDefaultsTable3, + props -> props.put(defaultIterPrioConflictKey, defaultIterPrioConflictVal)), + () -> tops.modifyProperties(noDefaultsTable3, + props -> props.put(defaultIterNameConflictKey, defaultIterNameConflictVal)), + false); + + // testing NewTableConfiguration.attachIterator + String defaultsTable4 = names[6]; + String noDefaultsTable4 = names[7]; + String noDefaultsTable5 = names[8]; + testDefaultIterConflict(IllegalArgumentException.class, + () -> tops.create(defaultsTable4, + new NewTableConfiguration().attachIterator(defaultIterPrioConflict)), + () -> tops.create(defaultsTable4, + new NewTableConfiguration().attachIterator(defaultIterNameConflict)), + () -> tops.create(noDefaultsTable4, + new NewTableConfiguration().attachIterator(defaultIterPrioConflict).withoutDefaults()), + () -> tops.create(noDefaultsTable5, + new NewTableConfiguration().attachIterator(defaultIterNameConflict).withoutDefaults()), + true); + + // testing TableOperations.attachIterator + String defaultsTable6 = names[9]; + tops.create(defaultsTable6); + String noDefaultsTable6 = names[10]; + tops.create(noDefaultsTable6, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> tops.attachIterator(defaultsTable6, defaultIterPrioConflict), + () -> tops.attachIterator(defaultsTable6, defaultIterNameConflict), + () -> tops.attachIterator(noDefaultsTable6, defaultIterPrioConflict), + () -> tops.attachIterator(noDefaultsTable6, defaultIterNameConflict), true); + + // testing NamespaceOperations.attachIterator + String ns7 = names[11]; + nops.create(ns7); + String defaultsTable7 = ns7 + "." + names[12]; + tops.create(defaultsTable7); + String ns8 = names[13]; + nops.create(ns8); + String noDefaultsTable8 = ns8 + "." + names[14]; + tops.create(noDefaultsTable8, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> nops.attachIterator(ns7, defaultIterPrioConflict), + () -> nops.attachIterator(ns7, defaultIterNameConflict), + () -> nops.attachIterator(ns8, defaultIterPrioConflict), + () -> nops.attachIterator(ns8, defaultIterNameConflict), false); + + // testing NamespaceOperations.setProperty + String ns9 = names[15]; + nops.create(ns9); + String defaultsTable9 = ns9 + "." + names[16]; + tops.create(defaultsTable9); + String ns10 = names[17]; + nops.create(ns10); + String noDefaultsTable10 = ns10 + "." + names[18]; + tops.create(noDefaultsTable10, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> nops.setProperty(ns9, defaultIterPrioConflictKey, defaultIterPrioConflictVal), + () -> nops.setProperty(ns9, defaultIterNameConflictKey, defaultIterNameConflictVal), + () -> nops.setProperty(ns10, defaultIterPrioConflictKey, defaultIterPrioConflictVal), + () -> nops.setProperty(ns10, defaultIterNameConflictKey, defaultIterNameConflictVal), + false); + + // testing NamespaceOperations.modifyProperties + String ns11 = names[19]; + nops.create(ns11); + String defaultsTable11 = ns11 + "." + names[20]; + tops.create(defaultsTable11); + String ns12 = names[21]; + nops.create(ns12); + String noDefaultsTable12 = ns12 + "." + names[22]; + tops.create(noDefaultsTable12, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> nops.modifyProperties(ns11, + props -> props.put(defaultIterPrioConflictKey, defaultIterPrioConflictVal)), + () -> nops.modifyProperties(ns11, + props -> props.put(defaultIterNameConflictKey, defaultIterNameConflictVal)), + () -> nops.modifyProperties(ns12, + props -> props.put(defaultIterPrioConflictKey, defaultIterPrioConflictVal)), + () -> nops.modifyProperties(ns12, + props -> props.put(defaultIterNameConflictKey, defaultIterNameConflictVal)), + false); + + // testing CloneConfiguration.Builder.setPropertiesToSet + String dst1 = names[23]; + String dst2 = names[24]; + String dst3 = names[25]; + String dst4 = names[26]; + String defaultsTable12 = names[27]; + tops.create(defaultsTable12); + String noDefaultsTable13 = names[28]; + tops.create(noDefaultsTable13, new NewTableConfiguration().withoutDefaults()); + testDefaultIterConflict(AccumuloException.class, + () -> tops + .clone(defaultsTable12, dst1, + CloneConfiguration.builder() + .setPropertiesToSet( + Map.of(defaultIterPrioConflictKey, defaultIterPrioConflictVal)) + .build()), + () -> tops + .clone(defaultsTable12, dst2, + CloneConfiguration.builder() + .setPropertiesToSet( + Map.of(defaultIterNameConflictKey, defaultIterNameConflictVal)) + .build()), + () -> tops + .clone(noDefaultsTable13, dst3, + CloneConfiguration.builder() + .setPropertiesToSet( + Map.of(defaultIterPrioConflictKey, defaultIterPrioConflictVal)) + .build()), + () -> tops.clone(noDefaultsTable13, dst4, + CloneConfiguration.builder() + .setPropertiesToSet(Map.of(defaultIterNameConflictKey, defaultIterNameConflictVal)) + .build()), + false); + } + + private void testDefaultIterConflict(Class exceptionClass, + Executable defaultsTableOp1, Executable defaultsTableOp2, Executable noDefaultsTableOp1, + Executable noDefaultsTableOp2, boolean shouldThrow) throws Throwable { + if (shouldThrow) { + var e = assertThrows(exceptionClass, defaultsTableOp1); + assertTrue(e.toString().contains("conflict with default table iterator") + || e.toString().contains("iterator priority conflict")); + e = assertThrows(exceptionClass, defaultsTableOp2); + assertTrue(e.toString().contains("conflict with default table iterator") + || e.toString().contains("iterator name conflict")); + } else { + assertTrue( + logsContain(List.of("conflict with default table iterator", "iterator priority conflict"), + defaultsTableOp1)); + assertTrue( + logsContain(List.of("conflict with default table iterator", "iterator name conflict"), + defaultsTableOp2)); + } + + noDefaultsTableOp1.execute(); // should NOT fail + noDefaultsTableOp2.execute(); // should NOT fail + } + + @Test + public void testSameIterNoConflict() throws Throwable { + // note about setProperty calls in this test. The default table iter has an option so the + // property representation of this iter is a property for the iter and a property for the + // option (2 properties). Obviously we cannot call setProperty with both of these properties, + // but calling setProperty for one of these properties should be fine as it has no effect on + // the config. + final String[] names = getUniqueNames(16); + + // testing Scanner.addScanIterator + final String table1 = names[0]; + tops.create(table1, new NewTableConfiguration().attachIterator(iter1)); + try (var scanner1 = client.createScanner(table1); var scanner2 = client.createScanner(table1)) { + testSameIterNoConflict(() -> { + scanner1.addScanIterator(iter1); + scanner1.iterator().hasNext(); + }, () -> { + scanner2.addScanIterator(defaultTableIter); + scanner2.iterator().hasNext(); + }); + } + + // testing TableOperations.setProperty + final String table2 = names[1]; + tops.create(table2, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict(() -> tops.setProperty(table2, iter1Key, iter1Val), + () -> tops.setProperty(table2, defaultIterKey, defaultIterVal)); + + // testing TableOperations.modifyProperties + final String table3 = names[2]; + tops.create(table3, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict( + () -> tops.modifyProperties(table3, props -> props.put(iter1Key, iter1Val)), + () -> tops.modifyProperties(table3, props -> { + props.put(defaultIterKey, defaultIterVal); + props.put(defaultIterOptKey, defaultIterOptVal); + })); + + // testing NewTableConfiguration.attachIterator + final String ns1 = names[3]; + final String table4 = ns1 + "." + names[4]; + final String table5 = names[5]; + nops.create(ns1); + nops.attachIterator(ns1, iter1); + Wait.waitFor(() -> nops.listIterators(ns1).containsKey(iter1.getName())); + testSameIterNoConflict( + () -> tops.create(table4, new NewTableConfiguration().attachIterator(iter1)), + () -> tops.create(table5, new NewTableConfiguration().attachIterator(defaultTableIter))); + + // testing TableOperations.attachIterator + final String table6 = names[6]; + tops.create(table6, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict(() -> tops.attachIterator(table6, iter1), + () -> tops.attachIterator(table6, defaultTableIter)); + + // testing NamespaceOperations.attachIterator + final String ns2 = names[7]; + final String table7 = ns2 + "." + names[8]; + nops.create(ns2); + tops.create(table7, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict(() -> nops.attachIterator(ns2, iter1), + () -> nops.attachIterator(ns2, defaultTableIter)); + + // testing NamespaceOperations.setProperty + final String ns3 = names[9]; + final String table8 = ns3 + "." + names[10]; + nops.create(ns3); + tops.create(table8, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict(() -> nops.setProperty(ns3, iter1Key, iter1Val), + () -> nops.setProperty(ns3, defaultIterKey, defaultIterVal)); + + // testing NamespaceOperations.modifyProperties + final String ns4 = names[11]; + final String table9 = ns4 + "." + names[12]; + nops.create(ns4); + tops.create(table9, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict(() -> nops.modifyProperties(ns4, props -> props.put(iter1Key, iter1Val)), + () -> nops.modifyProperties(ns4, props -> { + props.put(defaultIterKey, defaultIterVal); + props.put(defaultIterOptKey, defaultIterOptVal); + })); + + // testing CloneConfiguration.Builder.setPropertiesToSet + final String src = names[13]; + final String dst1 = names[14]; + final String dst2 = names[15]; + tops.create(src, new NewTableConfiguration().attachIterator(iter1)); + testSameIterNoConflict( + () -> tops.clone(src, dst1, + CloneConfiguration.builder().setPropertiesToSet(Map.of(iter1Key, iter1Val)).build()), + () -> tops.clone(src, dst2, + CloneConfiguration.builder() + .setPropertiesToSet( + Map.of(defaultIterKey, defaultIterVal, defaultIterOptKey, defaultIterOptVal)) + .build())); + } + + private void testSameIterNoConflict(Executable addIter1Executable, + Executable addDefaultIterExecutable) throws Throwable { + // should be able to add same exact iterator + addIter1Executable.execute(); + addDefaultIterExecutable.execute(); + } + + private static boolean logsContain(List expectedStrs, Executable exec) throws Throwable { + var timeBeforeExec = LocalDateTime.now(); + var timeBeforeExecMillis = System.currentTimeMillis(); + exec.execute(); + + // check the logs from other processes for a log that occurred after the execution and + // contains one of the expected strings + List warnLogsAfterExec = warnLogsAfter(timeBeforeExec); + for (var warnLog : warnLogsAfterExec) { + if (expectedStrs.stream().anyMatch(warnLog::contains)) { + return true; + } + } + + // check the logs from the test process (this process) for a log that occurred after the + // execution and contains one of the expected strings + return appender.events().stream() + .anyMatch(logEvent -> logEvent.getTimeMillis() > timeBeforeExecMillis && expectedStrs + .stream().anyMatch(logEvent.getMessage().getFormattedMessage()::contains)); + } + + private static String getDatePattern() { + String datePattern = null; + for (var appender : loggerConfig.getAppenders().values()) { + if (appender.getLayout() instanceof PatternLayout) { + PatternLayout layout = (PatternLayout) appender.getLayout(); + String pattern = layout.getConversionPattern(); + if (pattern.contains("%d{ISO8601}")) { + datePattern = "yyyy-MM-dd'T'HH:mm:ss,SSS"; + break; + } + } + } + assertNotNull(datePattern, + "Format of dates in log4j config has changed. This test needs to be updated"); + return datePattern; + } + + private static List warnLogsAfter(LocalDateTime timeBeforeExec) throws Exception { + var filesIter = getCluster().getFileSystem() + .listFiles(new Path(getCluster().getConfig().getLogDir().toURI()), false); + List files = new ArrayList<>(); + List lines = new ArrayList<>(); + + // get all the WARN logs in the Manager and TabletServer logs that happened after the given + // time. We only care about the Manager and TabletServer as these are the only servers that + // will check for iterator conflicts + while (filesIter.hasNext()) { + var file = filesIter.next(); + if (file.getPath().getName().matches("(Manager_|TabletServer_).+\\.out")) { + files.add(file.getPath()); + } + } + for (var path : files) { + try (var in = getCluster().getFileSystem().open(path); + BufferedReader reader = new BufferedReader(new InputStreamReader(in, UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + if (line.contains("WARN")) { + var words = line.split(" "); + if (words.length >= 1 + && LocalDateTime.parse(words[0], dateTimeFormatter).isAfter(timeBeforeExec)) { + lines.add(line); + } + } + } + } + } + + return lines; + } +}