Fix Spotbugs check failed (apache#3836)

* fix spotbugs check failed

* format code

* install the package first

* upgrade spotbugs version to 4.7.3.2

* upgrade javadoc plugin to 3.5.0

* upgrade javadoc plugin to 3.2.0

* format code

* set maven version 3.8.7

* set all test maven version to 3.8.7

* fix failed CI

* add test failure

* address test failure

* fix failed CI

* format code

Author: hangc0276

.github/workflows/bk-ci.yml

@@ -79,9 +79,16 @@ jobs:
           distribution: 'temurin'
           java-version: 11
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: Validate pull request
         if: steps.check_changes.outputs.docs_only != 'true'
-        run: mvn clean -T 1C -B -nsu apache-rat:check checkstyle:check spotbugs:check package -Ddistributedlog -DskipTests -Dorg.slf4j.simpleLogger.defaultLogLevel=INFO
+        run: |
+          mvn -T 1C -B -nsu clean install -Ddistributedlog -DskipTests -Dorg.slf4j.simpleLogger.defaultLogLevel=INFO
+          mvn -T 1C -B -nsu apache-rat:check checkstyle:check spotbugs:check package -Ddistributedlog -DskipTests -Dorg.slf4j.simpleLogger.defaultLogLevel=INFO
 
       - name: Check license files
         if: steps.check_changes.outputs.docs_only != 'true'
@@ -152,6 +159,11 @@ jobs:
           distribution: 'temurin'
           java-version: 11
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: Build
         run: |
           projects_list=
@@ -223,6 +235,11 @@ jobs:
           distribution: 'temurin'
           java-version: 11
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: Build with Maven
         run: mvn -B -nsu clean install -Pdocker -DskipTests -Dorg.slf4j.simpleLogger.defaultLogLevel=INFO
 
@@ -265,6 +282,11 @@ jobs:
           distribution: 'temurin'
           java-version: 8
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: Build with Maven
         run: mvn -B -nsu clean install -DskipTests -Dorg.slf4j.simpleLogger.defaultLogLevel=INFO
 
@@ -311,6 +333,11 @@ jobs:
           distribution: 'temurin'
           java-version: 11
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: mvn package
         run: mvn -B -nsu clean package -DskipTests
 
@@ -343,6 +370,11 @@ jobs:
           distribution: 'temurin'
           java-version: 11
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: mvn package
         run: mvn -B -nsu clean package -DskipTests
 
@@ -387,6 +419,11 @@ jobs:
           distribution: 'temurin'
           java-version: ${{ matrix.jdk_version }}
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: Build with Maven
         run: mvn clean package -B -nsu -DskipBookKeeperServerTests -Dorg.slf4j.simpleLogger.defaultLogLevel=INFO
 
@@ -422,6 +459,11 @@ jobs:
         with:
           java-version: 11
 
+      - name: Set up Maven
+        uses: apache/pulsar-test-infra/setup-maven@master
+        with:
+          maven-version: 3.8.7
+
       - name: run "clean install verify" to trigger dependency check
         # excluding dlfs because it includes hadoop lib with
         # CVEs that we cannot patch up anyways

bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java

@@ -23,6 +23,7 @@
 import static org.apache.bookkeeper.proto.BookieProtocol.FLAG_RECOVERY_ADD;
 
 import com.google.common.collect.ImmutableMap;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import io.netty.buffer.ByteBuf;
 import io.netty.util.Recycler;
 import io.netty.util.Recycler.Handle;
@@ -80,6 +81,7 @@ class PendingAddOp implements WriteCallback {
     boolean allowFailFast = false;
     List<BookieId> ensemble;
 
+    @SuppressFBWarnings("IS2_INCONSISTENT_SYNC")
     static PendingAddOp create(LedgerHandle lh, ClientContext clientCtx,
                                List<BookieId> ensemble,
                                ByteBuf payload, EnumSet<WriteFlag> writeFlags,

bookkeeper-server/src/main/java/org/apache/bookkeeper/util/PageCacheUtil.java

@@ -95,7 +95,7 @@ public static void bestEffortRemoveFromPageCache(int fd, long offset, long len)
         }
         try {
             NATIVE_IO.posix_fadvise(fd, offset, len, POSIX_FADV_DONTNEED);
-        } catch (Exception e) {
+        } catch (Throwable e) {
             log.warn("Failed to perform posix_fadvise: {}", e.getMessage());
             fadvisePossible = false;
         }

bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java

@@ -34,6 +34,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -797,13 +798,17 @@ public void testSortedLedgerFlushFailure() throws Exception {
         // after flush failure, the bookie is set to readOnly
         assertTrue("Bookie is expected to be in Read mode", bookie.isReadOnly());
         // write fail
+        CountDownLatch latch = new CountDownLatch(1);
         bookie.addEntry(generateEntry(1, 3), false, new BookkeeperInternalCallbacks.WriteCallback(){
             public void writeComplete(int rc, long ledgerId, long entryId, BookieId addr, Object ctx){
-                LOG.info("fail write to bk");
-                assertTrue(rc != OK);
+                LOG.info("Write to bk succeed due to the bookie readOnly mode check is in the request parse step. "
+                    + "In the addEntry step, we won't check bookie's mode");
+                assertEquals(OK, rc);
+                latch.countDown();
             }
 
         }, null, "passwd".getBytes());
+        latch.await();
         bookie.shutdown();
 
     }

buildtools/src/main/resources/bookkeeper/findbugsExclude.xml

@@ -317,6 +317,11 @@
     <!-- generated code, we can't be held responsible for findbugs in it //-->
     <Class name="~.*\.TransformedRecord" />
   </Match>
+  <!-- micro-benchmarks -->
+  <Match>
+    <!-- generated code, we can't be held responsible for findbugs in it //-->
+    <Class name="~org\.apache\.bookkeeper\.bookie\.generated.*" />
+  </Match>
   <Match>
     <!-- it is safe to store external bytes reference here. //-->
     <Class name="org.apache.distributedlog.messaging.PartitionedMultiWriter" />

microbenchmarks/src/main/java/org/apache/bookkeeper/bookie/GroupSortBenchmark.java

@@ -23,6 +23,7 @@
 
 import java.util.Arrays;
 import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
 import org.apache.bookkeeper.bookie.storage.ldb.ArrayGroupSort;
 import org.openjdk.jmh.annotations.Benchmark;
@@ -55,7 +56,7 @@ public static class TestState {
         private long[] items;
 
         public TestState() {
-            Random r = new Random();
+            Random r = ThreadLocalRandom.current();
             for (int i = 0; i < (N * 4); i++) {
                 randomItems[i] = r.nextLong();
             }

pom.xml

@@ -205,7 +205,7 @@
     <os-maven-plugin.version>1.4.1.Final</os-maven-plugin.version>
     <protobuf-maven-plugin.version>0.6.1</protobuf-maven-plugin.version>
     <puppycrawl.checkstyle.version>9.3</puppycrawl.checkstyle.version>
-    <spotbugs-maven-plugin.version>4.6.0.0</spotbugs-maven-plugin.version>
+    <spotbugs-maven-plugin.version>4.7.3.2</spotbugs-maven-plugin.version>
     <forkCount.variable>1</forkCount.variable>
     <servlet-api.version>4.0.0</servlet-api.version>
     <rxjava.version>3.0.1</rxjava.version>