From 0561312bc489ee96416a0bb2035d7713516eb222 Mon Sep 17 00:00:00 2001
From: Peter Somogyi <psomogyi@cloudera.com>
Date: Wed, 17 Jan 2018 09:47:59 +0100
Subject: [PATCH] HBASE-19809 Fix findbugs and error-prone warnings in
 hbase-procedure (branch-2)

---
 .../hbase/procedure2/RemoteProcedureDispatcher.java    |  3 +++
 .../store/wal/ProcedureWALPrettyPrinter.java           |  1 +
 .../apache/hadoop/hbase/procedure2/util/ByteSlot.java  |  2 ++
 .../hadoop/hbase/procedure2/TestProcedureEvents.java   |  2 +-
 .../hadoop/hbase/procedure2/TestProcedureExecutor.java |  2 +-
 .../hbase/procedure2/TestProcedureInMemoryChore.java   | 10 +++++-----
 .../hbase/procedure2/TestProcedureReplayOrder.java     |  5 +++--
 .../hbase/procedure2/TestStateMachineProcedure.java    |  8 +++++++-
 .../hadoop/hbase/procedure2/TestYieldProcedures.java   |  2 +-
 .../wal/ProcedureWALLoaderPerformanceEvaluation.java   |  5 ++---
 .../store/wal/ProcedureWALPerformanceEvaluation.java   |  2 +-
 11 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/RemoteProcedureDispatcher.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/RemoteProcedureDispatcher.java
index 71932b85009..72a2ebba79f 100644
--- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/RemoteProcedureDispatcher.java
+++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/RemoteProcedureDispatcher.java
@@ -331,10 +331,12 @@ public abstract class RemoteProcedureDispatcher<TEnv, TRemote extends Comparable
       super(key, 0);
     }
 
+    @Override
     public TRemote getKey() {
       return getObject();
     }
 
+    @Override
     public synchronized void add(final RemoteProcedure operation) {
       if (this.operations == null) {
         this.operations = new HashSet<>();
@@ -348,6 +350,7 @@ public abstract class RemoteProcedureDispatcher<TEnv, TRemote extends Comparable
       }
     }
 
