accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Christopher Tubbs" <ctubbsii...@apache.org>
Subject Re: Review Request 30226: ACCUMULO-3204 Unused code
Date Fri, 17 Apr 2015 22:35:41 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message