> On Jan. 28, 2015, 1:07 p.m., kturner wrote: > > core/src/main/java/org/apache/accumulo/core/iterators/DebugIterator.java, > > line 47 > > <https://reviews.apache.org/r/30226/diff/1/?file=832041#file832041line47> > > > > I know its not officially public API, but we do encourage users to use > > Iterators. This is something a user could have possible used. > > Christopher Tubbs wrote: > What would they use the constructor for? They might leverage iterators, > but I can't imagine they'd construct them. The framework should do that. > > Josh Elser wrote: > Testing is good example of why such a constructor is nice to have.
I suppose, but it seems to me that a better test would be to reproduce the behavior of Accumulo by calling the no-arg constructor and init() rather than this one... but sure, maybe somebody is using it. It doesn't have to be deleted. Though, if it stays, it should probably have a test... right now, it isn't used at all, so there's significant risk it's behavior will become incorrect over time without anybody noticing. And, I'm not even clear on how it's supposed to be used normally... the prefix field isn't even settable normally (from OptionsDescriber / options). It seems to have been created exclusively for internal use debugging. And then, with regards to the constructor itself... I have no idea how that's supposed to be useful at all. The source parameter, for instance, seems redundant with the init() method's use of source. So, is this a constructor expected to be used when *not* using init? Not sure that makes sense. Maybe its behavior has already diverged from its original intent, because of lack of use/testing? - Christopher ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30226/#review70025 ----------------------------------------------------------- On Jan. 23, 2015, 4:02 p.m., Christopher Tubbs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30226/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2015, 4:02 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-3204 > https://issues.apache.org/jira/browse/ACCUMULO-3204 > > > Repository: accumulo > > > Description > ------- > > I ran UCDetector in Eclipse and found about 2000 lines of code which could > (maybe?) be removed. > > Some of this code might represent bugs (the code is supposed to be used, but > isn't, because of a mistake... and I've already found a few of those cases), > some of it is unused because it's loaded dynamically, via reflection, or it's > required for a particular framework, or it's just stale code which is safe to > remove. I'd like the community's help in determining which is which by giving > some feedback on this rough first pass. > > UCDetector also detects code with a greater visibility than necessary. Some > of this patch includes those... I started ignoring those after a bit and just > focused on completely unused code, so you will see a few of those, but not as > many as there could be. > > I tried not to include any public API in these changes, but I may have missed > some. > > Some code might only be used in tests, too. I don't think I caught those > here. Some code also is unused, but is public API, and should minimally have > unit tests to verify public API functionality. I've tried to open JIRA issues > for those, as I found them. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/bloomfilter/Filter.java 12961a9 > > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationClient.java > a449389 > > core/src/main/java/org/apache/accumulo/core/client/impl/ScannerIterator.java > 276e1d6 > core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java > 5b6d9ac > core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java 20b1639 > core/src/main/java/org/apache/accumulo/core/client/mock/MockAccumulo.java > f171889 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java > 8c57c5e > core/src/main/java/org/apache/accumulo/core/client/mock/MockTable.java > 2244d20 > core/src/main/java/org/apache/accumulo/core/conf/Property.java 68fac73 > core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java > f0c9e59 > core/src/main/java/org/apache/accumulo/core/data/PartialKey.java f4289d2 > > core/src/main/java/org/apache/accumulo/core/file/blockfile/ABlockReader.java > 8df2469 > > core/src/main/java/org/apache/accumulo/core/file/blockfile/ABlockWriter.java > ece0a5e > > core/src/main/java/org/apache/accumulo/core/file/blockfile/BlockFileWriter.java > 3bdbea3 > > core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/BlockCache.java > a6c08ff > > core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/CachedBlock.java > b6d6d41 > > core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/ClassSize.java > 2d7586f > > core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java > 4d65c9f > > core/src/main/java/org/apache/accumulo/core/file/keyfunctor/ColumnFamilyFunctor.java > 3660291 > core/src/main/java/org/apache/accumulo/core/file/rfile/SplitLarge.java > b87705c > core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/BCFile.java > ecc0b90 > > core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/CompareUtils.java > d7651e8 > core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Utils.java > 84b861b > core/src/main/java/org/apache/accumulo/core/iterators/DebugIterator.java > 92f49f9 > > core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java > 534dd7f > > core/src/main/java/org/apache/accumulo/core/replication/AccumuloReplicationReplayer.java > fccafc5 > > core/src/main/java/org/apache/accumulo/core/replication/ReplicationSchema.java > ed46130 > > core/src/main/java/org/apache/accumulo/core/replication/StatusFormatter.java > bc04480 > core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java d880fb3 > core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java 1426239 > core/src/main/java/org/apache/accumulo/core/util/ByteArraySet.java ca43469 > core/src/main/java/org/apache/accumulo/core/util/Daemon.java a2c9e79 > core/src/main/java/org/apache/accumulo/core/util/MapCounter.java f6f3ff7 > core/src/main/java/org/apache/accumulo/core/util/StopWatch.java ddb612f > core/src/main/java/org/apache/accumulo/core/volume/VolumeConfiguration.java > 99032ad > > examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/ChunkInputStream.java > c902271 > > examples/simple/src/main/java/org/apache/accumulo/examples/simple/mapreduce/TeraSortIngest.java > 9aac75e > fate/src/main/java/org/apache/accumulo/fate/util/Daemon.java da7c41c > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 5cfdbb8 > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java a0100b2 > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooQueueLock.java > f9195f3 > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java > 6b5ec43 > minicluster/src/main/java/org/apache/accumulo/cluster/RemoteShell.java > 5a44acf > > minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java > 58536ed > > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java > 76f332b > > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/ZooKeeperBindException.java > 50217ce > server/base/src/main/java/org/apache/accumulo/server/ServerConstants.java > 9d969d1 > > server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java > 01d03ed > > server/base/src/main/java/org/apache/accumulo/server/conf/NamespaceConfWatcher.java > 945e904 > > server/base/src/main/java/org/apache/accumulo/server/conf/TableConfWatcher.java > 3c8d45d > > server/base/src/main/java/org/apache/accumulo/server/conf/TableParentConfiguration.java > bd2e5ab > > server/base/src/main/java/org/apache/accumulo/server/conf/ZooConfigurationFactory.java > 4fbb645 > > server/base/src/main/java/org/apache/accumulo/server/log/SortedLogState.java > c0580ac > > server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java > 917d8d2 > > server/base/src/main/java/org/apache/accumulo/server/master/state/DistributedStoreException.java > 3290075 > > server/base/src/main/java/org/apache/accumulo/server/master/state/TServerInstance.java > c0c71e6 > > server/base/src/main/java/org/apache/accumulo/server/master/state/TabletServerState.java > dde9807 > > server/base/src/main/java/org/apache/accumulo/server/metrics/AbstractMetricsImpl.java > 93db9c8 > > server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsConfiguration.java > b0ffd64 > > server/base/src/main/java/org/apache/accumulo/server/rpc/TBufferedServerSocket.java > 2887f48 > > server/base/src/main/java/org/apache/accumulo/server/rpc/TNonblockingServerSocket.java > d035862 > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java > 7adb46e > > server/base/src/main/java/org/apache/accumulo/server/security/SecurityUtil.java > 6014139 > > server/base/src/main/java/org/apache/accumulo/server/security/handler/KerberosAuthorizor.java > b047f1a > > server/base/src/main/java/org/apache/accumulo/server/security/handler/KerberosPermissionHandler.java > 691c555 > > server/base/src/main/java/org/apache/accumulo/server/tables/TableManager.java > 7e5f54d > > server/base/src/main/java/org/apache/accumulo/server/util/AccumuloStatus.java > 1e75124 > > server/base/src/main/java/org/apache/accumulo/server/util/CleanZookeeper.java > 3cfb1a7 > > server/base/src/main/java/org/apache/accumulo/server/util/FileSystemMonitor.java > 94a80d9 > > server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java > ed7626e > > server/base/src/main/java/org/apache/accumulo/server/util/RestoreZookeeper.java > e5a2add > server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java > 759d898 > > server/base/src/main/java/org/apache/accumulo/server/util/time/RelativeTime.java > bc48b10 > > server/base/src/main/java/org/apache/accumulo/server/util/time/SimpleTimer.java > 556e6b9 > > server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooCache.java > aca9c82 > server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java > da0b07c > > server/master/src/main/java/org/apache/accumulo/master/replication/DistributedWorkQueueWorkAssigner.java > 3e966c4 > > server/master/src/main/java/org/apache/accumulo/master/replication/SequentialWorkAssigner.java > e30e9ac > > server/master/src/main/java/org/apache/accumulo/master/replication/StatusMaker.java > c7f47e4 > > server/master/src/main/java/org/apache/accumulo/master/replication/WorkDriver.java > 3558d2d > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java > 1a2904c > > server/monitor/src/main/java/org/apache/accumulo/monitor/ZooKeeperStatus.java > 89e879e > server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java > 64fee7e > server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java > 3db77f0 > > server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java > dcbdae7 > > server/tserver/src/main/java/org/apache/accumulo/tserver/BulkFailedCopyProcessor.java > cfb5fb4 > > server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java > 0cb04a7 > > server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionalMutationSet.java > 79a1176 > server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java > 0a7de95 > > server/tserver/src/main/java/org/apache/accumulo/tserver/HoldTimeoutException.java > 1bd2c2c > server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java > 47936b6 > server/tserver/src/main/java/org/apache/accumulo/tserver/MemValue.java > 0ce3b9e > server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java > c2e45c3 > > server/tserver/src/main/java/org/apache/accumulo/tserver/RunnableStartedAt.java > 6513091 > server/tserver/src/main/java/org/apache/accumulo/tserver/TLevel.java > 5705c9e > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java > 7d49e65 > > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java > 351d526 > > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletStatsKeeper.java > 1e2cdf4 > > server/tserver/src/main/java/org/apache/accumulo/tserver/TooManyFilesException.java > 026f7e2 > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/MajorCompactionRequest.java > 900600f > > server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/strategies/ConfigurableCompactionStrategy.java > ba3ea42 > > server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/UnsatisfiableConstraint.java > 64bc2cd > > server/tserver/src/main/java/org/apache/accumulo/tserver/data/ServerConditionalMutation.java > 84137cc > server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java > 5acf5eb > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/LocalWALRecovery.java > 2658c1f > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java > 405ec70 > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java > 5c3fc2d > > server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileKey.java > 829cf2f > > server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogFileValue.java > 9ca0f38 > > server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java > df2f831 > > server/tserver/src/main/java/org/apache/accumulo/tserver/mastermessage/SplitReportMessage.java > 0c93a86 > > server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMetricsUtil.java > 5905aea > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java > a07f354 > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/BatchWriterReplicationReplayer.java > 8a80ea3 > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/ReplicationProcessor.java > c23cd94 > > server/tserver/src/main/java/org/apache/accumulo/tserver/session/Session.java > 9aaa17a > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactionStats.java > 68a2307 > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/KVEntry.java > 4b1cf8c > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Rate.java > a0ea2d6 > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java > a73356d > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletClosedException.java > d3ed507 > shell/src/main/java/org/apache/accumulo/shell/Shell.java a64ff45 > shell/src/main/java/org/apache/accumulo/shell/ShellCommandException.java > 49e7163 > shell/src/main/java/org/apache/accumulo/shell/ShellCompletor.java 3ed6a04 > shell/src/main/java/org/apache/accumulo/shell/ShellOptions.java 302a8a9 > shell/src/main/java/org/apache/accumulo/shell/Token.java cd25ada > shell/src/main/java/org/apache/accumulo/shell/commands/CompactCommand.java > 131534f > shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java > 5917b1e > > start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java > 1cee6d7 > > Diff: https://reviews.apache.org/r/30226/diff/ > > > Testing > ------- > > > Thanks, > > Christopher Tubbs > >
