> On Jan. 28, 2015, 6: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. > > Christopher Tubbs wrote: > 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?
Testing is also what I was thinking. The fact that its screwy is unrelated to the possibility that it may be used. Maybe just deprecate it and call it a day. > On Jan. 28, 2015, 6:07 p.m., kturner wrote: > > server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java, > > line 166 > > <https://reviews.apache.org/r/30226/diff/1/?file=832071#file832071line166> > > > > Is this thrift method used? > > Christopher Tubbs wrote: > Will have to check. If this was the only method using that thrift method, > then perhaps not. Should it be? > Should it be? don't know. Somehow tablets are flushed, maybe this method is directly called from clients? Not sure. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30226/#review70025 ----------------------------------------------------------- On Jan. 23, 2015, 9: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, 9: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 > >
