szetszwo commented on a change in pull request #563:
URL: https://github.com/apache/ratis/pull/563#discussion_r769156710



##########
File path: 
ratis-client/src/main/java/org/apache/ratis/client/impl/OrderedStreamAsync.java
##########
@@ -147,4 +156,13 @@ private void sendRequestToNetwork(DataStreamWindowRequest 
request){
       return null;
     });
   }
+
+  private void scheduleWithTimeout(DataStreamWindowRequest request, 
TimeDuration sleepTime) {

Review comment:
       Let's use requestTimeout directly.
   ```
     private void scheduleWithTimeout(DataStreamWindowRequest request) {
       scheduler.onTimeout(requestTimeout, () -> {
         if (!request.getReplyFuture().isDone()) {
           request.getReplyFuture().completeExceptionally(
               new TimeoutIOException("Timeout " + requestTimeout + ": Failed 
to send " + request));
         }
       }, LOG, () -> "Failed to completeExceptionally for " + request);
     }
   ```
   

##########
File path: 
ratis-client/src/main/java/org/apache/ratis/client/impl/OrderedStreamAsync.java
##########
@@ -147,4 +156,13 @@ private void sendRequestToNetwork(DataStreamWindowRequest 
request){
       return null;
     });
   }
+
+  private void scheduleWithTimeout(DataStreamWindowRequest request, 
TimeDuration sleepTime) {
+    scheduler.onTimeout(sleepTime, () -> {
+      if (!request.getReplyFuture().isDone()) {
+        request.getReplyFuture().completeExceptionally(
+            new IOException("Send request timeout " + request));
+      }
+    }, LOG, () -> "Send request timeout " + request);

Review comment:
       The error message is for handling the exception thrown by the timeout 
code.  Let's change it.
   ```
       }, LOG, () -> "Failed to completeExceptionally for " + request);
   ```
   

##########
File path: 
ratis-client/src/main/java/org/apache/ratis/client/impl/OrderedStreamAsync.java
##########
@@ -147,4 +156,13 @@ private void sendRequestToNetwork(DataStreamWindowRequest 
request){
       return null;
     });
   }
+
+  private void scheduleWithTimeout(DataStreamWindowRequest request, 
TimeDuration sleepTime) {
+    scheduler.onTimeout(sleepTime, () -> {
+      if (!request.getReplyFuture().isDone()) {
+        request.getReplyFuture().completeExceptionally(
+            new IOException("Send request timeout " + request));

Review comment:
       - Let's use TimeoutIOException instead of IOException
   - Include the timeout value in the error message of TimeoutIOException.
   ```
               new TimeoutIOException("Timeout " + requestTimeout + ": Failed 
to send " + request));
   ```




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