133tosakarin commented on PR #1142:
URL: https://github.com/apache/ratis/pull/1142#issuecomment-2347881421
> > For each received file, the difference between using a new md5 object
and using a member variable is almost negligible
>
> Thanks for running the benchmarks! Let's use a new digester. How about
moving the new MD5 method to `MD5Hash`?
>
> ```java
> diff --git a/ratis-common/src/main/java/org/apache/ratis/io/MD5Hash.java
b/ratis-common/src/main/java/org/apache/ratis/io/MD5Hash.java
> index e60bef9652..8aacd9a65d 100644
> --- a/ratis-common/src/main/java/org/apache/ratis/io/MD5Hash.java
> +++ b/ratis-common/src/main/java/org/apache/ratis/io/MD5Hash.java
> @@ -29,14 +29,15 @@ import java.util.Arrays;
> public class MD5Hash {
> public static final int MD5_LEN = 16;
>
> - private static final ThreadLocal<MessageDigest> DIGESTER_FACTORY =
> - ThreadLocal.withInitial(() -> {
> + private static final ThreadLocal<MessageDigest> DIGESTER_FACTORY =
ThreadLocal.withInitial(MD5Hash::newDigester);
> +
> + public static MessageDigest newDigester() {
> try {
> return MessageDigest.getInstance("MD5");
> } catch (NoSuchAlgorithmException e) {
> - throw new RuntimeException(e);
> + throw new IllegalStateException("Failed to create MessageDigest for
MD5", e);
> }
> - });
> + }
>
> private byte[] digest;
>
> diff --git
a/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
b/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
> index 794604d66b..5b5158aebd 100644
> ---
a/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
> +++
b/ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java
> @@ -112,9 +109,9 @@ public class SnapshotManager {
> LOG.info("Installing snapshot:{}, to tmp dir:{}",
> ServerStringUtils.toInstallSnapshotRequestString(request),
tmpDir);
>
> + final MessageDigest digester = MD5Hash.newDigester();
> // TODO: Make sure that subsequent requests for the same
installSnapshot are coming in order,
> // and are not lost when whole request cycle is done. Check requestId
and requestIndex here
> -
> for (FileChunkProto chunk : snapshotChunkRequest.getFileChunksList())
{
> SnapshotInfo pi = stateMachine.getLatestSnapshot();
> if (pi != null && pi.getTermIndex().getIndex() >=
lastIncludedIndex) {
> ```
Good idea, but there are some problems in the provided code, we should
construct digester when receiving the first chunk and calculate md5 after
receiving a complete file.
Therefore, we add a MessageDigest member variable in SnapshotManager and
assign it to the first chunk every time
--
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]
