Repository: accumulo Updated Branches: refs/heads/master 621e0f26e -> 930592f5e
ACCUMULO-4128: Use setiter instead of setscaniter as it does not exist in 2.0. Added more output validation in the test to catch this in the future. Project: http://git-wip-us.apache.org/repos/asf/accumulo/repo Commit: http://git-wip-us.apache.org/repos/asf/accumulo/commit/2c883889 Tree: http://git-wip-us.apache.org/repos/asf/accumulo/tree/2c883889 Diff: http://git-wip-us.apache.org/repos/asf/accumulo/diff/2c883889 Branch: refs/heads/master Commit: 2c8838899531080adfdb33ba75f25f4cdd643963 Parents: 7ad5dd6 Author: Dave Marion <dlmar...@apache.org> Authored: Tue Jul 19 09:43:12 2016 -0400 Committer: Dave Marion <dlmar...@apache.org> Committed: Tue Jul 19 09:43:12 2016 -0400 ---------------------------------------------------------------------- .../core/client/impl/ThriftScanner.java | 2 +- .../accumulo/core/iterators/IteratorUtil.java | 9 +++++-- .../impl/MiniAccumuloClusterImpl.java | 2 ++ .../accumulo/tserver/tablet/ScanDataSource.java | 11 +++++++++ .../accumulo/tserver/tablet/ScanOptions.java | 16 +++++++++++++ .../start/classloader/vfs/ContextManager.java | 9 +++++-- .../org/apache/accumulo/test/ShellServerIT.java | 25 +++++++++++++------- 7 files changed, 60 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java b/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java index 5f3b0f9..6e07b30 100644 --- a/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java +++ b/core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java @@ -415,7 +415,7 @@ public class ThriftScanner { if (scanState.scanID == null) { String msg = "Starting scan tserver=" + loc.tablet_location + " tablet=" + loc.tablet_extent + " range=" + scanState.range + " ssil=" - + scanState.serverSideIteratorList + " ssio=" + scanState.serverSideIteratorOptions; + + scanState.serverSideIteratorList + " ssio=" + scanState.serverSideIteratorOptions + " context=" + scanState.classLoaderContext; Thread.currentThread().setName(msg); if (log.isTraceEnabled()) { http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java b/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java index 91823c6..1d5728b 100644 --- a/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java +++ b/core/src/main/java/org/apache/accumulo/core/iterators/IteratorUtil.java @@ -255,6 +255,7 @@ public class IteratorUtil { for (IterInfo iterInfo : iters) { Class<? extends SortedKeyValueIterator<K,V>> clazz = null; + log.trace("Attempting to load iterator class {}", iterInfo.className); if (classCache != null) { clazz = classCache.get(iterInfo.className); @@ -294,13 +295,17 @@ public class IteratorUtil { String context, IterInfo iterInfo) throws ClassNotFoundException, IOException { Class<? extends SortedKeyValueIterator<K,V>> clazz; if (useAccumuloClassLoader) { - if (context != null && !context.equals("")) + if (context != null && !context.equals("")) { clazz = (Class<? extends SortedKeyValueIterator<K,V>>) AccumuloVFSClassLoader.getContextManager().loadClass(context, iterInfo.className, SortedKeyValueIterator.class); - else + log.trace("Iterator class {} loaded from context {}, classloader: {}", iterInfo.className, context, clazz.getClassLoader()); + } else { clazz = (Class<? extends SortedKeyValueIterator<K,V>>) AccumuloVFSClassLoader.loadClass(iterInfo.className, SortedKeyValueIterator.class); + log.trace("Iterator class {} loaded from AccumuloVFSClassLoader: {}", iterInfo.className, clazz.getClassLoader()); + } } else { clazz = (Class<? extends SortedKeyValueIterator<K,V>>) Class.forName(iterInfo.className).asSubclass(SortedKeyValueIterator.class); + log.trace("Iterator class {} loaded from classpath", iterInfo.className); } return clazz; } http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java ---------------------------------------------------------------------- diff --git a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java index 7a63910..fef66a8 100644 --- a/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java +++ b/minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java @@ -328,6 +328,8 @@ public class MiniAccumuloClusterImpl implements AccumuloCluster { if (config.getHadoopConfDir() != null) builder.environment().put("HADOOP_CONF_DIR", config.getHadoopConfDir().getAbsolutePath()); + log.info("Starting MiniAccumuloCluster process with class: " + clazz.getSimpleName() + "\n, jvmOpts: " + extraJvmOpts + "\n, classpath: " + classpath + + "\n, args: " + argList + "\n, environment: " + builder.environment().toString()); Process process = builder.start(); LogWriter lw; http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java ---------------------------------------------------------------------- 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 e48d91e..dd8c020 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 @@ -49,11 +49,14 @@ import org.apache.accumulo.tserver.FileManager.ScanFileManager; import org.apache.accumulo.tserver.InMemoryMap.MemoryIterator; import org.apache.accumulo.tserver.TabletIteratorEnvironment; import org.apache.accumulo.tserver.TabletServer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.collect.Iterables; class ScanDataSource implements DataSource { + private static final Logger log = LoggerFactory.getLogger(ScanDataSource.class); // data source state private final Tablet tablet; private ScanFileManager fileManager; @@ -76,6 +79,8 @@ class ScanDataSource implements DataSource { this.options = new ScanOptions(-1, authorizations, defaultLabels, columnSet, ssiList, ssio, interruptFlag, false, samplerConfig, batchTimeOut, context); this.interruptFlag = interruptFlag; this.loadIters = true; + log.debug("new scan data source, tablet: {}, options: {}, interruptFlag: {}, loadIterators: {}", this.tablet, this.options, this.interruptFlag, + this.loadIters); } ScanDataSource(Tablet tablet, ScanOptions options) { @@ -84,6 +89,8 @@ class ScanDataSource implements DataSource { this.options = options; this.interruptFlag = options.getInterruptFlag(); this.loadIters = true; + log.debug("new scan data source, tablet: {}, options: {}, interruptFlag: {}, loadIterators: {}", this.tablet, this.options, this.interruptFlag, + this.loadIters); } ScanDataSource(Tablet tablet, Authorizations authorizations, byte[] defaultLabels, AtomicBoolean iFlag) { @@ -92,6 +99,8 @@ class ScanDataSource implements DataSource { this.options = new ScanOptions(-1, authorizations, defaultLabels, EMPTY_COLS, null, null, iFlag, false, null, -1, null); this.interruptFlag = iFlag; this.loadIters = false; + log.debug("new scan data source, tablet: {}, options: {}, interruptFlag: {}, loadIterators: {}", this.tablet, this.options, this.interruptFlag, + this.loadIters); } @Override @@ -187,9 +196,11 @@ class ScanDataSource implements DataSource { if (!loadIters) { return visFilter; } else if (null == options.getClassLoaderContext()) { + log.trace("Loading iterators for scan"); return iterEnv.getTopLevelIterator(IteratorUtil.loadIterators(IteratorScope.scan, visFilter, tablet.getExtent(), tablet.getTableConfiguration(), options.getSsiList(), options.getSsio(), iterEnv)); } else { + log.trace("Loading iterators for scan with scan context: {}", options.getClassLoaderContext()); return iterEnv.getTopLevelIterator(IteratorUtil.loadIterators(IteratorScope.scan, visFilter, tablet.getExtent(), tablet.getTableConfiguration(), options.getSsiList(), options.getSsio(), iterEnv, true, options.getClassLoaderContext())); } http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanOptions.java ---------------------------------------------------------------------- diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanOptions.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanOptions.java index dceac08..a898653 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanOptions.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanOptions.java @@ -109,4 +109,20 @@ final class ScanOptions { public void setClassLoaderContext(String context) { this.classLoaderContext = context; } + + @Override + public String toString() { + StringBuilder buf = new StringBuilder(); + buf.append("["); + buf.append("auths=").append(this.authorizations); + buf.append(", batchTimeOut=").append(this.batchTimeOut); + buf.append(", context=").append(this.classLoaderContext); + buf.append(", columns=").append(this.columnSet); + buf.append(", interruptFlag=").append(this.interruptFlag); + buf.append(", isolated=").append(this.isolated); + buf.append(", num=").append(this.num); + buf.append(", samplerConfig=").append(this.samplerConfig); + buf.append("]"); + return buf.toString(); + } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java ---------------------------------------------------------------------- diff --git a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java index 7145b4a..ffb7dc1 100644 --- a/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java +++ b/start/src/main/java/org/apache/accumulo/start/classloader/vfs/ContextManager.java @@ -25,9 +25,13 @@ import java.util.Set; import org.apache.commons.vfs2.FileSystemException; import org.apache.commons.vfs2.FileSystemManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class ContextManager { + private static final Logger log = LoggerFactory.getLogger(ContextManager.class); + // there is a lock per context so that one context can initialize w/o blocking another context private class Context { AccumuloReloadingVFSClassLoader loader; @@ -202,9 +206,10 @@ public class ContextManager { contexts.keySet().removeAll(unused.keySet()); } - for (Context context : unused.values()) { + for (Entry<String,Context> e : unused.entrySet()) { // close outside of lock - context.close(); + log.info("Closing unused context: {}", e.getKey()); + e.getValue().close(); } } } http://git-wip-us.apache.org/repos/asf/accumulo/blob/2c883889/test/src/main/java/org/apache/accumulo/test/ShellServerIT.java ---------------------------------------------------------------------- diff --git a/test/src/main/java/org/apache/accumulo/test/ShellServerIT.java b/test/src/main/java/org/apache/accumulo/test/ShellServerIT.java index 8254570..d67dc0a 100644 --- a/test/src/main/java/org/apache/accumulo/test/ShellServerIT.java +++ b/test/src/main/java/org/apache/accumulo/test/ShellServerIT.java @@ -1747,16 +1747,21 @@ public class ShellServerIT extends SharedMiniClusterBase { assertTrue(true); } ts.exec("createtable t"); + // Assert that the TabletServer does not know anything about our class + String result = ts.exec("setiter -scan -n reverse -t t -p 21 -class org.apache.accumulo.test.functional.ValueReversingIterator"); + assertTrue(result.contains("class not found")); make10(); setupFakeContextPath(); - // Add the context to the table so that setscaniter works. After setscaniter succeeds, then - // remove the property from the table. - ts.exec("config -s " + Property.VFS_CONTEXT_CLASSPATH_PROPERTY + FAKE_CONTEXT + "=" + FAKE_CONTEXT_CLASSPATH); - ts.exec("config -t t -s table.classpath.context=" + FAKE_CONTEXT); - ts.exec("setscaniter -n reverse -t t -p 21 -class org.apache.accumulo.test.functional.ValueReversingIterator"); - String result = ts.exec("scan -np -b row1 -e row1"); + // Add the context to the table so that setiter works. + result = ts.exec("config -s " + Property.VFS_CONTEXT_CLASSPATH_PROPERTY + FAKE_CONTEXT + "=" + FAKE_CONTEXT_CLASSPATH); + assertEquals("root@miniInstance t> config -s " + Property.VFS_CONTEXT_CLASSPATH_PROPERTY + FAKE_CONTEXT + "=" + FAKE_CONTEXT_CLASSPATH + "\n", result); + result = ts.exec("config -t t -s table.classpath.context=" + FAKE_CONTEXT); + assertEquals("root@miniInstance t> config -t t -s table.classpath.context=" + FAKE_CONTEXT + "\n", result); + result = ts.exec("setiter -scan -n reverse -t t -p 21 -class org.apache.accumulo.test.functional.ValueReversingIterator"); + assertTrue(result.contains("The iterator class does not implement OptionDescriber")); + // The implementation of ValueReversingIterator in the FAKE context does nothing, the value is not reversed. + result = ts.exec("scan -np -b row1 -e row1"); assertEquals(2, result.split("\n").length); - log.error(result); assertTrue(result.contains("value")); result = ts.exec("scan -np -b row3 -e row5"); assertEquals(4, result.split("\n").length); @@ -1774,9 +1779,11 @@ public class ShellServerIT extends SharedMiniClusterBase { assertTrue(result.contains("value")); setupRealContextPath(); - ts.exec("config -s " + Property.VFS_CONTEXT_CLASSPATH_PROPERTY + REAL_CONTEXT + "=" + REAL_CONTEXT_CLASSPATH); + // Define a new classloader context, but don't set it on the table + result = ts.exec("config -s " + Property.VFS_CONTEXT_CLASSPATH_PROPERTY + REAL_CONTEXT + "=" + REAL_CONTEXT_CLASSPATH); + assertEquals("root@miniInstance t> config -s " + Property.VFS_CONTEXT_CLASSPATH_PROPERTY + REAL_CONTEXT + "=" + REAL_CONTEXT_CLASSPATH + "\n", result); + // Override the table classloader context with the REAL implementation of ValueReversingIterator, which does reverse the value. result = ts.exec("scan -np -b row1 -e row1 -cc " + REAL_CONTEXT); - log.error(result); assertEquals(2, result.split("\n").length); assertTrue(result.contains("eulav")); assertFalse(result.contains("value"));
