HBASE-26555 Fix findbugs/spotbugs findings

hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/AssignmentVerificationReport.java

@@ -230,7 +230,7 @@ public void fillUp(TableName tableName, SnapshotOfRegionAssignmentFromMeta snaps
         this.maxDispersionScoreServerSet.clear();
         this.maxDispersionScoreServerSet.add(primaryRS);
         this.maxDispersionScore = dispersionScore;
-      } else if (dispersionScore == this.maxDispersionScore) {
+      } else if (dispersionScore >= this.maxDispersionScore) {
         this.maxDispersionScoreServerSet.add(primaryRS);
       }
 
@@ -248,7 +248,7 @@ public void fillUp(TableName tableName, SnapshotOfRegionAssignmentFromMeta snaps
         this.minDispersionScoreServerSet.clear();
         this.minDispersionScoreServerSet.add(primaryRS);
         this.minDispersionScore = dispersionScore;
-      } else if (dispersionScore == this.minDispersionScore) {
+      } else if (dispersionScore <= this.minDispersionScore) {
         this.minDispersionScoreServerSet.add(primaryRS);
       }
 
@@ -404,7 +404,7 @@ public void fillUpDispersion(TableName tableName,
         this.minDispersionScoreServerSet.clear();
         this.minDispersionScoreServerSet.add(primaryRS);
         this.minDispersionScore = dispersionScore;
-      } else if (dispersionScore == this.minDispersionScore) {
+      } else if (dispersionScore <= this.minDispersionScore) {
         this.minDispersionScoreServerSet.add(primaryRS);
       }
 

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestCoprocessorDescriptor.java

