HBASE-28927 Fix spotbugs issues introduced by refactoring to hbase-diagnostics with HBASE-28432 (#6384)

NihalJain · web-flow · commit f4aafaf4d509 · 2024-10-23T15:33:34.000+05:30
Signed-off-by: Istvan Toth &lt;stoty@apache.org&gt;
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/PerformanceEvaluation.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/PerformanceEvaluation.java
@@ -249,6 +249,22 @@ public String toString() {
     public int compareTo(RunResult o) {
       return Long.compare(this.duration, o.duration);
     }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (this == obj) {
+        return true;
+      }
+      if (obj == null || getClass() != obj.getClass()) {
+        return false;
+      }
+      return this.compareTo((RunResult) obj) == 0;
+    }
+
+    @Override
+    public int hashCode() {
+      return Long.hashCode(duration);
+    }
   }
 
   /**
@@ -3144,14 +3160,15 @@ static TestOptions calculateRowsAndSize(final TestOptions opts) {
         && (opts.getCmdName().equals(RANDOM_READ) || opts.getCmdName().equals(RANDOM_SEEK_SCAN)))
         && opts.size != DEFAULT_OPTS.size && opts.perClientRunRows != DEFAULT_OPTS.perClientRunRows
     ) {
-      opts.totalRows = (int) opts.size * rowsPerGB;
+      opts.totalRows = (int) (opts.size * rowsPerGB);
     } else if (opts.size != DEFAULT_OPTS.size) {
       // total size in GB specified
-      opts.totalRows = (int) opts.size * rowsPerGB;
+      opts.totalRows = (int) (opts.size * rowsPerGB);
       opts.perClientRunRows = opts.totalRows / opts.numClientThreads;
     } else {
       opts.totalRows = opts.perClientRunRows * opts.numClientThreads;
-      opts.size = opts.totalRows / rowsPerGB;
+      // Cast to float to ensure floating-point division
+      opts.size = (float) opts.totalRows / rowsPerGB;
     }
     return opts;
   }
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/LoadTestTool.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/LoadTestTool.java
@@ -164,7 +164,7 @@ public class LoadTestTool extends AbstractHBaseTool {
   public static final String OPT_NUM_REGIONS_PER_SERVER = "num_regions_per_server";
   protected static final String OPT_NUM_REGIONS_PER_SERVER_USAGE =
     "Desired number of regions per region server. Defaults to 5.";
-  public static int DEFAULT_NUM_REGIONS_PER_SERVER = 5;
+  public static final int DEFAULT_NUM_REGIONS_PER_SERVER = 5;
 
   public static final String OPT_REGION_REPLICATION = "region_replication";
   protected static final String OPT_REGION_REPLICATION_USAGE =
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedAction.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedAction.java
@@ -203,16 +203,16 @@ public void run() {
           long numKeysDelta = numKeys - priorNumKeys;
           long totalOpTimeDelta = totalOpTime - priorCumulativeOpTime;
 
-          double averageKeysPerSecond = (time > 0) ? (numKeys * 1000 / time) : 0;
+          double averageKeysPerSecond = (time > 0) ? (numKeys * 1000.0 / time) : 0;
 
           LOG.info(threadsLeft + "Keys=" + numKeys + ", cols="
             + StringUtils.humanReadableInt(numCols.get()) + ", time=" + formatTime(time)
             + ((numKeys > 0 && time > 0)
-              ? (" Overall: [" + "keys/s= " + numKeys * 1000 / time + ", latency="
+              ? (" Overall: [" + "keys/s= " + (numKeys * 1000.0 / time) + ", latency="
                 + String.format("%.2f", (double) totalOpTime / (double) numKeys) + " ms]")
               : "")
             + ((numKeysDelta > 0)
-              ? (" Current: [" + "keys/s=" + numKeysDelta * 1000 / REPORTING_INTERVAL_MS
+              ? (" Current: [" + "keys/s=" + (numKeysDelta * 1000.0 / REPORTING_INTERVAL_MS)
                 + ", latency="
                 + String.format("%.2f", (double) totalOpTimeDelta / (double) numKeysDelta) + " ms]")
               : "")
@@ -407,15 +407,15 @@ public boolean verifyResultAgainstDataGenerator(Result result, boolean verifyVal
           verifyCfAndColumnIntegrity
             && !dataGenerator.verify(result.getRow(), cf, columnValues.keySet())
         ) {
-          String colsStr = "";
+          StringBuilder colsStr = new StringBuilder();
           for (byte[] col : columnValues.keySet()) {
             if (colsStr.length() > 0) {
-              colsStr += ", ";
+              colsStr.append(", ");
             }
-            colsStr += "[" + Bytes.toString(col) + "]";
+            colsStr.append("[").append(Bytes.toString(col)).append("]");
           }
-          LOG.error("Error checking data for key [" + rowKeyStr + "], bad columns for family ["
-            + cfStr + "]: " + colsStr);
+          LOG.error("Error checking data for key [{}], bad columns for family [{}]: {}", rowKeyStr,
+            cfStr, colsStr.toString());
           printLocations(result);
           return false;
         }
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedReader.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedReader.java
@@ -307,15 +307,15 @@ private Get[] readKey(long[] keysToRead) {
 
     protected Get createGet(long keyToRead) throws IOException {
       Get get = new Get(dataGenerator.getDeterministicUniqueKey(keyToRead));
-      String cfsString = "";
+      StringBuilder cfsString = new StringBuilder();
       byte[][] columnFamilies = dataGenerator.getColumnFamilies();
       for (byte[] cf : columnFamilies) {
         get.addFamily(cf);
         if (verbose) {
           if (cfsString.length() > 0) {
-            cfsString += ", ";
+            cfsString.append(", ");
           }
-          cfsString += "[" + Bytes.toStringBinary(cf) + "]";
+          cfsString.append("[").append(Bytes.toStringBinary(cf)).append("]");
         }
       }
       get = dataGenerator.beforeGet(keyToRead, get);
@@ -324,7 +324,7 @@ protected Get createGet(long keyToRead) throws IOException {
         get.setConsistency(Consistency.TIMELINE);
       }
       if (verbose) {
-        LOG.info("[" + readerId + "] " + "Querying key " + keyToRead + ", cfs " + cfsString);
+        LOG.info("[{}] Querying key {}, cfs {}", readerId, keyToRead, cfsString.toString());
       }
       return get;
     }
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedReaderWithACL.java
@@ -88,8 +88,6 @@ protected void closeTable() {
     @Override
     public void queryKey(final Get get, final boolean verify, final long keyToRead)
       throws IOException {
-      final String rowKey = Bytes.toString(get.getRow());
-
       // read the data
       final long start = System.nanoTime();
       PrivilegedExceptionAction<Object> action = new PrivilegedExceptionAction<Object>() {
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedUpdater.java
@@ -76,7 +76,7 @@ public void setBatchUpdate(boolean isBatchUpdate) {
     this.isBatchUpdate = isBatchUpdate;
   }
 
-  public void linkToWriter(MultiThreadedWriterBase writer) {
+  public synchronized void linkToWriter(MultiThreadedWriterBase writer) {
     this.writer = writer;
     writer.setTrackWroteKeys(true);
   }
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedUpdaterWithACL.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/util/MultiThreadedUpdaterWithACL.java
@@ -75,8 +75,6 @@ protected void addUpdaterThreads(int numThreads) throws IOException {
   }
 
   public class HBaseUpdaterThreadWithACL extends HBaseUpdaterThread {
-
-    private Table table;
     private MutateAccessAction mutateAction = new MutateAccessAction();
 
     public HBaseUpdaterThreadWithACL(int updaterId) throws IOException {
diff --git a/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/wal/WALPerformanceEvaluation.java b/hbase-diagnostics/src/main/java/org/apache/hadoop/hbase/wal/WALPerformanceEvaluation.java
@@ -168,7 +168,7 @@ public void run() {
             loopSpan.end();
           }
         }
-      } catch (Exception e) {
+      } catch (IOException e) {
         LOG.error(getClass().getSimpleName() + " Thread failed", e);
       } finally {
         threadSpan.end();
@@ -241,7 +241,7 @@ public int run(String[] args) throws Exception {
           System.err.println("UNEXPECTED: " + cmd);
           printUsageAndExit();
         }
-      } catch (Exception e) {
+      } catch (NumberFormatException e) {
         printUsageAndExit();
       }
     }
@@ -411,7 +411,7 @@ private static void logBenchmarkResult(String testName, long numTests, long tota
   }
 
   private void printUsageAndExit() {
-    System.err.printf("Usage: hbase %s [options]\n", getClass().getName());
+    System.err.printf("Usage: hbase %s [options]%n", getClass().getName());
     System.err.println(" where [options] are:");
     System.err.println("  -h|-help         Show this help and exit.");
     System.err.println("  -threads <N>     Number of threads writing on the WAL.");
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/LoadTestKVGenerator.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/LoadTestKVGenerator.java
@@ -98,6 +98,7 @@ public byte[] generateRandomSizeValue(byte[]... seedStrings) {
    * Generates random bytes of the given size for the given row and column qualifier. The random
    * seed is fully determined by these parameters.
    */
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = "DMI_RANDOM_USED_ONLY_ONCE")
   private static byte[] getValueForRowColumn(int dataSize, byte[]... seedStrings) {
     long seed = dataSize;
     for (byte[] str : seedStrings) {

Original file line number	Diff line number	Diff line change
`@@ -307,15 +307,15 @@ private Get[] readKey(long[] keysToRead) {`
`307`	`307`
`308`	`308`	`protected Get createGet(long keyToRead) throws IOException {`
`309`	`309`	`Get get = new Get(dataGenerator.getDeterministicUniqueKey(keyToRead));`
`310`		`- String cfsString = "";`
	`310`	`+ StringBuilder cfsString = new StringBuilder();`
`311`	`311`	`byte[][] columnFamilies = dataGenerator.getColumnFamilies();`
`312`	`312`	`for (byte[] cf : columnFamilies) {`
`313`	`313`	`get.addFamily(cf);`
`314`	`314`	`if (verbose) {`
`315`	`315`	`if (cfsString.length() > 0) {`
`316`		`- cfsString += ", ";`
	`316`	`+ cfsString.append(", ");`
`317`	`317`	`}`
`318`		`- cfsString += "[" + Bytes.toStringBinary(cf) + "]";`
	`318`	`+ cfsString.append("[").append(Bytes.toStringBinary(cf)).append("]");`
`319`	`319`	`}`
`320`	`320`	`}`
`321`	`321`	`get = dataGenerator.beforeGet(keyToRead, get);`
`@@ -324,7 +324,7 @@ protected Get createGet(long keyToRead) throws IOException {`
`324`	`324`	`get.setConsistency(Consistency.TIMELINE);`
`325`	`325`	`}`
`326`	`326`	`if (verbose) {`
`327`		`- LOG.info("[" + readerId + "] " + "Querying key " + keyToRead + ", cfs " + cfsString);`
	`327`	`+ LOG.info("[{}] Querying key {}, cfs {}", readerId, keyToRead, cfsString.toString());`
`328`	`328`	`}`
`329`	`329`	`return get;`
`330`	`330`	`}`
Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,7 @@ public void setBatchUpdate(boolean isBatchUpdate) {`
`76`	`76`	`this.isBatchUpdate = isBatchUpdate;`
`77`	`77`	`}`
`78`	`78`
`79`		`- public void linkToWriter(MultiThreadedWriterBase writer) {`
	`79`	`+ public synchronized void linkToWriter(MultiThreadedWriterBase writer) {`
`80`	`80`	`this.writer = writer;`
`81`	`81`	`writer.setTrackWroteKeys(true);`
`82`	`82`	`}`
Original file line number	Diff line number	Diff line change
`@@ -75,8 +75,6 @@ protected void addUpdaterThreads(int numThreads) throws IOException {`
`75`	`75`	`}`
`76`	`76`
`77`	`77`	`public class HBaseUpdaterThreadWithACL extends HBaseUpdaterThread {`
`78`		`-`
`79`		`- private Table table;`
`80`	`78`	`private MutateAccessAction mutateAction = new MutateAccessAction();`
`81`	`79`
`82`	`80`	`public HBaseUpdaterThreadWithACL(int updaterId) throws IOException {`
Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ public void run() {`
`168`	`168`	`loopSpan.end();`
`169`	`169`	`}`
`170`	`170`	`}`
`171`		`- } catch (Exception e) {`
	`171`	`+ } catch (IOException e) {`
`172`	`172`	`LOG.error(getClass().getSimpleName() + " Thread failed", e);`
`173`	`173`	`} finally {`
`174`	`174`	`threadSpan.end();`
`@@ -241,7 +241,7 @@ public int run(String[] args) throws Exception {`
`241`	`241`	`System.err.println("UNEXPECTED: " + cmd);`
`242`	`242`	`printUsageAndExit();`
`243`	`243`	`}`
`244`		`- } catch (Exception e) {`
	`244`	`+ } catch (NumberFormatException e) {`
`245`	`245`	`printUsageAndExit();`
`246`	`246`	`}`
`247`	`247`	`}`
`@@ -411,7 +411,7 @@ private static void logBenchmarkResult(String testName, long numTests, long tota`
`411`	`411`	`}`
`412`	`412`
`413`	`413`	`private void printUsageAndExit() {`
`414`		`- System.err.printf("Usage: hbase %s [options]\n", getClass().getName());`
	`414`	`+ System.err.printf("Usage: hbase %s [options]%n", getClass().getName());`
`415`	`415`	`System.err.println(" where [options] are:");`
`416`	`416`	`System.err.println(" -h\|-help Show this help and exit.");`
`417`	`417`	`System.err.println(" -threads <N> Number of threads writing on the WAL.");`