szetszwo commented on code in PR #661:
URL: https://github.com/apache/ratis/pull/661#discussion_r909069150
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -118,7 +126,7 @@ public void installSnapshot(StateMachine stateMachine,
new MD5Hash(chunk.getFileDigest().toByteArray());
// calculate the checksum of the snapshot file and compare it with the
// file digest in the request
- MD5Hash digest = MD5FileUtil.computeMd5ForFile(tmpSnapshotFile);
+ MD5Hash digest = new MD5Hash(digester.digest());
Review Comment:
Use
```
final MD5Hash digest = new MD5Hash(digester.get().digest());
```
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java:
##########
@@ -25,14 +26,19 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.nio.file.Path;
+import java.security.DigestInputStream;
+import java.security.MessageDigest;
import java.util.Optional;
/** Read {@link FileChunkProto}s from a file. */
public class FileChunkReader implements Closeable {
private final FileInfo info;
private final Path relativePath;
- private final FileInputStream in;
+ private final InputStream in;
+ private final boolean computeDigest;
+ private MessageDigest digester;
Review Comment:
Add final.
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java:
##########
@@ -52,7 +58,14 @@ public FileChunkReader(FileInfo info, RaftStorageDirectory
directory) throws IOE
.map(p -> directory.getRoot().toPath().relativize(p))
.orElse(info.getPath());
final File f = info.getPath().toFile();
- this.in = new FileInputStream(f);
+ if (info.getFileDigest() == null) {
+ digester = MD5Hash.getDigester();
+ this.in = new DigestInputStream(new FileInputStream(f), digester);
+ computeDigest = true;
+ } else {
+ this.in = new FileInputStream(f);
+ computeDigest = false;
Review Comment:
Set digester to null.
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java:
##########
@@ -25,14 +26,19 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.nio.file.Path;
+import java.security.DigestInputStream;
+import java.security.MessageDigest;
import java.util.Optional;
/** Read {@link FileChunkProto}s from a file. */
public class FileChunkReader implements Closeable {
private final FileInfo info;
private final Path relativePath;
- private final FileInputStream in;
+ private final InputStream in;
+ private final boolean computeDigest;
Review Comment:
Remove computeDigest and use digester != null.
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -98,17 +102,21 @@ public void installSnapshot(StateMachine stateMachine,
}
// create the temp snapshot file and put padding inside
out = new FileOutputStream(tmpSnapshotFile);
+ digester = MD5Hash.getDigester();
+ outWithDigest = new DigestOutputStream(out, digester);
} else {
Preconditions.assertTrue(tmpSnapshotFile.exists());
out = new FileOutputStream(tmpSnapshotFile, true);
+ outWithDigest = new DigestOutputStream(out, digester);
FileChannel fc = out.getChannel();
fc.position(chunk.getOffset());
}
// write data to the file
- out.write(chunk.getData().toByteArray());
+ outWithDigest.write(chunk.getData().toByteArray());
Review Comment:
Use
```
new DigestOutputStream(out,
digester.get()).write(chunk.getData().toByteArray());
```
The other changes are not needed.
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/FileChunkReader.java:
##########
@@ -66,14 +79,23 @@ public FileChunkProto readFileChunk(int chunkMaxSize)
throws IOException {
final long remaining = info.getFileSize() - offset;
final int chunkLength = remaining < chunkMaxSize ? (int) remaining :
chunkMaxSize;
final ByteString data = ByteString.readFrom(in, chunkLength);
+ boolean isDone = offset + chunkLength == info.getFileSize();
+ ByteString fileDigest = ByteString.EMPTY;
Review Comment:
Just set fileDigest to the current digest.
```
final ByteString fileDigest = ByteString.copyFrom(
digester != null? digester.digest():
info.getFileDigest().getDigest());
```
##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -51,6 +53,7 @@ public class SnapshotManager {
private final RaftStorage storage;
private final RaftPeerId selfId;
+ private MessageDigest digester;
Review Comment:
Use
```
private final Supplier<MessageDigest> digester =
JavaUtils.memoize(MD5Hash::getDigester);
```
Then, the code change below will be reduced.
--
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]
