szetszwo commented on code in PR #1245:
URL: https://github.com/apache/ratis/pull/1245#discussion_r2025452956
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java:
##########
@@ -169,23 +163,28 @@ public long skip(long amt) throws IOException {
*/
boolean verifyHeader() throws IOException {
final int headerLength = SegmentedRaftLogFormat.getHeaderLength();
- final int readLength = in.read(temp, 0, headerLength);
- Preconditions.assertTrue(readLength <= headerLength);
- final int matchLength = SegmentedRaftLogFormat.matchHeader(temp, 0,
readLength);
- Preconditions.assertTrue(matchLength <= readLength);
-
- if (readLength == headerLength && matchLength == readLength) {
- // The header is matched successfully
- return true;
- } else if (SegmentedRaftLogFormat.isTerminator(temp, matchLength,
readLength - matchLength)) {
- // The header is partially written
- return false;
+ try{
Review Comment:
Let's try-catch only the read, i.e.
```diff
@@ -169,7 +163,14 @@ class SegmentedRaftLogReader implements Closeable {
*/
boolean verifyHeader() throws IOException {
final int headerLength = SegmentedRaftLogFormat.getHeaderLength();
- final int readLength = in.read(temp, 0, headerLength);
+ final int readLength;
+ try {
+ readLength = in.read(temp, 0, headerLength);
+ } catch (ClosedByInterruptException e) {
+ Thread.currentThread().interrupt();
+ throw new IOException("Interrupted while reading the header of " +
file, e);
+ }
+
```
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogReader.java:
##########
@@ -34,14 +34,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.BufferedInputStream;
-import java.io.Closeable;
-import java.io.DataInputStream;
-import java.io.EOFException;
-import java.io.File;
-import java.io.FilterInputStream;
-import java.io.IOException;
-import java.io.InputStream;
+import java.io.*;
Review Comment:
Please don't change the existing imports.
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogInputStream.java:
##########
@@ -103,8 +104,12 @@ public LogEntryProto nextEntry() throws IOException {
if (state.isUnopened()) {
try {
init();
- } catch (Exception e) {
Review Comment:
Let's keep catching `Exception`. It will print an error for other
exceptions.
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/SegmentedRaftLogInputStream.java:
##########
@@ -103,8 +104,12 @@ public LogEntryProto nextEntry() throws IOException {
if (state.isUnopened()) {
try {
init();
- } catch (Exception e) {
- LOG.error("caught exception initializing " + this, e);
+ } catch (IOException e) {
+ if (e.getCause() instanceof ClosedByInterruptException) {
+ LOG.warn("Initialization is interrupted: {}", this, e);
+ } else {
+ LOG.error("caught exception initializing " + this, e);
Review Comment:
Could you also update the log message?
```java
LOG.error("Failed to initialize {}", this, e);
```
##########
ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/LogSegment.java:
##########
@@ -292,7 +292,7 @@ ReferenceCountedObject<LogEntryProto> load(TermIndex key)
throws IOException {
}
});
loadingTimes.incrementAndGet();
- return Objects.requireNonNull(toReturn.get(), () -> "toReturn == null
for " + key);
+ return toReturn.get();
Review Comment:
We should throw `RaftLogIOException` if it is null.
```java
+++
b/ratis-server/src/main/java/org/apache/ratis/server/raftlog/segmented/LogSegment.java
@@ -292,7 +292,11 @@ public final class LogSegment {
}
});
loadingTimes.incrementAndGet();
- return Objects.requireNonNull(toReturn.get(), () -> "toReturn == null
for " + key);
+ final ReferenceCountedObject<LogEntryProto> proto = toReturn.get();
+ if (proto == null) {
+ throw new RaftLogIOException("Failed to load log entry " + key);
+ }
+ return proto;
}
}
@@ -502,8 +506,10 @@ public final class LogSegment {
}
try {
return cacheLoader.load(ti);
+ } catch (RaftLogIOException e) {
+ throw e;
} catch (Exception e) {
- throw new RaftLogIOException(e);
+ throw new RaftLogIOException("Failed to loadCache for log entry " +
ti, e);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