+    @Override
     public synchronized void dispatch() {
       if (operations != null) {
         remoteDispatch(getKey(), operations);
diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPrettyPrinter.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPrettyPrinter.java
index eb8994b4741..3f007c611df 100644
--- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPrettyPrinter.java
+++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPrettyPrinter.java
@@ -149,6 +149,7 @@ public class ProcedureWALPrettyPrinter extends Configured implements Tool {
    * @throws IOException
    *           Thrown upon file system errors etc.
    */
+  @Override
   public int run(final String[] args) throws IOException {
     // create options
     Options options = new Options();
diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/util/ByteSlot.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/util/ByteSlot.java
index 068a823bef7..3e95de56f25 100644
--- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/util/ByteSlot.java
+++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/util/ByteSlot.java
@@ -81,11 +81,13 @@ public class ByteSlot extends OutputStream {
     buf[offset] = (byte)b;
   }
 
+  @Override
   public void write(int b) {
     ensureCapacity(size + 1);
     buf[size++] = (byte)b;
   }
 
+  @Override
   public void write(byte[] b, int off, int len) {
     ensureCapacity(size + len);
     System.arraycopy(b, off, buf, size, len);
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureEvents.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureEvents.java
index a0afe707680..f36da823599 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureEvents.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureEvents.java
@@ -160,7 +160,7 @@ public class TestProcedureEvents {
     }
 
     @Override
-    protected boolean setTimeoutFailure(final TestProcEnv env) {
+    protected synchronized boolean setTimeoutFailure(final TestProcEnv env) {
       int n = ntimeouts.incrementAndGet();
       LOG.info("HANDLE TIMEOUT " + this + " ntimeouts=" + n);
       setState(ProcedureState.RUNNABLE);
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureExecutor.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureExecutor.java
index 29a0472a452..e5b50707722 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureExecutor.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureExecutor.java
@@ -186,5 +186,5 @@ public class TestProcedureExecutor {
     }
   }
 
-  private class TestProcEnv { }
+  private static class TestProcEnv { }
 }
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureInMemoryChore.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureInMemoryChore.java
index 6546ea3c602..33d93d548fa 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureInMemoryChore.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureInMemoryChore.java
@@ -18,14 +18,17 @@
 
 package org.apache.hadoop.hbase.procedure2;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import java.io.IOException;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.hadoop.hbase.HBaseCommonTestingUtility;
 import org.apache.hadoop.hbase.procedure2.store.NoopProcedureStore;
-import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -33,10 +36,6 @@ import org.junit.experimental.categories.Category;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 @Category({MasterTests.class, SmallTests.class})
 public class TestProcedureInMemoryChore {
   private static final Logger LOG = LoggerFactory.getLogger(TestProcedureInMemoryChore.class);
@@ -49,6 +48,7 @@ public class TestProcedureInMemoryChore {
 
   private HBaseCommonTestingUtility htu;
 
+  @SuppressWarnings("rawtypes")
   @Before
   public void setUp() throws IOException {
     htu = new HBaseCommonTestingUtility();
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureReplayOrder.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureReplayOrder.java
index dc359a15a6a..821901ca980 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureReplayOrder.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureReplayOrder.java
@@ -132,8 +132,9 @@ public class TestProcedureReplayOrder {
         public void run() {
           for (int i = 0; i < nprocPerThread; ++i) {
             try {
-              procExecutor.submitProcedure((Procedure)procClazz.newInstance());
-            } catch (InstantiationException|IllegalAccessException e) {
+              procExecutor.submitProcedure((Procedure)
+                procClazz.getDeclaredConstructor().newInstance());
+            } catch (Exception e) {
               LOG.error("unable to instantiate the procedure", e);
               fail("failure during the proc.newInstance(): " + e.getMessage());
             }
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java
index f304ba72300..fdaa65eb2b9 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestStateMachineProcedure.java
@@ -152,6 +152,7 @@ public class TestStateMachineProcedure {
   public enum TestSMProcedureState { STEP_1, STEP_2 };
   public static class TestSMProcedure
       extends StateMachineProcedure<TestProcEnv, TestSMProcedureState> {
+    @Override
     protected Flow executeFromState(TestProcEnv env, TestSMProcedureState state) {
       LOG.info("EXEC " + state + " " + this);
       env.execCount.incrementAndGet();
@@ -168,25 +169,30 @@ public class TestStateMachineProcedure {
       return Flow.HAS_MORE_STATE;
     }
 
+    @Override
     protected void rollbackState(TestProcEnv env, TestSMProcedureState state) {
       LOG.info("ROLLBACK " + state + " " + this);
       env.rollbackCount.incrementAndGet();
     }
 
+    @Override
     protected TestSMProcedureState getState(int stateId) {
       return TestSMProcedureState.values()[stateId];
     }
 
+    @Override
     protected int getStateId(TestSMProcedureState state) {
       return state.ordinal();
     }
 
+    @Override
     protected TestSMProcedureState getInitialState() {
       return TestSMProcedureState.STEP_1;
     }
   }
 
   public static class SimpleChildProcedure extends NoopProcedure<TestProcEnv> {
+    @Override
     protected Procedure[] execute(TestProcEnv env) {
       LOG.info("EXEC " + this);
       env.execCount.incrementAndGet();
@@ -203,7 +209,7 @@ public class TestStateMachineProcedure {
     }
   }
 
-  public class TestProcEnv {
+  public static class TestProcEnv {
     AtomicInteger execCount = new AtomicInteger(0);
     AtomicInteger rollbackCount = new AtomicInteger(0);
     boolean triggerChildRollback = false;
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java
index 202353526aa..e6fc4bbd8a5 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestYieldProcedures.java
@@ -186,7 +186,7 @@ public class TestYieldProcedures {
       extends StateMachineProcedure<TestProcEnv, TestStateMachineProcedure.State> {
     enum State { STATE_1, STATE_2, STATE_3 }
 
-    public class ExecutionInfo {
+    public static class ExecutionInfo {
       private final boolean rollback;
       private final long timestamp;
       private final State step;
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALLoaderPerformanceEvaluation.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALLoaderPerformanceEvaluation.java
index 503850d3aa9..3c908fd0084 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALLoaderPerformanceEvaluation.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALLoaderPerformanceEvaluation.java
@@ -70,7 +70,7 @@ public class ProcedureWALLoaderPerformanceEvaluation extends AbstractHBaseTool {
   private WALProcedureStore store;
   static byte[] serializedState;
 
-  private class LoadCounter implements ProcedureStore.ProcedureLoader {
+  private static class LoadCounter implements ProcedureStore.ProcedureLoader {
     public LoadCounter() {}
 
     @Override
@@ -165,8 +165,7 @@ public class ProcedureWALLoaderPerformanceEvaluation extends AbstractHBaseTool {
   private void writeWals() throws IOException {
     List<Integer> procStates = shuffleProcWriteSequence();
     TestProcedure[] procs = new TestProcedure[numProcs + 1];  // 0 is not used.
-    int numProcsPerWal = numWals > 0 ? (int)Math.ceil(procStates.size() / numWals)
-        : Integer.MAX_VALUE;
+    int numProcsPerWal = numWals > 0 ? procStates.size() / numWals : Integer.MAX_VALUE;
     long startTime = currentTimeMillis();
     long lastTime = startTime;
     for (int i = 0; i < procStates.size(); ++i) {
diff --git a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPerformanceEvaluation.java b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPerformanceEvaluation.java
index 1a7fc800c09..12ed930fcce 100644
--- a/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPerformanceEvaluation.java
+++ b/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/store/wal/ProcedureWALPerformanceEvaluation.java
@@ -243,7 +243,7 @@ public class ProcedureWALPerformanceEvaluation extends AbstractHBaseTool {
     }
   }
 
-  private class NoSyncWalProcedureStore extends WALProcedureStore {
+  private static class NoSyncWalProcedureStore extends WALProcedureStore {
     public NoSyncWalProcedureStore(final Configuration conf, final Path logDir) throws IOException {
       super(conf, logDir, null, new WALProcedureStore.LeaseRecovery() {
         @Override