[incubator-uniffle] branch master updated: [SpotBugs] Set threshold to middle with exceptions (#517)

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xianjin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


The following commit(s) were added to refs/heads/master by this push:
     new b80972c2 [SpotBugs] Set threshold to middle with exceptions (#517)
b80972c2 is described below

commit b80972c226a3afd5568555a9a76fe3bb70ed3d9b
Author: Kaijie Chen <ck...@apache.org>
AuthorDate: Mon Jan 30 15:12:01 2023 +0800

    [SpotBugs] Set threshold to middle with exceptions (#517)
    
    ### What changes were proposed in this pull request?
    1. Enable middle priority checks in spotbugs.
    2. Fix some violations.
    3. Exclude other violations to make CI pass.
    
    ### Why are the changes needed?
    Enforce stronger code quality check.
    See violations fixed in this PR for example.
    
    ### Does this PR introduce _any_ user-facing change?
    No.
    
    ### How was this patch tested?
    Run `mvn test-compile spotbugs:check` manually.
---
 pom.xml                                                           | 2 +-
 .../main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java | 3 ++-
 spotbugs-exclude.xml                                              | 3 +++
 .../storage/handler/impl/PooledHdfsShuffleWriteHandler.java       | 8 ++++++--
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/pom.xml b/pom.xml
index eeab2547..50b6a675 100644
--- a/pom.xml
+++ b/pom.xml
@@ -861,7 +861,7 @@
           </dependency>
         </dependencies>
         <configuration>
-          <threshold>high</threshold>
+          <threshold>middle</threshold>
           <excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
         </configuration>
       </plugin>
diff --git a/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java b/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java
index cdd55629..dc305c67 100644
--- a/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java
+++ b/server/src/main/java/org/apache/uniffle/server/buffer/ShuffleBuffer.java
@@ -17,6 +17,7 @@
 
 package org.apache.uniffle.server.buffer;
 
+import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -91,7 +92,7 @@ public class ShuffleBuffer {
        * So we should create a reference copy to avoid this.
        */
       inFlushedQueueBlocks = new LinkedList<>(spBlocks);
-      spBlocks.sort((o1, o2) -> new Long(o1.getTaskAttemptId()).compareTo(o2.getTaskAttemptId()));
+      spBlocks.sort(Comparator.comparingLong(ShufflePartitionedBlock::getTaskAttemptId));
     }
     long eventId = ShuffleFlushManager.ATOMIC_EVENT_ID.getAndIncrement();
     final ShuffleDataFlushEvent event = new ShuffleDataFlushEvent(
diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml
index 268fd6ff..bead73be 100644
--- a/spotbugs-exclude.xml
+++ b/spotbugs-exclude.xml
@@ -19,4 +19,7 @@
   <Match>
     <Package name="~org\.apache\.uniffle\.proto.*"/>
   </Match>
+  <Match>
+    <Bug pattern="MS_EXPOSE_REP,EI_EXPOSE_REP,EI_EXPOSE_REP2,EI_EXPOSE_STATIC_REP2,THROWS_METHOD_THROWS_RUNTIMEEXCEPTION,THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION,REC_CATCH_EXCEPTION,THROWS_METHOD_THROWS_CLAUSE_THROWABLE,MS_PKGPROTECT,MS_CANNOT_BE_FINAL,UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD,SE_BAD_FIELD,URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD,SC_START_IN_CTOR,SWL_SLEEP_WITH_LOCK_HELD,IS2_INCONSISTENT_SYNC" />
+  </Match>
 </FindBugsFilter>
diff --git a/storage/src/main/java/org/apache/uniffle/storage/handler/impl/PooledHdfsShuffleWriteHandler.java b/storage/src/main/java/org/apache/uniffle/storage/handler/impl/PooledHdfsShuffleWriteHandler.java
index e6ee9ea8..6b2188a7 100644
--- a/storage/src/main/java/org/apache/uniffle/storage/handler/impl/PooledHdfsShuffleWriteHandler.java
+++ b/storage/src/main/java/org/apache/uniffle/storage/handler/impl/PooledHdfsShuffleWriteHandler.java
@@ -55,7 +55,9 @@ public class PooledHdfsShuffleWriteHandler implements ShuffleWriteHandler {
     // todo: support init lazily
     try {
       for (int i = 0; i < maxConcurrency; i++) {
-        queue.offer(
+        // Use add() here because we are sure the capacity will not be exceeded.
+        // Note: add() throws IllegalStateException when queue is full.
+        queue.add(
             new HdfsShuffleWriteHandler(
                 appId,
                 shuffleId,
@@ -82,7 +84,9 @@ public class PooledHdfsShuffleWriteHandler implements ShuffleWriteHandler {
     try {
       writeHandler.write(shuffleBlocks);
     } finally {
-      queue.offer(writeHandler);
+      // Use add() here because we are sure the capacity will not be exceeded.
+      // Note: add() throws IllegalStateException when queue is full.
+      queue.add(writeHandler);
     }
   }
 }