133tosakarin commented on code in PR #1142:
URL: https://github.com/apache/ratis/pull/1142#discussion_r1756267843


##########
ratis-server/src/main/java/org/apache/ratis/server/storage/SnapshotManager.java:
##########
@@ -114,7 +115,8 @@ public void installSnapshot(InstallSnapshotRequestProto 
request, StateMachine st
 
     // 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
-
+    MessageDigest digester = newMd5Digest();
+    digester.reset();

Review Comment:
   > Just taking a quick look. I think simply removing the `MemoizedSupplier` 
(just calling `MD5Hash#getDigester` every time) should technically work. From 
what I see, `MemoizedSupplier` only call the `MessageDigest#reset` at the first 
invocation. The subsequent `MemoizedSupplier` invocation will just return the 
`MessageDigest` instance without calling the `MessageDigest#reset` which might 
cause the mismatch issue.
   > 
   > Ozone encountered some thread local MD5 issue 
([apache/ozone#6435](https://github.com/apache/ozone/pull/6435)), might be 
related.
   
   Thank you for your comment!
   
   I also think that whether to remove MemoizedSupplier or not is irrelevant to 
the problem. My idea is to use a new digester for each file to solve this 
problem well.
   
   I have pushed the code to this PR, you can take a look



-- 
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]

Reply via email to