This is an automated email from the ASF dual-hosted git repository.
ckj 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 721b22cf [SpotBugs] Fix REC_CATCH_EXCEPTION (#527)
721b22cf is described below
commit 721b22cf0a02c0d3fdb65851aa1edd8271dbe138
Author: Kaijie Chen <[email protected]>
AuthorDate: Wed Feb 1 15:51:19 2023 +0800
[SpotBugs] Fix REC_CATCH_EXCEPTION (#527)
### What changes were proposed in this pull request?
1. Fix REC_CATCH_EXCEPTION violations (ignored one case).
2. Remove REC_CATCH_EXCEPTION from SpotBugs exclusion.
### Why are the changes needed?
Sub-task of #532 - Fix [REC_CATCH_EXCEPTION][1].
[1]:
https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#rec-exception-is-caught-when-exception-is-not-thrown-rec-catch-exception
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI.
---
.../org/apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java | 2 +-
.../java/org/apache/spark/shuffle/RssShuffleManager.java | 2 ++
.../java/org/apache/uniffle/common/config/ConfigUtils.java | 4 ++--
.../org/apache/uniffle/coordinator/ClientConfManager.java | 3 ++-
.../coordinator/access/checker/AccessCandidatesChecker.java | 3 ++-
pom.xml | 12 ++++++++++++
spotbugs-exclude.xml | 2 +-
.../storage/handler/impl/HdfsShuffleWriteHandler.java | 2 +-
.../uniffle/storage/handler/impl/LocalFileWriteHandler.java | 2 +-
9 files changed, 24 insertions(+), 8 deletions(-)
diff --git
a/client-mr/src/main/java/org/apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java
b/client-mr/src/main/java/org/apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java
index 9d6c38ec..d7d2d883 100644
---
a/client-mr/src/main/java/org/apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java
+++
b/client-mr/src/main/java/org/apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java
@@ -366,7 +366,7 @@ public class RssMRAppMaster extends MRAppMaster {
long size = status.getLen();
String sizes = conf.get(MRJobConfig.CACHE_FILES_SIZES);
conf.set(MRJobConfig.CACHE_FILES_SIZES, sizes == null ?
String.valueOf(size) : size + "," + sizes);
- } catch (Exception e) {
+ } catch (InterruptedException | IOException e) {
LOG.error("Upload extra conf exception", e);
throw new RuntimeException("Upload extra conf exception ", e);
}
diff --git
a/client-spark/spark3/src/main/java/org/apache/spark/shuffle/RssShuffleManager.java
b/client-spark/spark3/src/main/java/org/apache/spark/shuffle/RssShuffleManager.java
index cabccb3b..e095637b 100644
---
a/client-spark/spark3/src/main/java/org/apache/spark/shuffle/RssShuffleManager.java
+++
b/client-spark/spark3/src/main/java/org/apache/spark/shuffle/RssShuffleManager.java
@@ -34,6 +34,7 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Maps;
import com.google.common.collect.Queues;
import com.google.common.collect.Sets;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.hadoop.conf.Configuration;
import org.apache.spark.MapOutputTracker;
import org.apache.spark.ShuffleDependency;
@@ -506,6 +507,7 @@ public class RssShuffleManager implements ShuffleManager {
);
}
+ @SuppressFBWarnings("REC_CATCH_EXCEPTION")
private Roaring64NavigableMap getExpectedTasksByExecutorId(
int shuffleId,
int startPartition,
diff --git
a/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
b/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
index 578276ad..66df92f7 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
@@ -195,8 +195,8 @@ public class ConfigUtils {
configOptionList.add((ConfigOption<Object>) field.get(null));
}
}
- } catch (Exception e) {
- throw new IllegalArgumentException("Load Configuration option
exception");
+ } catch (IllegalArgumentException | IllegalAccessException e) {
+ throw new IllegalArgumentException("Exception when loading configuration
option", e);
}
return configOptionList;
}
diff --git
a/coordinator/src/main/java/org/apache/uniffle/coordinator/ClientConfManager.java
b/coordinator/src/main/java/org/apache/uniffle/coordinator/ClientConfManager.java
index 6b57730c..1c3959eb 100644
---
a/coordinator/src/main/java/org/apache/uniffle/coordinator/ClientConfManager.java
+++
b/coordinator/src/main/java/org/apache/uniffle/coordinator/ClientConfManager.java
@@ -18,6 +18,7 @@
package org.apache.uniffle.coordinator;
import java.io.Closeable;
+import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.concurrent.Executors;
@@ -137,7 +138,7 @@ public class ClientConfManager implements Closeable {
String content = null;
try (FSDataInputStream in = fileSystem.open(path)) {
content = IOUtils.toString(in, StandardCharsets.UTF_8);
- } catch (Exception e) {
+ } catch (IOException e) {
LOG.error("Fail to load content from {}", path.toUri().toString());
}
return content;
diff --git
a/coordinator/src/main/java/org/apache/uniffle/coordinator/access/checker/AccessCandidatesChecker.java
b/coordinator/src/main/java/org/apache/uniffle/coordinator/access/checker/AccessCandidatesChecker.java
index bbc8f382..76f091cd 100644
---
a/coordinator/src/main/java/org/apache/uniffle/coordinator/access/checker/AccessCandidatesChecker.java
+++
b/coordinator/src/main/java/org/apache/uniffle/coordinator/access/checker/AccessCandidatesChecker.java
@@ -17,6 +17,7 @@
package org.apache.uniffle.coordinator.access.checker;
+import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Set;
import java.util.concurrent.Executors;
@@ -153,7 +154,7 @@ public class AccessCandidatesChecker extends
AbstractAccessChecker {
String content = null;
try (FSDataInputStream in = fileSystem.open(path)) {
content = IOUtils.toString(in, StandardCharsets.UTF_8);
- } catch (Exception e) {
+ } catch (IOException e) {
LOG.error("Fail to load content from {}", path.toUri().toString());
}
return content;
diff --git a/pom.xml b/pom.xml
index 50b6a675..df2153c3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -128,6 +128,11 @@
<artifactId>error_prone_annotations</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-annotations</artifactId>
+ </dependency>
+
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
@@ -614,6 +619,13 @@
<version>${zstd-jni.version}</version>
</dependency>
+ <dependency>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-annotations</artifactId>
+ <version>${spotbugs.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
<dependency>
<groupId>org.xerial.snappy</groupId>
<artifactId>snappy-java</artifactId>
diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml
index f7b5d80a..182730e4 100644
--- a/spotbugs-exclude.xml
+++ b/spotbugs-exclude.xml
@@ -20,6 +20,6 @@
<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,SC_START_IN_CTOR,SWL_SLEEP_WITH_LOCK_HELD,IS2_INCONSISTENT_SYNC"
/>
+ <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,THROWS_METHOD_THROWS_CLAUSE_THROWABLE,MS_PKGPROTECT,MS_CANNOT_BE_FINAL,UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD,SE_BAD_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/HdfsShuffleWriteHandler.java
b/storage/src/main/java/org/apache/uniffle/storage/handler/impl/HdfsShuffleWriteHandler.java
index 8ac30e13..dc4b94f0 100644
---
a/storage/src/main/java/org/apache/uniffle/storage/handler/impl/HdfsShuffleWriteHandler.java
+++
b/storage/src/main/java/org/apache/uniffle/storage/handler/impl/HdfsShuffleWriteHandler.java
@@ -127,7 +127,7 @@ public class HdfsShuffleWriteHandler implements
ShuffleWriteHandler {
"Write handler inside cost {} ms for {}",
(System.currentTimeMillis() - ss),
fileNamePrefix);
- } catch (Exception e) {
+ } catch (IOException e) {
LOG.warn("Write failed with " + shuffleBlocks.size() + " blocks for "
+ fileNamePrefix + "_" + failTimes, e);
failTimes++;
throw new RuntimeException(e);
diff --git
a/storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileWriteHandler.java
b/storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileWriteHandler.java
index 881982d2..fa466a08 100644
---
a/storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileWriteHandler.java
+++
b/storage/src/main/java/org/apache/uniffle/storage/handler/impl/LocalFileWriteHandler.java
@@ -57,7 +57,7 @@ public class LocalFileWriteHandler implements
ShuffleWriteHandler {
try {
// try to create folder, it may be created by other Shuffle Server
baseFolder.mkdirs();
- } catch (Exception e) {
+ } catch (SecurityException e) {
// if folder exist, ignore the exception
if (!baseFolder.exists()) {
LOG.error("Can't create shuffle folder:" + basePath, e);
