rpuch commented on code in PR #7488:
URL: https://github.com/apache/ignite-3/pull/7488#discussion_r2811120222


##########
modules/network/src/main/java/org/apache/ignite/internal/network/DefaultMessagingService.java:
##########
@@ -651,7 +700,13 @@ private void onInvokeResponse(NetworkMessage response, 
Long correlationId) {
         TimeoutObjectImpl responseFuture = requestsMap.remove(correlationId);
 
         if (responseFuture != null) {
-            responseFuture.future().complete(response);
+            var fut = responseFuture.future();
+
+            if (fut.isCompletedExceptionally()) {
+                metrics.incrementInvokeTimeouts();
+            } else {
+                fut.complete(response);
+            }

Review Comment:
   With the current code, we could lose a count in the following scenario:
   
   1. future is not complete yet
   2. we check whether it's completed exceptionally -> no, it's not
   3. concurrently, it gets completed exceptionally
   4. we attempt to complete the future normally, which is a no-op
   
   As a result, the future gets completed exceptionally, but we don't count 
this.
   
   Not a big deal, but we could do it like this:
   
   1. attempt to complete normally
   2. THEN check whether it was actually completed exceptionally
   
   This seems to solve the race



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