Project: Apache HBase - RSGroup
FindBugs version: 3.0.0
Code analyzed:
1482 lines of code analyzed, in 13 classes, in 1 packages.
| Metric | Total | Density* |
|---|---|---|
| High Priority Warnings | 1 | 0.67 |
| Medium Priority Warnings | 5 | 3.37 |
| Total Warnings | 6 | 4.05 |
(* Defects per Thousand lines of non-commenting source statements)
| Warning Type | Number |
|---|---|
| Multithreaded correctness Warnings | 2 |
| Performance Warnings | 2 |
| Dodgy code Warnings | 2 |
| Total | 6 |
Click on a warning row to see full context information.
| Code | Warning |
|---|---|
| SC | new org.apache.hadoop.hbase.rsgroup.RSGroupInfoManagerImpl(MasterServices) invokes org.apache.hadoop.hbase.rsgroup.RSGroupInfoManagerImpl$RSGroupStartupWorker.start() |
| SWL | org.apache.hadoop.hbase.rsgroup.RSGroupAdminServer.moveServers(Set, String) calls Thread.sleep() with a lock held |
| Code | Warning |
|---|---|
| WMI | org.apache.hadoop.hbase.rsgroup.RSGroupBasedLoadBalancer.correctAssignments(Map) makes inefficient use of keySet iterator instead of entrySet iterator |
| WMI | org.apache.hadoop.hbase.rsgroup.RSGroupBasedLoadBalancer.getMisplacedRegions(Map) makes inefficient use of keySet iterator instead of entrySet iterator |
| Code | Warning |
|---|---|
| REC | Exception is caught when Exception is not thrown in org.apache.hadoop.hbase.rsgroup.RSGroupInfoManagerImpl$RSGroupStartupWorker.waitForGroupTableOnline() |
| ST | Write to static field org.apache.hadoop.hbase.rsgroup.RSGroupAdminEndpoint.groupInfoManager from instance method org.apache.hadoop.hbase.rsgroup.RSGroupAdminEndpoint.start(CoprocessorEnvironment) |
This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.
A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:
try {
...
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
... deal with all non-runtime exceptions ...
}
The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.
This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.
This method calls Thread.sleep() with a lock held. This may result in very poor performance and scalability, or a deadlock, since other threads may be waiting to acquire the lock. It is a much better idea to call wait() on the lock, which releases the lock and allows other threads to run.
This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.