This is an automated email from the ASF dual-hosted git repository. mmiller pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new bd27df02c8 Create IteratorBuilder to clean up internals (#2600) bd27df02c8 is described below commit bd27df02c86bc224de2d39f0839bbbf8f6b46357 Author: Mike Miller <mmil...@apache.org> AuthorDate: Thu Apr 14 11:25:14 2022 -0400 Create IteratorBuilder to clean up internals (#2600) * Create builder classes for iterator stack creation * Replace IterLoad class with new IteratorBuilder and refactor classes that use it to determine whether or not there is a bug in iterator code --- .../core/client/ClientSideIteratorScanner.java | 16 +++--- .../core/client/admin/NewTableConfiguration.java | 4 +- .../accumulo/core/client/rfile/RFileScanner.java | 16 +++--- .../accumulo/core/clientImpl/OfflineIterator.java | 11 ++-- .../IteratorBuilder.java} | 60 +++++++++++----------- .../IteratorBuilderImpl.java} | 46 ++++++++++------- .../IteratorConfigUtil.java} | 47 +++++++++-------- .../IteratorConfigUtilTest.java} | 26 ++++++---- .../accumulo/server/compaction/FileCompactor.java | 4 +- .../accumulo/server/conf/TableConfiguration.java | 4 +- .../accumulo/tserver/ConditionCheckerContext.java | 17 +++--- .../accumulo/tserver/tablet/ScanDataSource.java | 15 +++--- .../shell/commands/CreateTableCommand.java | 4 +- .../test/performance/scan/CollectTabletStats.java | 11 ++-- 14 files changed, 151 insertions(+), 130 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java b/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java index b56d47583c..d561ce3918 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java +++ b/core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java @@ -33,8 +33,6 @@ import java.util.TreeSet; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.client.sample.SamplerConfiguration; import org.apache.accumulo.core.clientImpl.ScannerOptions; -import org.apache.accumulo.core.conf.IterConfigUtil; -import org.apache.accumulo.core.conf.IterLoad; import org.apache.accumulo.core.data.ArrayByteSequence; import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Column; @@ -46,6 +44,8 @@ import org.apache.accumulo.core.iterators.IteratorAdapter; import org.apache.accumulo.core.iterators.IteratorEnvironment; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorBuilder; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.security.Authorizations; import org.apache.hadoop.io.Text; @@ -250,11 +250,13 @@ public class ClientSideIteratorScanner extends ScannerOptions implements Scanner SortedKeyValueIterator<Key,Value> skvi; try { - IteratorEnvironment env = new ClientSideIteratorEnvironment(getSamplerConfiguration() != null, - getIteratorSamplerConfigurationInternal()); - IterLoad iterLoad = new IterLoad().iters(tm.values()).iterOpts(serverSideIteratorOptions) - .iterEnv(env).useAccumuloClassLoader(false); - skvi = IterConfigUtil.loadIterators(smi, iterLoad); + IteratorEnvironment iterEnv = new ClientSideIteratorEnvironment( + getSamplerConfiguration() != null, getIteratorSamplerConfigurationInternal()); + + IteratorBuilder ib = + IteratorBuilder.builder(tm.values()).opts(serverSideIteratorOptions).env(iterEnv).build(); + + skvi = IteratorConfigUtil.loadIterators(smi, ib); } catch (IOException e) { throw new RuntimeException(e); } 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 c1cb21b0c8..364a6ff122 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 @@ -38,10 +38,10 @@ import org.apache.accumulo.core.client.sample.SamplerConfiguration; import org.apache.accumulo.core.client.summary.Summarizer; import org.apache.accumulo.core.client.summary.SummarizerConfiguration; import org.apache.accumulo.core.clientImpl.TableOperationsHelper; -import org.apache.accumulo.core.conf.IterConfigUtil; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.user.VersioningIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.sample.impl.SamplerConfigurationImpl; import org.apache.accumulo.core.summary.SummarizerConfigurationUtil; import org.apache.accumulo.core.util.LocalityGroupUtil; @@ -170,7 +170,7 @@ public class NewTableConfiguration { Map<String,String> propertyMap = new HashMap<>(); if (limitVersion) - propertyMap.putAll(IterConfigUtil.generateInitialTableProperties(limitVersion)); + propertyMap.putAll(IteratorConfigUtil.generateInitialTableProperties(limitVersion)); propertyMap.putAll(summarizerProps); propertyMap.putAll(samplerProps); diff --git a/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java b/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java index bb4356d2d4..b181e36261 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java +++ b/core/src/main/java/org/apache/accumulo/core/client/rfile/RFileScanner.java @@ -38,8 +38,6 @@ import org.apache.accumulo.core.clientImpl.ScannerOptions; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.ConfigurationCopy; import org.apache.accumulo.core.conf.DefaultConfiguration; -import org.apache.accumulo.core.conf.IterConfigUtil; -import org.apache.accumulo.core.conf.IterLoad; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.crypto.CryptoServiceFactory; import org.apache.accumulo.core.crypto.CryptoServiceFactory.ClassloaderType; @@ -59,6 +57,8 @@ import org.apache.accumulo.core.iterators.IteratorAdapter; import org.apache.accumulo.core.iterators.IteratorEnvironment; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorBuilder; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.iteratorsImpl.system.MultiIterator; import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil; import org.apache.accumulo.core.sample.impl.SamplerConfigurationImpl; @@ -378,14 +378,14 @@ class RFileScanner extends ScannerOptions implements Scanner { try { if (opts.tableConfig != null && !opts.tableConfig.isEmpty()) { - IterLoad il = IterConfigUtil.loadIterConf(IteratorScope.scan, serverSideIteratorList, + var ibEnv = IteratorConfigUtil.loadIterConf(IteratorScope.scan, serverSideIteratorList, serverSideIteratorOptions, tableConf); - iterator = IterConfigUtil.loadIterators(iterator, - il.iterEnv(new IterEnv()).useAccumuloClassLoader(true)); + var iteratorBuilder = ibEnv.env(new IterEnv()).build(); + iterator = IteratorConfigUtil.loadIterators(iterator, iteratorBuilder); } else { - iterator = IterConfigUtil.loadIterators(iterator, - new IterLoad().iters(serverSideIteratorList).iterOpts(serverSideIteratorOptions) - .iterEnv(new IterEnv()).useAccumuloClassLoader(false)); + var iteratorBuilder = IteratorBuilder.builder(serverSideIteratorList) + .opts(serverSideIteratorOptions).env(new IterEnv()).build(); + iterator = IteratorConfigUtil.loadIterators(iterator, iteratorBuilder); } } catch (IOException e) { throw new RuntimeException(e); diff --git a/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java b/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java index fed8130e67..e28612c956 100644 --- a/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java +++ b/core/src/main/java/org/apache/accumulo/core/clientImpl/OfflineIterator.java @@ -38,8 +38,6 @@ import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.client.sample.SamplerConfiguration; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.ConfigurationCopy; -import org.apache.accumulo.core.conf.IterConfigUtil; -import org.apache.accumulo.core.conf.IterLoad; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.crypto.CryptoServiceFactory; import org.apache.accumulo.core.data.Key; @@ -54,6 +52,7 @@ import org.apache.accumulo.core.file.FileSKVIterator; import org.apache.accumulo.core.iterators.IteratorEnvironment; 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.iteratorsImpl.system.MultiIterator; import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil; import org.apache.accumulo.core.manager.state.tables.TableState; @@ -326,11 +325,11 @@ class OfflineIterator implements Iterator<Entry<Key,Value>> { Value> visFilter = SystemIteratorUtil.setupSystemScanIterators(multiIter, new HashSet<>(options.fetchedColumns), authorizations, defaultSecurityLabel, acuTableConf); - IterLoad iterLoad = IterConfigUtil.loadIterConf(IteratorScope.scan, + var iteratorBuilderEnv = IteratorConfigUtil.loadIterConf(IteratorScope.scan, options.serverSideIteratorList, options.serverSideIteratorOptions, acuTableConf); - - return iterEnv.getTopLevelIterator(IterConfigUtil.loadIterators(visFilter, - iterLoad.iterEnv(iterEnv).useAccumuloClassLoader(false))); + var iteratorBuilder = iteratorBuilderEnv.env(iterEnv).build(); + return iterEnv + .getTopLevelIterator(IteratorConfigUtil.loadIterators(visFilter, iteratorBuilder)); } @Override diff --git a/core/src/main/java/org/apache/accumulo/core/conf/IterLoad.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorBuilder.java similarity index 52% copy from core/src/main/java/org/apache/accumulo/core/conf/IterLoad.java copy to core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorBuilder.java index f69a6dc19c..d18388118e 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/IterLoad.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorBuilder.java @@ -16,53 +16,55 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.accumulo.core.conf; +package org.apache.accumulo.core.iteratorsImpl; import java.util.Collection; import java.util.Map; -import org.apache.accumulo.core.data.Key; -import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.dataImpl.thrift.IterInfo; import org.apache.accumulo.core.iterators.IteratorEnvironment; -import org.apache.accumulo.core.iterators.SortedKeyValueIterator; - -public class IterLoad { +/** + * Builder class for setting up the iterator stack. + */ +public class IteratorBuilder { Collection<IterInfo> iters; Map<String,Map<String,String>> iterOpts; IteratorEnvironment iteratorEnvironment; boolean useAccumuloClassLoader; - String context; - Map<String,Class<SortedKeyValueIterator<Key,Value>>> classCache; + String context = null; + boolean useClassCache = false; - public IterLoad iters(Collection<IterInfo> iters) { - this.iters = iters; - return this; - } + IteratorBuilder() {} - public IterLoad iterOpts(Map<String,Map<String,String>> iterOpts) { - this.iterOpts = iterOpts; - return this; + /** + * Start building the iterator builder. + */ + public static IteratorBuilderImpl builder(Collection<IterInfo> iters) { + return new IteratorBuilderImpl(iters); } - public IterLoad iterEnv(IteratorEnvironment iteratorEnvironment) { - this.iteratorEnvironment = iteratorEnvironment; - return this; + public interface IteratorBuilderEnv { + /** + * Set the iteratorEnvironment. + */ + IteratorBuilderOptions env(IteratorEnvironment iteratorEnvironment); } - public IterLoad useAccumuloClassLoader(boolean useAccumuloClassLoader) { - this.useAccumuloClassLoader = useAccumuloClassLoader; - return this; - } + public interface IteratorBuilderOptions extends IteratorBuilderEnv { + /** + * Option to iterator classes when loading, defaults to false. + */ + IteratorBuilderOptions useClassCache(boolean useClassCache); - public IterLoad context(String context) { - this.context = context; - return this; - } + /** + * Call to use the class loader. The String context param is optional and can be null. + */ + IteratorBuilderOptions useClassLoader(String context); - public IterLoad classCache(Map<String,Class<SortedKeyValueIterator<Key,Value>>> classCache) { - this.classCache = classCache; - return this; + /** + * Finish building and return the completed IteratorBuilder. + */ + IteratorBuilder build(); } } diff --git a/core/src/main/java/org/apache/accumulo/core/conf/IterLoad.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorBuilderImpl.java similarity index 53% rename from core/src/main/java/org/apache/accumulo/core/conf/IterLoad.java rename to core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorBuilderImpl.java index f69a6dc19c..ebc78f4b75 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/IterLoad.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorBuilderImpl.java @@ -16,53 +16,61 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.accumulo.core.conf; +package org.apache.accumulo.core.iteratorsImpl; import java.util.Collection; import java.util.Map; -import org.apache.accumulo.core.data.Key; -import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.dataImpl.thrift.IterInfo; import org.apache.accumulo.core.iterators.IteratorEnvironment; -import org.apache.accumulo.core.iterators.SortedKeyValueIterator; -public class IterLoad { +public class IteratorBuilderImpl + implements IteratorBuilder.IteratorBuilderEnv, IteratorBuilder.IteratorBuilderOptions { Collection<IterInfo> iters; Map<String,Map<String,String>> iterOpts; IteratorEnvironment iteratorEnvironment; - boolean useAccumuloClassLoader; - String context; - Map<String,Class<SortedKeyValueIterator<Key,Value>>> classCache; + boolean useAccumuloClassLoader = false; + String context = null; + boolean useClassCache = false; - public IterLoad iters(Collection<IterInfo> iters) { + public IteratorBuilderImpl(Collection<IterInfo> iters) { this.iters = iters; - return this; } - public IterLoad iterOpts(Map<String,Map<String,String>> iterOpts) { + public IteratorBuilder.IteratorBuilderEnv opts(Map<String,Map<String,String>> iterOpts) { this.iterOpts = iterOpts; return this; } - public IterLoad iterEnv(IteratorEnvironment iteratorEnvironment) { + @Override + public IteratorBuilder.IteratorBuilderOptions env(IteratorEnvironment iteratorEnvironment) { this.iteratorEnvironment = iteratorEnvironment; return this; } - public IterLoad useAccumuloClassLoader(boolean useAccumuloClassLoader) { - this.useAccumuloClassLoader = useAccumuloClassLoader; + @Override + public IteratorBuilder.IteratorBuilderOptions useClassLoader(String context) { + this.useAccumuloClassLoader = true; + this.context = context; return this; } - public IterLoad context(String context) { - this.context = context; + @Override + public IteratorBuilder.IteratorBuilderOptions useClassCache(boolean useClassCache) { + this.useClassCache = useClassCache; return this; } - public IterLoad classCache(Map<String,Class<SortedKeyValueIterator<Key,Value>>> classCache) { - this.classCache = classCache; - return this; + @Override + public IteratorBuilder build() { + var ib = new IteratorBuilder(); + ib.iters = this.iters; + ib.iterOpts = this.iterOpts; + ib.iteratorEnvironment = this.iteratorEnvironment; + ib.useAccumuloClassLoader = this.useAccumuloClassLoader; + ib.context = this.context; + ib.useClassCache = this.useClassCache; + return ib; } } diff --git a/core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java similarity index 83% rename from core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java rename to core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java index 47347397d0..9939c5d966 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/IterConfigUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtil.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.accumulo.core.conf; +package org.apache.accumulo.core.iteratorsImpl; import static java.util.Objects.requireNonNull; @@ -33,6 +33,8 @@ import java.util.TreeMap; import org.apache.accumulo.core.classloader.ClassLoaderUtil; import org.apache.accumulo.core.client.IteratorSetting; +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.data.constraints.DefaultKeySizeConstraint; @@ -48,8 +50,8 @@ import org.slf4j.LoggerFactory; * Utility class for configuring iterators. These methods were moved from IteratorUtil so that it * could be treated as API. */ -public class IterConfigUtil { - private static final Logger log = LoggerFactory.getLogger(IterConfigUtil.class); +public class IteratorConfigUtil { + private static final Logger log = LoggerFactory.getLogger(IteratorConfigUtil.class); public static final Comparator<IterInfo> ITER_INFO_COMPARATOR = Comparator.comparingInt(IterInfo::getPriority); @@ -160,12 +162,12 @@ public class IterConfigUtil { }); } - public static IterLoad loadIterConf(IteratorScope scope, List<IterInfo> iters, - Map<String,Map<String,String>> iterOpts, AccumuloConfiguration conf) { + public static IteratorBuilder.IteratorBuilderEnv loadIterConf(IteratorScope scope, + List<IterInfo> iters, Map<String,Map<String,String>> iterOpts, AccumuloConfiguration conf) { Map<String,Map<String,String>> allOptions = new HashMap<>(); List<IterInfo> iterators = parseIterConf(scope, iters, allOptions, conf); mergeOptions(iterOpts, allOptions); - return new IterLoad().iters(iterators).iterOpts(allOptions); + return IteratorBuilder.builder(iterators).opts(allOptions); } /** @@ -183,42 +185,45 @@ public class IterConfigUtil { ssio.put(is.getName(), is.getOptions()); } - IterLoad il = loadIterConf(scope, ssiList, ssio, conf); - il = il.iterEnv(env).useAccumuloClassLoader(true).context(ClassLoaderUtil.tableContext(conf)); - return loadIterators(source, il); + var ibEnv = loadIterConf(scope, ssiList, ssio, conf); + var iterBuilder = ibEnv.env(env).useClassLoader(ClassLoaderUtil.tableContext(conf)).build(); + return loadIterators(source, iterBuilder); } /** - * Load a stack of iterators provided in the IterLoad, starting with source. + * Load a stack of iterators provided in the iterator builder, starting with source. */ - public static SortedKeyValueIterator<Key,Value> loadIterators( - SortedKeyValueIterator<Key,Value> source, IterLoad iterLoad) throws IOException { + public static SortedKeyValueIterator<Key,Value> + loadIterators(SortedKeyValueIterator<Key,Value> source, IteratorBuilder iteratorBuilder) + throws IOException { SortedKeyValueIterator<Key,Value> prev = source; + final boolean useClassLoader = iteratorBuilder.useAccumuloClassLoader; + Map<String,Class<SortedKeyValueIterator<Key,Value>>> classCache = new HashMap<>(); try { - for (IterInfo iterInfo : iterLoad.iters) { + for (IterInfo iterInfo : iteratorBuilder.iters) { Class<SortedKeyValueIterator<Key,Value>> clazz = null; log.trace("Attempting to load iterator class {}", iterInfo.className); - if (iterLoad.classCache != null) { - clazz = iterLoad.classCache.get(iterInfo.className); + if (iteratorBuilder.useClassCache) { + clazz = classCache.get(iterInfo.className); if (clazz == null) { - clazz = loadClass(iterLoad.useAccumuloClassLoader, iterLoad.context, iterInfo); - iterLoad.classCache.put(iterInfo.className, clazz); + clazz = loadClass(useClassLoader, iteratorBuilder.context, iterInfo); + classCache.put(iterInfo.className, clazz); } } else { - clazz = loadClass(iterLoad.useAccumuloClassLoader, iterLoad.context, iterInfo); + clazz = loadClass(useClassLoader, iteratorBuilder.context, iterInfo); } SortedKeyValueIterator<Key,Value> skvi = clazz.getDeclaredConstructor().newInstance(); - Map<String,String> options = iterLoad.iterOpts.get(iterInfo.iterName); + Map<String,String> options = iteratorBuilder.iterOpts.get(iterInfo.iterName); if (options == null) options = Collections.emptyMap(); - skvi.init(prev, options, iterLoad.iteratorEnvironment); + skvi.init(prev, options, iteratorBuilder.iteratorEnvironment); prev = skvi; } } catch (ReflectiveOperationException e) { @@ -230,7 +235,7 @@ public class IterConfigUtil { @SuppressWarnings("unchecked") private static Class<SortedKeyValueIterator<Key,Value>> loadClass(boolean useAccumuloClassLoader, - String context, IterInfo iterInfo) throws ClassNotFoundException, IOException { + String context, IterInfo iterInfo) throws ClassNotFoundException { Class<SortedKeyValueIterator<Key,Value>> clazz = null; if (useAccumuloClassLoader) { clazz = (Class<SortedKeyValueIterator<Key,Value>>) ClassLoaderUtil.loadClass(context, diff --git a/core/src/test/java/org/apache/accumulo/core/conf/IterConfigUtilTest.java b/core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java similarity index 92% rename from core/src/test/java/org/apache/accumulo/core/conf/IterConfigUtilTest.java rename to core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java index 5faa3fd725..b6ade6e233 100644 --- a/core/src/test/java/org/apache/accumulo/core/conf/IterConfigUtilTest.java +++ b/core/src/test/java/org/apache/accumulo/core/iteratorsImpl/IteratorConfigUtilTest.java @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.accumulo.core.conf; +package org.apache.accumulo.core.iteratorsImpl; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -31,6 +31,9 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import org.apache.accumulo.core.conf.AccumuloConfiguration; +import org.apache.accumulo.core.conf.ConfigurationCopy; +import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Range; @@ -48,9 +51,9 @@ import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class IterConfigUtilTest { +public class IteratorConfigUtilTest { - private static final Logger log = LoggerFactory.getLogger(IterConfigUtilTest.class); + private static final Logger log = LoggerFactory.getLogger(IteratorConfigUtilTest.class); private static final Collection<ByteSequence> EMPTY_COL_FAMS = new ArrayList<>(); private static final List<IterInfo> EMPTY_ITERS = Collections.emptyList(); @@ -135,9 +138,10 @@ public class IterConfigUtilTest { private SortedKeyValueIterator<Key,Value> createIter(IteratorScope scope, SortedMapIterator source, AccumuloConfiguration conf) throws IOException { - IterLoad iterLoad = IterConfigUtil.loadIterConf(scope, EMPTY_ITERS, new HashMap<>(), conf); - iterLoad = iterLoad.iterEnv(new DefaultIteratorEnvironment(conf)).useAccumuloClassLoader(true); - return IterConfigUtil.loadIterators(source, iterLoad); + var ibEnv = IteratorConfigUtil.loadIterConf(scope, EMPTY_ITERS, new HashMap<>(), conf); + var iteratorBuilder = + ibEnv.env(new DefaultIteratorEnvironment(conf)).useClassLoader(null).build(); + return IteratorConfigUtil.loadIterators(source, iteratorBuilder); } @Test @@ -324,7 +328,7 @@ public class IterConfigUtilTest { AccumuloConfiguration conf = new ConfigurationCopy(data); List<IterInfo> iterators = - IterConfigUtil.parseIterConf(IteratorScope.scan, EMPTY_ITERS, new HashMap<>(), conf); + IteratorConfigUtil.parseIterConf(IteratorScope.scan, EMPTY_ITERS, new HashMap<>(), conf); assertEquals(1, iterators.size()); IterInfo ii = iterators.get(0); @@ -348,7 +352,7 @@ public class IterConfigUtilTest { data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foo.bar", "50," + SummingCombiner.class.getName()); conf = new ConfigurationCopy(data); - iterators = IterConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); + iterators = IteratorConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); } catch (IllegalArgumentException ex) { log.debug("caught expected exception: " + ex.getMessage()); } @@ -362,7 +366,7 @@ public class IterConfigUtilTest { data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foo.bar.baz", "49," + SummingCombiner.class.getName()); conf = new ConfigurationCopy(data); - iterators = IterConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); + iterators = IteratorConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); } catch (IllegalArgumentException ex) { log.debug("caught expected exception: " + ex.getMessage()); } @@ -376,7 +380,7 @@ public class IterConfigUtilTest { "48," + SummingCombiner.class.getName()); data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foobar.opt", "fakevalue"); conf = new ConfigurationCopy(data); - iterators = IterConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); + iterators = IteratorConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); assertEquals(1, iterators.size()); IterInfo ii = iterators.get(0); assertEquals(new IterInfo(48, SummingCombiner.class.getName(), "foobar"), ii); @@ -393,7 +397,7 @@ public class IterConfigUtilTest { "47," + SummingCombiner.class.getName()); data.put(Property.TABLE_ITERATOR_SCAN_PREFIX + "foobaz.fake.opt", "fakevalue"); conf = new ConfigurationCopy(data); - iterators = IterConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); + iterators = IteratorConfigUtil.parseIterConf(IteratorScope.scan, iterators, options, conf); assertEquals(1, iterators.size()); IterInfo ii = iterators.get(0); assertEquals(new IterInfo(47, SummingCombiner.class.getName(), "foobaz"), ii); diff --git a/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java b/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java index b05215a56f..e2b66cd698 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java +++ b/server/base/src/main/java/org/apache/accumulo/server/compaction/FileCompactor.java @@ -35,7 +35,6 @@ import java.util.concurrent.atomic.AtomicLong; import org.apache.accumulo.core.client.IteratorSetting; import org.apache.accumulo.core.conf.AccumuloConfiguration; -import org.apache.accumulo.core.conf.IterConfigUtil; import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.TableId; @@ -46,6 +45,7 @@ import org.apache.accumulo.core.file.FileSKVIterator; import org.apache.accumulo.core.file.FileSKVWriter; 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.iteratorsImpl.system.ColumnFamilySkippingIterator; import org.apache.accumulo.core.iteratorsImpl.system.DeletingIterator; import org.apache.accumulo.core.iteratorsImpl.system.MultiIterator; @@ -358,7 +358,7 @@ public class FileCompactor implements Callable<CompactionStats> { SystemIteratorEnvironment iterEnv = env.createIteratorEnv(context, acuTableConf, getExtent().tableId()); - SortedKeyValueIterator<Key,Value> itr = iterEnv.getTopLevelIterator(IterConfigUtil + SortedKeyValueIterator<Key,Value> itr = iterEnv.getTopLevelIterator(IteratorConfigUtil .convertItersAndLoad(env.getIteratorScope(), cfsi, acuTableConf, iterators, iterEnv)); itr.seek(extent.toDataRange(), columnFamilies, inclusive); 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 030e8dc70d..2819c70181 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 @@ -33,11 +33,11 @@ import java.util.stream.Collectors; import org.apache.accumulo.core.Constants; import org.apache.accumulo.core.classloader.ClassLoaderUtil; import org.apache.accumulo.core.conf.AccumuloConfiguration; -import org.apache.accumulo.core.conf.IterConfigUtil; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.thrift.IterInfo; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.spi.common.ServiceEnvironment; import org.apache.accumulo.core.spi.compaction.CompactionDispatcher; import org.apache.accumulo.core.spi.scan.ScanDispatcher; @@ -74,7 +74,7 @@ public class TableConfiguration extends AccumuloConfiguration { iteratorConfig.put(scope, newDeriver(conf -> { Map<String,Map<String,String>> allOpts = new HashMap<>(); List<IterInfo> iters = - IterConfigUtil.parseIterConf(scope, Collections.emptyList(), allOpts, conf); + IteratorConfigUtil.parseIterConf(scope, Collections.emptyList(), allOpts, conf); return new ParsedIteratorConfig(iters, allOpts, ClassLoaderUtil.tableContext(conf)); })); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java index 658bb30ee3..cae2a77628 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionCheckerContext.java @@ -30,8 +30,6 @@ import java.util.Map; import org.apache.accumulo.core.clientImpl.CompressedIterators; import org.apache.accumulo.core.clientImpl.CompressedIterators.IterConfig; -import org.apache.accumulo.core.conf.IterConfigUtil; -import org.apache.accumulo.core.conf.IterLoad; import org.apache.accumulo.core.data.ArrayByteSequence; import org.apache.accumulo.core.data.ByteSequence; import org.apache.accumulo.core.data.Key; @@ -43,6 +41,8 @@ import org.apache.accumulo.core.dataImpl.thrift.TCMStatus; import org.apache.accumulo.core.dataImpl.thrift.TCondition; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorBuilder; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.server.ServerContext; import org.apache.accumulo.server.conf.TableConfiguration; import org.apache.accumulo.server.conf.TableConfiguration.ParsedIteratorConfig; @@ -57,7 +57,6 @@ public class ConditionCheckerContext { private Map<String,Map<String,String>> tableIterOpts; private TabletIteratorEnvironment tie; private String context; - private Map<String,Class<SortedKeyValueIterator<Key,Value>>> classCache; private static class MergedIterConfig { List<IterInfo> mergedIters; @@ -81,8 +80,6 @@ public class ConditionCheckerContext { tableIterOpts = pic.getOpts(); this.context = pic.getServiceEnv(); - classCache = new HashMap<>(); - tie = new TabletIteratorEnvironment(context, IteratorScope.scan, tableConf, tableConf.getTableId()); } @@ -99,17 +96,17 @@ public class ConditionCheckerContext { Map<String,Map<String,String>> mergedItersOpts = new HashMap<>(tableIterOpts.size() + ic.ssio.size()); - IterConfigUtil.mergeIteratorConfig(mergedIters, mergedItersOpts, tableIters, tableIterOpts, - ic.ssiList, ic.ssio); + IteratorConfigUtil.mergeIteratorConfig(mergedIters, mergedItersOpts, tableIters, + tableIterOpts, ic.ssiList, ic.ssio); mic = new MergedIterConfig(mergedIters, mergedItersOpts); mergedIterCache.put(key, mic); } - IterLoad iterLoad = new IterLoad().iters(mic.mergedIters).iterOpts(mic.mergedItersOpts) - .iterEnv(tie).useAccumuloClassLoader(true).context(context).classCache(classCache); - return IterConfigUtil.loadIterators(systemIter, iterLoad); + var iteratorBuilder = IteratorBuilder.builder(mic.mergedIters).opts(mic.mergedItersOpts) + .env(tie).useClassLoader(context).useClassCache(true).build(); + return IteratorConfigUtil.loadIterators(systemIter, iteratorBuilder); } boolean checkConditions(SortedKeyValueIterator<Key,Value> systemIter, 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 9ef99e8c4d..a4de0a156f 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 @@ -26,14 +26,14 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.accumulo.core.conf.IterConfigUtil; -import org.apache.accumulo.core.conf.IterLoad; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.dataImpl.thrift.IterInfo; import org.apache.accumulo.core.iterators.IteratorEnvironment; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iteratorsImpl.IteratorBuilder; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.core.iteratorsImpl.system.InterruptibleIterator; import org.apache.accumulo.core.iteratorsImpl.system.IterationInterruptedException; import org.apache.accumulo.core.iteratorsImpl.system.MultiIterator; @@ -204,8 +204,8 @@ class ScanDataSource implements DataSource { // iterator options. iterOpts = new HashMap<>(pic.getOpts().size() + scanParams.getSsio().size()); iterInfos = new ArrayList<>(pic.getIterInfo().size() + scanParams.getSsiList().size()); - IterConfigUtil.mergeIteratorConfig(iterInfos, iterOpts, pic.getIterInfo(), pic.getOpts(), - scanParams.getSsiList(), scanParams.getSsio()); + IteratorConfigUtil.mergeIteratorConfig(iterInfos, iterOpts, pic.getIterInfo(), + pic.getOpts(), scanParams.getSsiList(), scanParams.getSsio()); } String context; @@ -223,9 +223,10 @@ class ScanDataSource implements DataSource { } } - IterLoad il = new IterLoad().iters(iterInfos).iterOpts(iterOpts).iterEnv(iterEnv) - .useAccumuloClassLoader(true).context(context); - return iterEnv.getTopLevelIterator(IterConfigUtil.loadIterators(visFilter, il)); + var iteratorBuilder = IteratorBuilder.builder(iterInfos).opts(iterOpts).env(iterEnv) + .useClassLoader(context).build(); + return iterEnv + .getTopLevelIterator(IteratorConfigUtil.loadIterators(visFilter, iteratorBuilder)); } else { return visFilter; } diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java index b6d2ae1c20..39f4db74a4 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java +++ b/shell/src/main/java/org/apache/accumulo/shell/commands/CreateTableCommand.java @@ -38,10 +38,10 @@ import org.apache.accumulo.core.client.TableExistsException; import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.client.admin.NewTableConfiguration; import org.apache.accumulo.core.client.admin.TimeType; -import org.apache.accumulo.core.conf.IterConfigUtil; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.constraints.VisibilityConstraint; import org.apache.accumulo.core.iterators.IteratorUtil.IteratorScope; +import org.apache.accumulo.core.iteratorsImpl.IteratorConfigUtil; import org.apache.accumulo.shell.Shell; import org.apache.accumulo.shell.Shell.Command; import org.apache.accumulo.shell.ShellUtil; @@ -134,7 +134,7 @@ public class CreateTableCommand extends Command { shellState.setTableName(tableName); // switch shell to new table context if (cl.hasOption(createTableNoDefaultIters.getOpt())) { - for (String key : IterConfigUtil.generateInitialTableProperties(true).keySet()) { + for (String key : IteratorConfigUtil.generateInitialTableProperties(true).keySet()) { shellState.getAccumuloClient().tableOperations().removeProperty(tableName, key); } } diff --git a/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java b/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java index 1358ab7446..8d54bb50f1 100644 --- a/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java +++ b/test/src/main/java/org/apache/accumulo/test/performance/scan/CollectTabletStats.java @@ -41,8 +41,6 @@ import org.apache.accumulo.core.client.AccumuloClient; import org.apache.accumulo.core.client.Scanner; import org.apache.accumulo.core.clientImpl.ClientContext; import org.apache.accumulo.core.conf.AccumuloConfiguration; -import org.apache.accumulo.core.conf.IterConfigUtil; -import org.apache.accumulo.core.conf.IterLoad; import org.apache.accumulo.core.crypto.CryptoServiceFactory; import org.apache.accumulo.core.data.ArrayByteSequence; import org.apache.accumulo.core.data.ByteSequence; @@ -55,8 +53,10 @@ import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.core.dataImpl.thrift.IterInfo; import org.apache.accumulo.core.file.FileOperations; import org.apache.accumulo.core.file.FileSKVIterator; +import org.apache.accumulo.core.iterators.IteratorEnvironment; 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.iteratorsImpl.system.ColumnFamilySkippingIterator; import org.apache.accumulo.core.iteratorsImpl.system.ColumnQualifierFilter; import org.apache.accumulo.core.iteratorsImpl.system.DeletingIterator; @@ -103,6 +103,8 @@ public class CollectTabletStats { String columns; } + static class TestEnvironment implements IteratorEnvironment {} + public static void main(String[] args) throws Exception { final CollectOptions opts = new CollectOptions(); @@ -445,8 +447,9 @@ public class CollectTabletStats { VisibilityFilter.wrap(colFilter, authorizations, defaultLabels); if (useTableIterators) { - IterLoad il = IterConfigUtil.loadIterConf(IteratorScope.scan, ssiList, ssio, conf); - return IterConfigUtil.loadIterators(visFilter, il.useAccumuloClassLoader(true)); + var ibEnv = IteratorConfigUtil.loadIterConf(IteratorScope.scan, ssiList, ssio, conf); + var iteratorBuilder = ibEnv.env(new TestEnvironment()).useClassLoader("test").build(); + return IteratorConfigUtil.loadIterators(visFilter, iteratorBuilder); } return visFilter; }