This is an automated email from the ASF dual-hosted git repository.
szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git
The following commit(s) were added to refs/heads/master by this push:
new eb465d56d RATIS-2371. Fix
LeaderElection/SegmentedRaftLogReader/FileChunkReader CT_CONSTRUCTOR_THROW
spotbugs (#1327)
eb465d56d is described below
commit eb465d56d6fc6cfb849e6807161cbaf675433282
Author: Potato <[email protected]>
AuthorDate: Wed Dec 17 03:03:32 2025 +0800
RATIS-2371. Fix LeaderElection/SegmentedRaftLogReader/FileChunkReader
CT_CONSTRUCTOR_THROW spotbugs (#1327)
---
ratis-server/dev-support/findbugsExcludeFile.xml | 12 ---------
.../apache/ratis/server/impl/LeaderElection.java | 30 ++++++++++++++--------
.../org/apache/ratis/server/impl/RoleInfo.java | 2 +-
.../server/leader/InstallSnapshotRequests.java | 2 +-
.../segmented/SegmentedRaftLogInputStream.java | 2 +-
.../raftlog/segmented/SegmentedRaftLogReader.java | 16 +++++++++---
.../ratis/server/storage/FileChunkReader.java | 22 +++++++++++-----
.../ratis/server/impl/LeaderElectionTests.java | 4 +--
.../impl/TestLeaderElectionServerInterface.java | 2 +-
9 files changed, 53 insertions(+), 39 deletions(-)
diff --git a/ratis-server/dev-support/findbugsExcludeFile.xml
b/ratis-server/dev-support/findbugsExcludeFile.xml
index 918085620..0161c226b 100644
--- a/ratis-server/dev-support/findbugsExcludeFile.xml
+++ b/ratis-server/dev-support/findbugsExcludeFile.xml
@@ -15,10 +15,6 @@
limitations under the License.
-->
<FindBugsFilter>
- <Match>
- <Class name="org.apache.ratis.server.impl.LeaderElection"/>
- <Bug pattern="CT_CONSTRUCTOR_THROW"/>
- </Match>
<Match>
<Class name="org.apache.ratis.server.impl.LeaderStateImpl"/>
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
@@ -51,10 +47,6 @@
<Class
name="org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogOutputStream"/>
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
</Match>
- <Match>
- <Class
name="org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogReader"/>
- <Bug pattern="CT_CONSTRUCTOR_THROW"/>
- </Match>
<Match>
<Class
name="org.apache.ratis.server.raftlog.segmented.SegmentedRaftLog$Builder"/>
<Bug pattern="EI_EXPOSE_REP2"/>
@@ -67,10 +59,6 @@
<Class
name="org.apache.ratis.server.raftlog.segmented.SegmentedRaftLogWorker$WriteLog"/>
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
</Match>
- <Match>
- <Class name="org.apache.ratis.server.storage.FileChunkReader"/>
- <Bug pattern="CT_CONSTRUCTOR_THROW"/>
- </Match>
<Match>
<Class name="org.apache.ratis.server.storage.RaftStorageImpl"/>
<Bug pattern="EI_EXPOSE_REP"/>
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
index 9953e12af..385d33833 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/impl/LeaderElection.java
@@ -77,7 +77,7 @@ import static org.apache.ratis.util.LifeCycle.State.STARTING;
* Ongaro, D. Consensus: Bridging Theory and Practice. PhD thesis, Stanford
University, 2014.
* Available at https://github.com/ongardie/dissertation
*/
-class LeaderElection implements Runnable {
+final class LeaderElection implements Runnable {
public static final Logger LOG =
LoggerFactory.getLogger(LeaderElection.class);
interface ServerInterface {
@@ -306,25 +306,33 @@ class LeaderElection implements Runnable {
private final boolean skipPreVote;
private final ConfAndTerm round0;
- LeaderElection(RaftServerImpl server, boolean force) {
- this(ServerInterface.get(server), force);
+ static LeaderElection newInstance(RaftServerImpl server, boolean force) {
+ return newInstance(ServerInterface.get(server), force);
}
- LeaderElection(ServerInterface server, boolean force) {
- this.name = ServerStringUtils.generateUnifiedName(server.getMemberId(),
getClass()) + COUNT.incrementAndGet();
- this.lifeCycle = new LifeCycle(this);
- this.daemon = Daemon.newBuilder().setName(name).setRunnable(this)
- .setThreadGroup(server.getThreadGroup()).build();
- this.server = server;
- this.skipPreVote = force || !server.isPreVoteEnabled();
+ static LeaderElection newInstance(ServerInterface server, boolean force) {
+ String name = ServerStringUtils.generateUnifiedName(server.getMemberId(),
LeaderElection.class)
+ + COUNT.incrementAndGet();
try {
// increase term of the candidate in advance if it's forced to election
- this.round0 = force ? server.initElection(Phase.ELECTION) : null;
+ final ConfAndTerm round0 = force ? server.initElection(Phase.ELECTION) :
null;
+ return new LeaderElection(name, server, force, round0);
} catch (IOException e) {
throw new IllegalStateException(name + ": Failed to initialize
election", e);
}
}
+
+ private LeaderElection(String name, ServerInterface server, boolean force,
ConfAndTerm round0) {
+ this.name = name;
+ this.lifeCycle = new LifeCycle(this);
+ this.daemon = Daemon.newBuilder().setName(name).setRunnable(this)
+ .setThreadGroup(server.getThreadGroup()).build();
+ this.server = server;
+ this.skipPreVote = force || !server.isPreVoteEnabled();
+ this.round0 = round0;
+ }
+
void start() {
startIfNew(daemon::start);
}
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/impl/RoleInfo.java
b/ratis-server/src/main/java/org/apache/ratis/server/impl/RoleInfo.java
index a5cd7da66..409d7a06b 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/impl/RoleInfo.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/impl/RoleInfo.java
@@ -126,7 +126,7 @@ class RoleInfo {
if (pauseLeaderElection.get()) {
return;
}
- updateAndGet(leaderElection, new LeaderElection(server, force)).start();
+ updateAndGet(leaderElection, LeaderElection.newInstance(server,
force)).start();
}
void setLeaderElectionPause(boolean pause) {
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/leader/InstallSnapshotRequests.java
b/ratis-server/src/main/java/org/apache/ratis/server/leader/InstallSnapshotRequests.java
index 6300ea483..218c864e6 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/leader/InstallSnapshotRequests.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/leader/InstallSnapshotRequests.java
@@ -119,7 +119,7 @@ class InstallSnapshotRequests implements
Iterable<InstallSnapshotRequestProto> {
final FileInfo info = snapshot.getFiles().get(fileIndex);
try {
if (current == null) {
- current = new FileChunkReader(info, getRelativePath.apply(info));
+ current = FileChunkReader.newInstance(info,
getRelativePath.apply(info));
}
final FileChunkProto chunk =
current.readFileChunk(snapshotChunkMaxSize);
if (chunk.getDone()) {
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogInputStream.java
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogInputStream.java
index 3cc8767fa..c302d1f9a 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogInputStream.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogInputStream.java
@@ -83,7 +83,7 @@ public class SegmentedRaftLogInputStream implements Closeable
{
state.open();
boolean initSuccess = false;
try {
- reader = new SegmentedRaftLogReader(logFile, maxOpSize, raftLogMetrics);
+ reader = SegmentedRaftLogReader.newInstance(logFile, maxOpSize,
raftLogMetrics);
initSuccess = reader.verifyHeader();
} finally {
if (!initSuccess) {
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java
index 57baffb2f..b8c906366 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java
@@ -46,7 +46,7 @@ import java.nio.channels.ClosedByInterruptException;
import java.util.Optional;
import java.util.zip.Checksum;
-class SegmentedRaftLogReader implements Closeable {
+final class SegmentedRaftLogReader implements Closeable {
static final Logger LOG =
LoggerFactory.getLogger(SegmentedRaftLogReader.class);
/**
* InputStream wrapper that keeps track of the current stream position.
@@ -150,10 +150,18 @@ class SegmentedRaftLogReader implements Closeable {
private final SegmentedRaftLogMetrics raftLogMetrics;
private final SizeInBytes maxOpSize;
- SegmentedRaftLogReader(File file, SizeInBytes maxOpSize,
SegmentedRaftLogMetrics raftLogMetrics) throws IOException {
+ static SegmentedRaftLogReader newInstance(File file, SizeInBytes maxOpSize,
SegmentedRaftLogMetrics raftLogMetrics)
+ throws IOException {
+ final LimitedInputStream limiter = new LimitedInputStream(new
BufferedInputStream(FileUtils.newInputStream(file)));
+ final DataInputStream in = new DataInputStream(limiter);
+ return new SegmentedRaftLogReader(file, maxOpSize, raftLogMetrics,
limiter, in);
+ }
+
+ private SegmentedRaftLogReader(File file, SizeInBytes maxOpSize,
SegmentedRaftLogMetrics raftLogMetrics,
+ LimitedInputStream limiter, DataInputStream in) {
this.file = file;
- this.limiter = new LimitedInputStream(new
BufferedInputStream(FileUtils.newInputStream(file)));
- in = new DataInputStream(limiter);
+ this.limiter = limiter;
+ this.in = in;
checksum = new PureJavaCrc32C();
this.maxOpSize = maxOpSize;
this.raftLogMetrics = raftLogMetrics;
diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java
b/ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java
index b80924eef..6c4541209 100644
---
a/ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java
@@ -34,7 +34,7 @@ import java.security.DigestInputStream;
import java.security.MessageDigest;
/** Read {@link FileChunkProto}s from a file. */
-public class FileChunkReader implements Closeable {
+public final class FileChunkReader implements Closeable {
private final FileInfo info;
private final Path relativePath;
private final InputStream in;
@@ -51,17 +51,27 @@ public class FileChunkReader implements Closeable {
* @param relativePath the relative path of the file.
* @throws IOException if it failed to open the file.
*/
- public FileChunkReader(FileInfo info, Path relativePath) throws IOException {
- this.info = info;
- this.relativePath = relativePath;
+ public static FileChunkReader newInstance(FileInfo info, Path relativePath)
throws IOException {
final File f = info.getPath().toFile();
+ final InputStream in;
+ final MessageDigest digester;
+
if (info.getFileDigest() == null) {
digester = MD5FileUtil.newMD5();
- this.in = new DigestInputStream(FileUtils.newInputStream(f), digester);
+ in = new DigestInputStream(FileUtils.newInputStream(f), digester);
} else {
digester = null;
- this.in = FileUtils.newInputStream(f);
+ in = FileUtils.newInputStream(f);
}
+
+ return new FileChunkReader(info, relativePath, in, digester);
+ }
+
+ private FileChunkReader(FileInfo info, Path relativePath, InputStream in,
MessageDigest digester) {
+ this.info = info;
+ this.relativePath = relativePath;
+ this.in = in;
+ this.digester = digester;
}
static ByteString readFileChunk(int chunkLength, InputStream in) throws
IOException {
diff --git
a/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
b/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
index 724a06643..456d2ad2a 100644
---
a/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
+++
b/ratis-server/src/test/java/org/apache/ratis/server/impl/LeaderElectionTests.java
@@ -592,7 +592,7 @@ public abstract class LeaderElectionTests<CLUSTER extends
MiniRaftCluster>
@Test
public void testImmediatelyRevertedToFollower() {
RaftServerImpl server = createMockServer(true);
- LeaderElection subject = new LeaderElection(server, false);
+ LeaderElection subject = LeaderElection.newInstance(server, false);
try {
subject.startInForeground();
@@ -606,7 +606,7 @@ public abstract class LeaderElectionTests<CLUSTER extends
MiniRaftCluster>
@Test
public void testShutdownBeforeStart() {
RaftServerImpl server = createMockServer(false);
- LeaderElection subject = new LeaderElection(server, false);
+ LeaderElection subject = LeaderElection.newInstance(server, false);
try {
subject.shutdown();
diff --git
a/ratis-test/src/test/java/org/apache/ratis/server/impl/TestLeaderElectionServerInterface.java
b/ratis-test/src/test/java/org/apache/ratis/server/impl/TestLeaderElectionServerInterface.java
index 876633db1..3a91f9a34 100644
---
a/ratis-test/src/test/java/org/apache/ratis/server/impl/TestLeaderElectionServerInterface.java
+++
b/ratis-test/src/test/java/org/apache/ratis/server/impl/TestLeaderElectionServerInterface.java
@@ -186,7 +186,7 @@ public class TestLeaderElectionServerInterface extends
BaseTest {
for(int i = 0; i < lastEntries.length; i++) {
map.put(peers.get(i).getId(), lastEntries[i]);
}
- final LeaderElection election = new
LeaderElection(newServerInterface(expectToPass, map), false);
+ final LeaderElection election =
LeaderElection.newInstance(newServerInterface(expectToPass, map), false);
election.startInForeground();
}
