HTHou commented on code in PR #1577:
URL: https://github.com/apache/ratis/pull/1577#discussion_r3963792946
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java:
##########
@@ -485,7 +546,11 @@ private class AppendLogResponseHandler implements
StreamObserver<AppendEntriesRe
*/
@Override
public void onNext(AppendEntriesReplyProto reply) {
- AppendEntriesRequest request = pendingRequests.remove(reply);
+ final AppendEntriesRequest request;
+ try (AutoCloseableLock writeLock = lock.writeLock(caller, LOG::trace)) {
Review Comment:
The extra write lock was intended to serialize reply notifications with
reset notifications, not to protect ConcurrentHashMap.remove(). I have removed
it in 7c6604cd4 and updated the listener contract: replies may race with
reset/inconsistency and other terminal callbacks, so consumers handle
correlation and deduplication. A regression now holds the appender write lock
on one thread while a SUCCESS reply is processed and reported on another
thread. All 24 targeted tests and Checkstyle/SpotBugs/RAT checks pass locally
on JDK 25.
##########
ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcLogAppender.java:
##########
@@ -571,6 +641,7 @@ public String toString() {
private void updateNextIndex(long replyNextIndex) {
try (AutoCloseableLock writeLock = lock.writeLock(caller, LOG::trace)) {
+ notifyReset("AppendEntries INCONSISTENCY", null);
Review Comment:
Agreed. Updated in 7c6604cd4: added AppendEntries.onReplyInconsistency() at
the pending-request clear in updateNextIndex, and renamed the parent callback
to onResetClient. INCONSISTENCY handling no longer emits a client-reset
notification. The new callback follows onReply for that response and runs under
the existing index/queue write lock. The regression verifies the callback order
and that the two events remain separate.
--
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]