@@ -33,8 +33,6 @@
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.junit.rules.TestName;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 @Category({ MiscTests.class, SmallTests.class })
 public class TestCoprocessorDescriptor {
@@ -43,8 +41,6 @@ public class TestCoprocessorDescriptor {
   public static final HBaseClassTestRule CLASS_RULE =
     HBaseClassTestRule.forClass(TestCoprocessorDescriptor.class);
 
-  private static final Logger LOG = LoggerFactory.getLogger(TestCoprocessorDescriptor.class);
-
   @Rule
   public TestName name = new TestName();
 
@@ -71,7 +67,7 @@ public void testSetCoprocessor() throws IOException {
     List<CoprocessorDescriptor> cps = new ArrayList<>();
     for (String className : Arrays.asList("className0", "className1", "className2")) {
       String path = "path";
-      int priority = Math.abs(className.hashCode());
+      int priority = 100;
       String propertyValue = "propertyValue";
       cps.add(
         CoprocessorDescriptorBuilder.newBuilder(className).setJarPath(path).setPriority(priority)

hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestScan.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.client;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -106,7 +107,7 @@ public void testGetToScan() throws Exception {
     assertEquals(get.getColumnFamilyTimeRange().get(Bytes.toBytes("cf")).getMax(),
             scan.getColumnFamilyTimeRange().get(Bytes.toBytes("cf")).getMax());
     assertEquals(get.getReplicaId(), scan.getReplicaId());
-    assertEquals(get.getACL(), scan.getACL());
+    assertArrayEquals(get.getACL(), scan.getACL());
     assertEquals(get.getAuthorizations().getLabels(), scan.getAuthorizations().getLabels());
     assertEquals(get.getPriority(), scan.getPriority());
   }
@@ -257,9 +258,9 @@ public void testScanCopyConstructor() throws Exception {
     Scan scanCopy = new Scan(scan);
 
     // validate fields of copied scan object match with the original scan object
-    assertEquals(scan.getACL(), scanCopy.getACL());
+    assertArrayEquals(scan.getACL(), scanCopy.getACL());
     assertEquals(scan.getAllowPartialResults(), scanCopy.getAllowPartialResults());
-    assertEquals(scan.getAttribute("test_key"), scanCopy.getAttribute("test_key"));
+    assertArrayEquals(scan.getAttribute("test_key"), scanCopy.getAttribute("test_key"));
     assertEquals(scan.getAttributeSize(), scanCopy.getAttributeSize());
     assertEquals(scan.getAttributesMap(), scanCopy.getAttributesMap());
     assertEquals(scan.getAuthorizations().getLabels(), scanCopy.getAuthorizations().getLabels());
@@ -268,7 +269,7 @@ public void testScanCopyConstructor() throws Exception {
     assertEquals(scan.getCaching(), scanCopy.getCaching());
     assertEquals(scan.getConsistency(), scanCopy.getConsistency());
     assertEquals(scan.getFamilies().length, scanCopy.getFamilies().length);
-    assertEquals(scan.getFamilies()[0], scanCopy.getFamilies()[0]);
+    assertArrayEquals(scan.getFamilies()[0], scanCopy.getFamilies()[0]);
     assertEquals(scan.getFamilyMap(), scanCopy.getFamilyMap());
     assertEquals(scan.getFilter(), scanCopy.getFilter());
     assertEquals(scan.getId(), scanCopy.getId());
@@ -284,8 +285,8 @@ public void testScanCopyConstructor() throws Exception {
     assertEquals(scan.getReadType(), scanCopy.getReadType());
     assertEquals(scan.getReplicaId(), scanCopy.getReplicaId());
     assertEquals(scan.getRowOffsetPerColumnFamily(), scanCopy.getRowOffsetPerColumnFamily());
-    assertEquals(scan.getStartRow(), scanCopy.getStartRow());
-    assertEquals(scan.getStopRow(), scanCopy.getStopRow());
+    assertArrayEquals(scan.getStartRow(), scanCopy.getStartRow());
+    assertArrayEquals(scan.getStopRow(), scanCopy.getStopRow());
     assertEquals(scan.getTimeRange(), scanCopy.getTimeRange());
 
     assertTrue("Make sure copy constructor adds all the fields in the copied object",

hbase-client/src/test/java/org/apache/hadoop/hbase/security/token/TestClientTokenUtil.java

@@ -19,6 +19,7 @@
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
 
 import java.io.IOException;
 import java.lang.reflect.Field;
@@ -81,13 +82,13 @@ public void testObtainToken() throws Exception {
     shouldInjectFault.set(null, new ServiceException(injected));
 
     try {
-      ClientTokenUtil.obtainToken((Connection)null);
+      ClientTokenUtil.obtainToken(mock(Connection.class));
       fail("Should have injected exception.");
     } catch (IOException e) {
       assertException(injected, e);
     }
 
-    CompletableFuture<?> future = ClientTokenUtil.obtainToken((AsyncConnection)null);
+    CompletableFuture<?> future = ClientTokenUtil.obtainToken(mock(AsyncConnection.class));
     try {
       future.get();
       fail("Should have injected exception.");

hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestRoundRobinPoolMap.java

@@ -25,11 +25,9 @@
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Random;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.testclassification.MiscTests;

hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestThreadLocalPoolMap.java

@@ -20,11 +20,9 @@
 import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
-import java.util.Random;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.testclassification.MiscTests;

hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.nio;
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -153,7 +154,7 @@ public void testArrayBasedMethods() {
     mbb1 = new SingleByteBuff(bb1);
     assertTrue(mbb1.hasArray());
     assertEquals(1, mbb1.arrayOffset());
-    assertEquals(b, mbb1.array());
+    assertArrayEquals(b, mbb1.array());
     mbb1 = new SingleByteBuff(ByteBuffer.allocateDirect(10));
     assertFalse(mbb1.hasArray());
     try {

hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestSimpleKdcServerUtil.java

@@ -49,7 +49,9 @@ public void testBindException() throws KrbException, IOException {
         getRunningSimpleKdcServer(dir, HBaseCommonTestingUtil::randomFreePort, true);
       kdc.createPrincipal("wah");
     } finally {
-      kdc.stop();
+      if (kdc != null) {
+        kdc.stop();
+      }
     }
   }
 }

hbase-http/src/test/java/org/apache/hadoop/hbase/http/HttpServerFunctionalTest.java

@@ -260,12 +260,15 @@ protected static String readOutput(URL url) throws IOException {
    */
   protected static void deleteRecursively(File d) {
     if (d.isDirectory()) {
-      for (String name : d.list()) {
-        File child = new File(d, name);
-        if (child.isFile()) {
-          child.delete();
-        } else {
-          deleteRecursively(child);
+      String[] names = d.list();
+      if (names != null) {
+        for (String name: names) {
+          File child = new File(d, name);
+          if (child.isFile()) {
+            child.delete();
+          } else {
+            deleteRecursively(child);
+          }
         }
       }
     }

hbase-it/src/test/java/org/apache/hadoop/hbase/TestIntegrationTestBase.java

@@ -40,11 +40,10 @@ public void testMonkeyPropertiesParsing() {
     conf.set(MonkeyConstants.BATCH_RESTART_RS_RATIO, "0.85");
     conf.set(MonkeyConstants.MOVE_REGIONS_MAX_TIME, "60000");
     conf.set("hbase.rootdir", "/foo/bar/baz");
-
     final Properties props = new Properties();
-    IntegrationTestBase testBase = new IntegrationTestDDLMasterFailover();
     assertEquals(0, props.size());
-    testBase.loadMonkeyProperties(props, conf);
+    new IntegrationTestDDLMasterFailover();
+    IntegrationTestBase.loadMonkeyProperties(props, conf);
     assertEquals(2, props.size());
     assertEquals("0.85", props.getProperty(MonkeyConstants.BATCH_RESTART_RS_RATIO));
     assertEquals("60000", props.getProperty(MonkeyConstants.MOVE_REGIONS_MAX_TIME));

hbase-it/src/test/java/org/apache/hadoop/hbase/ipc/IntegrationTestRpcClient.java

@@ -136,15 +136,14 @@ void stopRandomServer() throws Exception {
         int size = rpcServers.size();
         int rand = random.nextInt(size);
         rpcServer = serverList.remove(rand);
-        InetSocketAddress address = rpcServer.getListenerAddress();
-        if (address == null) {
-          // Throw exception here. We can't remove this instance from the server map because
-          // we no longer have access to its map key
-          throw new IOException("Listener channel is closed");
-        }
-        rpcServers.remove(address);
-
         if (rpcServer != null) {
+          InetSocketAddress address = rpcServer.getListenerAddress();
+          if (address == null) {
+            // Throw exception here. We can't remove this instance from the server map because
+            // we no longer have access to its map key
+            throw new IOException("Listener channel is closed");
+          }
+          rpcServers.remove(address);
           stopServer(rpcServer);
         }
       } finally {

hbase-it/src/test/java/org/apache/hadoop/hbase/mttr/IntegrationTestMTTR.java

@@ -133,7 +133,7 @@ public class IntegrationTestMTTR {
   /**
    * Util to get at the cluster.
    */
-  private static IntegrationTestingUtility util;
+  private static final IntegrationTestingUtility util = new IntegrationTestingUtility();
 
   /**
    * Executor for test threads.
@@ -157,10 +157,6 @@ public class IntegrationTestMTTR {
 
   @BeforeClass
   public static void setUp() throws Exception {
-    // Set up the integration test util
-    if (util == null) {
-      util = new IntegrationTestingUtility();
-    }
 
     // Make sure there are three servers.
     util.initializeCluster(3);
@@ -251,7 +247,6 @@ private static void setupTables() throws IOException {
   public static void after() throws IOException {
     // Clean everything up.
     util.restoreCluster();
-    util = null;
 
     // Stop the threads so that we know everything is complete.
     executorService.shutdown();

hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestBigLinkedList.java

@@ -1327,15 +1327,15 @@ private StringBuilder dumpExtraInfoOnRefs(final BytesWritable key, final Context
                 hrl = rl.getRegionLocation(key.getBytes());
                 if (hrl != null) keyRegionLocation = hrl.toString();
               }
+              LOG.error("Extras on ref without a def, ref=" + Bytes.toStringBinary(ref) +
+                ", refPrevEqualsKey=" +
+                  (Bytes.compareTo(key.getBytes(), 0, key.getLength(), b, 0, b.length) == 0) +
+                  ", key=" + Bytes.toStringBinary(key.getBytes(), 0, key.getLength()) +
+                  ", ref row date=" + ts + ", jobStr=" + jobStr +
+                  ", ref row count=" + count +
+                  ", ref row regionLocation=" + refRegionLocation +
+                  ", key row regionLocation=" + keyRegionLocation);
             }
-            LOG.error("Extras on ref without a def, ref=" + Bytes.toStringBinary(ref) +
-              ", refPrevEqualsKey=" +
-                (Bytes.compareTo(key.getBytes(), 0, key.getLength(), b, 0, b.length) == 0) +
-                ", key=" + Bytes.toStringBinary(key.getBytes(), 0, key.getLength()) +
-                ", ref row date=" + ts + ", jobStr=" + jobStr +
-                ", ref row count=" + count +
-                ", ref row regionLocation=" + refRegionLocation +
-                ", key row regionLocation=" + keyRegionLocation);
             refsSb.append(comma);
             comma = ",";
             refsSb.append(Bytes.toStringBinary(ref));

hbase-it/src/test/java/org/apache/hadoop/hbase/test/IntegrationTestReplication.java

@@ -21,6 +21,7 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.Configured;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.HBaseCommonTestingUtil;
 import org.apache.hadoop.hbase.HBaseConfiguration;
 import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.IntegrationTestingUtility;
@@ -155,9 +156,16 @@ public void closeConnection() throws Exception {
       this.connection = null;
     }
 
-    public boolean equals(ClusterID other) {
+    public boolean equals(Object other) {
+      if (other == null) {
+        return false;
+      }
       return this.toString().equalsIgnoreCase(other.toString());
     }
+
+    public int hashCode() {
+      return this.toString().hashCode();
+    }
   }
 
   /**
@@ -263,7 +271,7 @@ protected void waitForReplication() throws Exception {
      */
     protected void runGenerator() throws Exception {
       Path outputPath = new Path(outputDir);
-      UUID uuid = util.getRandomUUID(); //create a random UUID.
+      UUID uuid = HBaseCommonTestingUtil.getRandomUUID(); //create a random UUID.
       Path generatorOutput = new Path(outputPath, uuid.toString());
 
       Generator generator = new Generator();
@@ -287,7 +295,7 @@ protected void runGenerator() throws Exception {
      */
     protected void runVerify(long expectedNumNodes) throws Exception {
       Path outputPath = new Path(outputDir);
-      UUID uuid = util.getRandomUUID(); //create a random UUID.
+      UUID uuid = HBaseCommonTestingUtil.getRandomUUID(); //create a random UUID.
       Path iterationOutput = new Path(outputPath, uuid.toString());
 
       Verify verify = new Verify();

hbase-logging/src/test/java/org/apache/log4j/FileAppender.java

@@ -126,7 +126,7 @@ public boolean getAppend() {
   }
 
   /** Returns the value of the <b>File</b> option. */
-  public String getFile() {
+  public synchronized String getFile() {
     return fileName;
   }
 

hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestSyncTable.java

@@ -314,7 +314,7 @@ private void assertTargetDoDeletesFalse(int expectedRows, TableName sourceTableN
           if (!CellUtil.matchingQualifier(sourceCell, targetCell)) {
             Assert.fail("Qualifiers don't match");
           }
-          if (targetRowKey < 80 && targetRowKey >= 90){
+          if (targetRowKey < 80 || targetRowKey >= 90){
             if (!CellUtil.matchingTimestamp(sourceCell, targetCell)) {
               Assert.fail("Timestamps don't match");
             }

hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestTableSnapshotInputFormat.java

@@ -100,7 +100,7 @@ protected byte[] getEndRow() {
 
   @Test
   public void testGetBestLocations() throws IOException {
-    TableSnapshotInputFormatImpl tsif = new TableSnapshotInputFormatImpl();
+    new TableSnapshotInputFormatImpl();
     Configuration conf = UTIL.getConfiguration();
 
     HDFSBlocksDistribution blockDistribution = new HDFSBlocksDistribution();

hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureInMemoryChore.java

@@ -19,6 +19,7 @@
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.IOException;
 import java.util.concurrent.CountDownLatch;
@@ -88,8 +89,8 @@ public void testChoreAddAndRemove() throws Exception {
     procExecutor.removeChore(chore);
     latch = new CountDownLatch(nCountDown);
     chore.setLatch(latch);
-    latch.await(timeoutMSec * nCountDown, TimeUnit.MILLISECONDS);
-    LOG.info("chore latch count=" + latch.getCount());
+    boolean reached = latch.await(timeoutMSec * nCountDown, TimeUnit.MILLISECONDS);
+    LOG.info("chore latch reached={} count={}", reached, latch.getCount());
     assertFalse(chore.isWaiting());
     assertTrue("latchCount=" + latch.getCount(), latch.getCount() > 0);
   }