You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@accumulo.apache.org by Christopher Tubbs <ct...@apache.org> on 2015/01/23 22:02:22 UTC

Review Request 30226: ACCUMULO-3204 Unused code

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/
-----------------------------------------------------------

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

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/
-----------------------------------------------------------

(Updated April 14, 2015, 6:44 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 (updated)
-----

  core/src/main/java/org/apache/accumulo/core/bloomfilter/Filter.java 27eab69 
  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 123f532 
  core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java 3d486d2 
  core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java 44f48c3 
  core/src/main/java/org/apache/accumulo/core/client/mock/MockAccumulo.java f171889 
  core/src/main/java/org/apache/accumulo/core/client/mock/MockTable.java 2244d20 
  core/src/main/java/org/apache/accumulo/core/conf/Property.java 08ba3a2 
  core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java e8c49b4 
  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 eef900c 
  core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/ClassSize.java b46da23 
  core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/LruBlockCache.java 2bd1a38 
  core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java 54da7d9 
  core/src/main/java/org/apache/accumulo/core/file/keyfunctor/ColumnFamilyFunctor.java 3660291 
  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 ac0bdb1 
  core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Utils.java fca120e 
  core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
  core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java 0f952c2 
  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/mapreduce/TeraSortIngest.java 9aac75e 
  fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 1fade84 
  fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java 11da1ec 
  fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooQueueLock.java f9195f3 
  fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java 811f035 
  minicluster/src/main/java/org/apache/accumulo/cluster/RemoteShell.java 5a44acf 
  minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java dc0bc18 
  minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 2df1c5d 
  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 8e70d9b 
  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 13cd0a1 
  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 39d5602 
  server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsConfiguration.java 8d11243 
  server/base/src/main/java/org/apache/accumulo/server/replication/StatusFormatter.java cee30bc 
  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 c0bcdb7 
  server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java f08742d 
  server/base/src/main/java/org/apache/accumulo/server/security/SecurityUtil.java e27a7e7 
  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 293aaf8 
  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 afe7f6f 
  server/base/src/main/java/org/apache/accumulo/server/util/FileSystemMonitor.java f1ba8dc 
  server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 0c0ecc0 
  server/base/src/main/java/org/apache/accumulo/server/util/RestoreZookeeper.java e5a2add 
  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 cbe021a 
  server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooCache.java aca9c82 
  server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java ffa59cd 
  server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java 5eac8bb 
  server/monitor/src/main/java/org/apache/accumulo/monitor/ZooKeeperStatus.java 1daf8f6 
  server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java 4ba6309 
  server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java c02f7f2 
  server/tserver/src/main/java/org/apache/accumulo/tserver/BulkFailedCopyProcessor.java 26fe8ba 
  server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java 3ba7a5c 
  server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionalMutationSet.java 68ccdf0 
  server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java 02dacd7 
  server/tserver/src/main/java/org/apache/accumulo/tserver/HoldTimeoutException.java 1bd2c2c 
  server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java cd96717 
  server/tserver/src/main/java/org/apache/accumulo/tserver/MemValue.java 0ce3b9e 
  server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java 2c47f07 
  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 ec51ac4 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java cb8d01f 
  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 444a97f 
  server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/strategies/ConfigurableCompactionStrategy.java b97b88b 
  server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/UnsatisfiableConstraint.java 16fb9bc 
  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 60c8e8d 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
  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 09bc705 
  server/tserver/src/main/java/org/apache/accumulo/tserver/mastermessage/SplitReportMessage.java fa35cd3 
  server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMetricsUtil.java 5905aea 
  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 2342789 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletClosedException.java d3ed507 
  shell/src/main/java/org/apache/accumulo/shell/Shell.java 8abafdb 
  shell/src/main/java/org/apache/accumulo/shell/ShellCommandException.java d24c4e0 
  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 a533aa1 
  shell/src/main/java/org/apache/accumulo/shell/commands/CompactCommand.java e39d862 
  shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java 5917b1e 
  start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java 4df3f40 

Diff: https://reviews.apache.org/r/30226/diff/


Testing
-------


Thanks,

Christopher Tubbs

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> On Jan. 31, 2015, midnight, Sean Busbey wrote:
> > server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java, lines 50-53
> > <https://reviews.apache.org/r/30226/diff/1/?file=832100#file832100line50>
> >
> >     It looks like the util just ignores the start/end ranges? Can we get a follow on issue to fix this (even if we remove the parameters until it's implemented)?

Fixed in ACCUMULO-3715


> On Jan. 31, 2015, midnight, Sean Busbey wrote:
> > server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java, lines 56-57
> > <https://reviews.apache.org/r/30226/diff/1/?file=832100#file832100line56>
> >
> >     does this instance get ignored for the one on ClientOpts?

Fixed in ACCUMULO-3715


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70490
-----------------------------------------------------------


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Sean Busbey <se...@manvsbeard.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70490
-----------------------------------------------------------



core/src/main/java/org/apache/accumulo/core/client/mock/MockConfiguration.java
<https://reviews.apache.org/r/30226/#comment115699>

    If this is aiming at 1.7, then you can't remove this because it's public api.



server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java
<https://reviews.apache.org/r/30226/#comment115702>

    It looks like the util just ignores the start/end ranges? Can we get a follow on issue to fix this (even if we remove the parameters until it's implemented)?



server/tracer/src/main/java/org/apache/accumulo/tracer/TraceDump.java
<https://reviews.apache.org/r/30226/#comment115701>

    does this instance get ignored for the one on ClientOpts?


- Sean Busbey


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java, line 144
> > <https://reviews.apache.org/r/30226/diff/1/?file=832098#file832098line144>
> >
> >     Does this really mean that we're just not displaying it on the monitor? I think we'd want to do that rather than just throw away these stats.
> 
> Christopher Tubbs wrote:
>     Yeah, so that's the kind of responses I was hoping for in this review... I don't know. Is this something that should be displayed, or is it really unneeded? Is it a bug? I just don't know enough about what this particular code was supposed to be doing, without taking a longer amount of time to investigate each instance of this kind of thing.
> 
> Josh Elser wrote:
>     I think it's a sign that we have more data we could display that we're not in this specific case.

Created ACCUMULO-3739


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java, line 737
> > <https://reviews.apache.org/r/30226/diff/1/?file=832114#file832114line737>
> >
> >     These looks kind of important. Are we not stopping these pools?
> 
> Christopher Tubbs wrote:
>     I agree. It does look important... and I cannot find any place where we are shutting down these pools.

Created ACCUMULO-3740


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------


On April 14, 2015, 6:44 p.m., Christopher Tubbs wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30226/
> -----------------------------------------------------------
> 
> (Updated April 14, 2015, 6:44 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 27eab69 
>   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 123f532 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java 3d486d2 
>   core/src/main/java/org/apache/accumulo/core/client/impl/Tables.java 44f48c3 
>   core/src/main/java/org/apache/accumulo/core/client/mock/MockAccumulo.java f171889 
>   core/src/main/java/org/apache/accumulo/core/client/mock/MockTable.java 2244d20 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 08ba3a2 
>   core/src/main/java/org/apache/accumulo/core/conf/SiteConfiguration.java e8c49b4 
>   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 eef900c 
>   core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/ClassSize.java b46da23 
>   core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/LruBlockCache.java 2bd1a38 
>   core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/CachableBlockFile.java 54da7d9 
>   core/src/main/java/org/apache/accumulo/core/file/keyfunctor/ColumnFamilyFunctor.java 3660291 
>   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 ac0bdb1 
>   core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Utils.java fca120e 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/rpc/ThriftUtil.java 0f952c2 
>   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/mapreduce/TeraSortIngest.java 9aac75e 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooCache.java 1fade84 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java 11da1ec 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooQueueLock.java f9195f3 
>   fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java 811f035 
>   minicluster/src/main/java/org/apache/accumulo/cluster/RemoteShell.java 5a44acf 
>   minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java dc0bc18 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java 2df1c5d 
>   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 8e70d9b 
>   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 13cd0a1 
>   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 39d5602 
>   server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsConfiguration.java 8d11243 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusFormatter.java cee30bc 
>   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 c0bcdb7 
>   server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java f08742d 
>   server/base/src/main/java/org/apache/accumulo/server/security/SecurityUtil.java e27a7e7 
>   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 293aaf8 
>   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 afe7f6f 
>   server/base/src/main/java/org/apache/accumulo/server/util/FileSystemMonitor.java f1ba8dc 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 0c0ecc0 
>   server/base/src/main/java/org/apache/accumulo/server/util/RestoreZookeeper.java e5a2add 
>   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 cbe021a 
>   server/base/src/main/java/org/apache/accumulo/server/zookeeper/ZooCache.java aca9c82 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java ffa59cd 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java 5eac8bb 
>   server/monitor/src/main/java/org/apache/accumulo/monitor/ZooKeeperStatus.java 1daf8f6 
>   server/tracer/src/main/java/org/apache/accumulo/tracer/ZooTraceClient.java 4ba6309 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/ActiveAssignmentRunnable.java c02f7f2 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/BulkFailedCopyProcessor.java 26fe8ba 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/CompactionQueue.java 3ba7a5c 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionalMutationSet.java 68ccdf0 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/FileManager.java 02dacd7 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/HoldTimeoutException.java 1bd2c2c 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/InMemoryMap.java cd96717 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/MemValue.java 0ce3b9e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java 2c47f07 
>   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 ec51ac4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java cb8d01f 
>   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 444a97f 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/compaction/strategies/ConfigurableCompactionStrategy.java b97b88b 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/constraints/UnsatisfiableConstraint.java 16fb9bc 
>   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 60c8e8d 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   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 09bc705 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/mastermessage/SplitReportMessage.java fa35cd3 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/TabletServerMetricsUtil.java 5905aea 
>   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 2342789 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletClosedException.java d3ed507 
>   shell/src/main/java/org/apache/accumulo/shell/Shell.java 8abafdb 
>   shell/src/main/java/org/apache/accumulo/shell/ShellCommandException.java d24c4e0 
>   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 a533aa1 
>   shell/src/main/java/org/apache/accumulo/shell/commands/CompactCommand.java e39d862 
>   shell/src/main/java/org/apache/accumulo/shell/commands/ScanCommand.java 5917b1e 
>   start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java 4df3f40 
> 
> Diff: https://reviews.apache.org/r/30226/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Christopher Tubbs
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Josh Elser <jo...@gmail.com>.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java, line 49
> > <https://reviews.apache.org/r/30226/diff/1/?file=832061#file832061line49>
> >
> >     I don't see the need to remove this. Would be happy to add a test so it's "used".
> 
> Christopher Tubbs wrote:
>     I don't recall this one specifically. What's it for?

It's what lets us run ITs against a "real" cluster.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/ChunkInputStream.java, line 53
> > <https://reviews.apache.org/r/30226/diff/1/?file=832053#file832053line53>
> >
> >     Example code could have been used by users. We might not want to remove things here?
> 
> Christopher Tubbs wrote:
>     Examples are like documentation, in my opinion, and should be subject to change, because by its very nature as an example, we recognize that it's not worthy of being used directly. I also think it should be succinct. Too much code in the examples complicates them and makes them harder to teach the underlying principles. That's why I think unused code should probably be stripped out there.

Ok, I mostly brought it up because I don't think we have an agreed upon precedent here.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationClient.java, line 184
> > <https://reviews.apache.org/r/30226/diff/1/?file=832019#file832019line184>
> >
> >     I don't know what we want the policy to be here. No, this isn't used because we have no one-way thrift RPCs presently. This may change in the future.
> 
> Christopher Tubbs wrote:
>     If we don't need it now, and we don't know of any specific work that will leverage it shortly, I think we should strip it out, otherwise it risks getting stale. It may be good to save off the diff in a separate branch to add later as part of a larger patch to implement a feature that would leverage these.

IMO, an interface and a stub method throwing an Exception likely isn't going to turn stale. Mostly bringing up why it was there at all.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java, line 747
> > <https://reviews.apache.org/r/30226/diff/1/?file=832062#file832062line747>
> >
> >     Should preserve the accompanying getter for the setter.
> 
> Christopher Tubbs wrote:
>     Why is a setter needed if the getter is never used?

Testing


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > server/master/src/main/java/org/apache/accumulo/master/replication/DistributedWorkQueueWorkAssigner.java, line 71
> > <https://reviews.apache.org/r/30226/diff/1/?file=832094#file832094line71>
> >
> >     I would prefer to not nuke these. It's just going to be more work when I inevitably find bugs and need to write more tests.
> 
> Christopher Tubbs wrote:
>     Do you have any existing tests that could benefit from using it now? And, which is the greater risk? That there may be some extra effort needed later, or that the current unused method will become stale or broken until that time?

Not having/using the getters/setters make me think that coverage on the tests could be improved. I don't see getters/setters going stale, and their hypothetical removal just causing more work later.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java, line 144
> > <https://reviews.apache.org/r/30226/diff/1/?file=832098#file832098line144>
> >
> >     Does this really mean that we're just not displaying it on the monitor? I think we'd want to do that rather than just throw away these stats.
> 
> Christopher Tubbs wrote:
>     Yeah, so that's the kind of responses I was hoping for in this review... I don't know. Is this something that should be displayed, or is it really unneeded? Is it a bug? I just don't know enough about what this particular code was supposed to be doing, without taking a longer amount of time to investigate each instance of this kind of thing.

I think it's a sign that we have more data we could display that we're not in this specific case.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java, line 88
> > <https://reviews.apache.org/r/30226/diff/1/?file=832130#file832130line88>
> >
> >     Again, unnecessary removal of getters/setters IMO
> 
> Christopher Tubbs wrote:
>     It seems that if we don't use the getter, we don't need the setter either.

Again, a sign that test coverage could/should be improved


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationClient.java, line 184
> > <https://reviews.apache.org/r/30226/diff/1/?file=832019#file832019line184>
> >
> >     I don't know what we want the policy to be here. No, this isn't used because we have no one-way thrift RPCs presently. This may change in the future.

If we don't need it now, and we don't know of any specific work that will leverage it shortly, I think we should strip it out, otherwise it risks getting stale. It may be good to save off the diff in a separate branch to add later as part of a larger patch to implement a feature that would leverage these.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/replication/AccumuloReplicationReplayer.java, line 34
> > <https://reviews.apache.org/r/30226/diff/1/?file=832043#file832043line34>
> >
> >     This is stubbed out to eventually support bulk imports

Same reply as above.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/ChunkInputStream.java, line 53
> > <https://reviews.apache.org/r/30226/diff/1/?file=832053#file832053line53>
> >
> >     Example code could have been used by users. We might not want to remove things here?

Examples are like documentation, in my opinion, and should be subject to change, because by its very nature as an example, we recognize that it's not worthy of being used directly. I also think it should be succinct. Too much code in the examples complicates them and makes them harder to teach the underlying principles. That's why I think unused code should probably be stripped out there.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java, line 747
> > <https://reviews.apache.org/r/30226/diff/1/?file=832062#file832062line747>
> >
> >     Should preserve the accompanying getter for the setter.

Why is a setter needed if the getter is never used?


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java, line 49
> > <https://reviews.apache.org/r/30226/diff/1/?file=832061#file832061line49>
> >
> >     I don't see the need to remove this. Would be happy to add a test so it's "used".

I don't recall this one specifically. What's it for?


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/master/src/main/java/org/apache/accumulo/master/replication/DistributedWorkQueueWorkAssigner.java, line 71
> > <https://reviews.apache.org/r/30226/diff/1/?file=832094#file832094line71>
> >
> >     I would prefer to not nuke these. It's just going to be more work when I inevitably find bugs and need to write more tests.

Do you have any existing tests that could benefit from using it now? And, which is the greater risk? That there may be some extra effort needed later, or that the current unused method will become stale or broken until that time?


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java, line 144
> > <https://reviews.apache.org/r/30226/diff/1/?file=832098#file832098line144>
> >
> >     Does this really mean that we're just not displaying it on the monitor? I think we'd want to do that rather than just throw away these stats.

Yeah, so that's the kind of responses I was hoping for in this review... I don't know. Is this something that should be displayed, or is it really unneeded? Is it a bug? I just don't know enough about what this particular code was supposed to be doing, without taking a longer amount of time to investigate each instance of this kind of thing.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java, line 88
> > <https://reviews.apache.org/r/30226/diff/1/?file=832130#file832130line88>
> >
> >     Again, unnecessary removal of getters/setters IMO

It seems that if we don't use the getter, we don't need the setter either.


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java, line 238
> > <https://reviews.apache.org/r/30226/diff/1/?file=832098#file832098line238>
> >
> >     Is the online tablets in the monitor calculated some different way?

We don't display online tablets. We display a total tablet count, and the unassigned tablets.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java, line 737
> > <https://reviews.apache.org/r/30226/diff/1/?file=832114#file832114line737>
> >
> >     These looks kind of important. Are we not stopping these pools?

I agree. It does look important... and I cannot find any place where we are shutting down these pools.


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Josh Elser <jo...@gmail.com>.


> On Jan. 30, 2015, 6:21 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java, line 88
> > <https://reviews.apache.org/r/30226/diff/1/?file=832130#file832130line88>
> >
> >     Again, unnecessary removal of getters/setters IMO
> 
> Christopher Tubbs wrote:
>     It seems that if we don't use the getter, we don't need the setter either.
> 
> Josh Elser wrote:
>     Again, a sign that test coverage could/should be improved
> 
> Christopher Tubbs wrote:
>     Do any of these fall into what you consider API for replication? Not "public API", of course, but API in the sense that it's an essential function of the replication feature that could be leveraged to create an alternate implementation?
>     
>     If so, I think: needs tests. If not, I think: delete.

I kind of see the ReplicaSystem API falling into something like less-public than SKVI but more-public than random server-side code. Something akin to LimitedPrivate from Hadoop.

I covered a couple of examples of how ReplicaSystem could be implemented in a recent blog post. I think it would be interesting to investigate them. Including this as "needs more tests" would be super.


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationClient.java, line 184
> > <https://reviews.apache.org/r/30226/diff/1/?file=832019#file832019line184>
> >
> >     I don't know what we want the policy to be here. No, this isn't used because we have no one-way thrift RPCs presently. This may change in the future.
> 
> Christopher Tubbs wrote:
>     If we don't need it now, and we don't know of any specific work that will leverage it shortly, I think we should strip it out, otherwise it risks getting stale. It may be good to save off the diff in a separate branch to add later as part of a larger patch to implement a feature that would leverage these.
> 
> Josh Elser wrote:
>     IMO, an interface and a stub method throwing an Exception likely isn't going to turn stale. Mostly bringing up why it was there at all.

Maybe... but it also might not be useful at all in the eventual implementation. Or, with it there, somebody could use it for something other than what was intended. I think it might be better to just strip it out, keep those extra methods in a working branch for the eventual feature it is intended for, if necessary. Keith suggested we could just comment on the ticket about the commit in which it was removed, to save anybody who eventually implements it some effort.

I don't really care that much. From my perspective, it's just another bit of unused code. It's not obvious it's for a planned future feature, or from an long-dead one.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/ChunkInputStream.java, line 53
> > <https://reviews.apache.org/r/30226/diff/1/?file=832053#file832053line53>
> >
> >     Example code could have been used by users. We might not want to remove things here?
> 
> Christopher Tubbs wrote:
>     Examples are like documentation, in my opinion, and should be subject to change, because by its very nature as an example, we recognize that it's not worthy of being used directly. I also think it should be succinct. Too much code in the examples complicates them and makes them harder to teach the underlying principles. That's why I think unused code should probably be stripped out there.
> 
> Josh Elser wrote:
>     Ok, I mostly brought it up because I don't think we have an agreed upon precedent here.

You're right, of course. We don't. Are you leaning towards an opinion about this? (I'm leaning towards stripping it out.)


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java, line 49
> > <https://reviews.apache.org/r/30226/diff/1/?file=832061#file832061line49>
> >
> >     I don't see the need to remove this. Would be happy to add a test so it's "used".
> 
> Christopher Tubbs wrote:
>     I don't recall this one specifically. What's it for?
> 
> Josh Elser wrote:
>     It's what lets us run ITs against a "real" cluster.

Ah, we should definitely have a test for it, then.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java, line 747
> > <https://reviews.apache.org/r/30226/diff/1/?file=832062#file832062line747>
> >
> >     Should preserve the accompanying getter for the setter.
> 
> Christopher Tubbs wrote:
>     Why is a setter needed if the getter is never used?
> 
> Josh Elser wrote:
>     Testing

Hmm, I see. It is low-risk, but it is confusing that it's not used... because it seems like it should be if it's there. If this were API, I'd definitely be in favor of keeping symmetric getters/setters, but for testing on an impl class... I'm not heartbroken about stripping it out.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/master/src/main/java/org/apache/accumulo/master/replication/DistributedWorkQueueWorkAssigner.java, line 71
> > <https://reviews.apache.org/r/30226/diff/1/?file=832094#file832094line71>
> >
> >     I would prefer to not nuke these. It's just going to be more work when I inevitably find bugs and need to write more tests.
> 
> Christopher Tubbs wrote:
>     Do you have any existing tests that could benefit from using it now? And, which is the greater risk? That there may be some extra effort needed later, or that the current unused method will become stale or broken until that time?
> 
> Josh Elser wrote:
>     Not having/using the getters/setters make me think that coverage on the tests could be improved. I don't see getters/setters going stale, and their hypothetical removal just causing more work later.

At the very least, they'd be at no risk of removal by me if they had tests. I'm still not clear on their intended future purpose... and think that if that work is in progress (even slow progress), maybe these extra methods should live in a branch for that feature, and should be added when they are needed.


> On Jan. 30, 2015, 1:21 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java, line 88
> > <https://reviews.apache.org/r/30226/diff/1/?file=832130#file832130line88>
> >
> >     Again, unnecessary removal of getters/setters IMO
> 
> Christopher Tubbs wrote:
>     It seems that if we don't use the getter, we don't need the setter either.
> 
> Josh Elser wrote:
>     Again, a sign that test coverage could/should be improved

Do any of these fall into what you consider API for replication? Not "public API", of course, but API in the sense that it's an essential function of the replication feature that could be leveraged to create an alternate implementation?

If so, I think: needs tests. If not, I think: delete.


- Christopher


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Josh Elser <jo...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70370
-----------------------------------------------------------



core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationClient.java
<https://reviews.apache.org/r/30226/#comment115485>

    I don't know what we want the policy to be here. No, this isn't used because we have no one-way thrift RPCs presently. This may change in the future.



core/src/main/java/org/apache/accumulo/core/replication/AccumuloReplicationReplayer.java
<https://reviews.apache.org/r/30226/#comment115529>

    This is stubbed out to eventually support bulk imports



examples/simple/src/main/java/org/apache/accumulo/examples/simple/filedata/ChunkInputStream.java
<https://reviews.apache.org/r/30226/#comment115491>

    Example code could have been used by users. We might not want to remove things here?



fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooLock.java
<https://reviews.apache.org/r/30226/#comment115492>

    This is a good one to remove since it's not safe for use anyways (the bare ZooKeeper object).



minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java
<https://reviews.apache.org/r/30226/#comment115493>

    I don't see the need to remove this. Would be happy to add a test so it's "used".



minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
<https://reviews.apache.org/r/30226/#comment115494>

    Should preserve the accompanying getter for the setter.



server/base/src/main/java/org/apache/accumulo/server/util/CleanZookeeper.java
<https://reviews.apache.org/r/30226/#comment115495>

    This would break users used this tool, despite the value not being used. I guess this is ok since it's a "server" util.



server/base/src/main/java/org/apache/accumulo/server/util/RestoreZookeeper.java
<https://reviews.apache.org/r/30226/#comment115496>

    Same here, might have been used by users.



server/master/src/main/java/org/apache/accumulo/master/replication/DistributedWorkQueueWorkAssigner.java
<https://reviews.apache.org/r/30226/#comment115497>

    I would prefer to not nuke these. It's just going to be more work when I inevitably find bugs and need to write more tests.



server/master/src/main/java/org/apache/accumulo/master/replication/WorkDriver.java
<https://reviews.apache.org/r/30226/#comment115498>

    Same here. Removing for the sake of saying we removed.



server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
<https://reviews.apache.org/r/30226/#comment115502>

    Does this really mean that we're just not displaying it on the monitor? I think we'd want to do that rather than just throw away these stats.



server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
<https://reviews.apache.org/r/30226/#comment115505>

    Is the online tablets in the monitor calculated some different way?



server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
<https://reviews.apache.org/r/30226/#comment115513>

    These looks kind of important. Are we not stopping these pools?



server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java
<https://reviews.apache.org/r/30226/#comment115515>

    Again, unnecessary removal of getters/setters IMO



server/tserver/src/main/java/org/apache/accumulo/tserver/replication/BatchWriterReplicationReplayer.java
<https://reviews.apache.org/r/30226/#comment115530>

    Stubbed out for replicating bulk loads


- Josh Elser


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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by ke...@deenlo.com.


> 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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> On Jan. 28, 2015, 1: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?
> 
> kturner wrote:
>     > Should it be?
>     
>     don't know.  Somehow tablets are flushed, maybe this method is directly called from clients?  Not sure.

Checking again, I cannot find any place the `flushTablet` method is being called, except in this unused method. There is a separate `flush` method, which is used by `MasterClientServiceHandler.waitForFlush(...)`. Clients must be calling that instead.


- 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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> 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.

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.


> On Jan. 28, 2015, 1:07 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/file/rfile/SplitLarge.java, line 82
> > <https://reviews.apache.org/r/30226/diff/1/?file=832037#file832037line82>
> >
> >     this is a nice catch

FWIW, all these were fixed in separate tickets, too, but I fixed them after I did this pass.


> On Jan. 28, 2015, 1:07 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionalMutationSet.java, line 40
> > <https://reviews.apache.org/r/30226/diff/1/?file=832105#file832105line40>
> >
> >     In addition to finding unused code, I suppose the tool helps you find code with a higher than needed visibility?

Yes. You could say "unused at the particular visibility level". I started to track some of those, but they were too numerous, so I stopped.


> On Jan. 28, 2015, 1:07 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java, line 511
> > <https://reviews.apache.org/r/30226/diff/1/?file=832110#file832110line511>
> >
> >     So the tool let you know this code was only used by test?

Yes. I believe this was caught by the higher-than-necessary visibility checks that I abandoned. I also replaced some comments with this annotation on other methods. I don't plan on committing those as part of this overall issue, though. If I do those changes, I'll do them separately.


> On Jan. 28, 2015, 1: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?

Will have to check. If this was the only method using that thrift method, then perhaps not. Should it be?


- 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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Josh Elser <jo...@gmail.com>.


> 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.

Testing is good example of why such a constructor is nice to have.


- Josh


-----------------------------------------------------------
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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by Christopher Tubbs <ct...@apache.org>.


> 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
> 
>

Re: Review Request 30226: ACCUMULO-3204 Unused code

Posted by ke...@deenlo.com.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30226/#review70025
-----------------------------------------------------------



core/src/main/java/org/apache/accumulo/core/file/rfile/SplitLarge.java
<https://reviews.apache.org/r/30226/#comment114952>

    this is a nice catch



core/src/main/java/org/apache/accumulo/core/iterators/DebugIterator.java
<https://reviews.apache.org/r/30226/#comment114953>

    I know its not officially public API, but we do encourage users to use Iterators.  This is something a user could have possible used.



server/base/src/main/java/org/apache/accumulo/server/master/LiveTServerSet.java
<https://reviews.apache.org/r/30226/#comment114954>

    Is this thrift method used?



server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java
<https://reviews.apache.org/r/30226/#comment114957>

    another nice catch



server/tserver/src/main/java/org/apache/accumulo/tserver/ConditionalMutationSet.java
<https://reviews.apache.org/r/30226/#comment114958>

    In addition to finding unused code, I suppose the tool helps you find code with a higher than needed visibility?



server/tserver/src/main/java/org/apache/accumulo/tserver/NativeMap.java
<https://reviews.apache.org/r/30226/#comment114959>

    So the tool let you know this code was only used by test?


- kturner


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
> 
>