Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3866112900 > +1 the change looks good. @szetszwo Thanks for reviewing the code! -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
szetszwo merged PR #1342: URL: https://github.com/apache/ratis/pull/1342 -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3865866381 > > Let's move the try-catch from the sendRequest(request) to sendRequest(request, proxy). Then, the code is simpler; See https://issues.apache.org/jira/secure/attachment/13080704/1342_review.patch > > @slfan1989 , Have you looked at the comment above? @szetszwo Thank you for your suggestions! I’m updating this PR based on your feedback and will update it soon. -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
szetszwo commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3865854200 > Let's move the try-catch from the sendRequest(request) to sendRequest(request, proxy). Then, the code is simpler; See https://issues.apache.org/jira/secure/attachment/13080704/1342_review.patch @slfan1989 , Have you looked at the comment above? -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on code in PR #1342:
URL: https://github.com/apache/ratis/pull/1342#discussion_r2778271182
##
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientRpc.java:
##
@@ -164,12 +176,35 @@ public void onCompleted() {
}
}
});
+replyFuture.whenComplete((reply, exception) -> {
+ if (replyFuture.isCancelled()) {
+requestObserver.onError(Status.CANCELLED
+.withDescription(clientId + ": request #" + request.getCallId() +
" cancelled")
+.asRuntimeException());
Review Comment:
Thank you so much for your suggestion! I’ve already updated the code.
--
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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
szetszwo commented on code in PR #1342:
URL: https://github.com/apache/ratis/pull/1342#discussion_r2778149777
##
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientRpc.java:
##
@@ -164,12 +176,35 @@ public void onCompleted() {
}
}
});
+replyFuture.whenComplete((reply, exception) -> {
+ if (replyFuture.isCancelled()) {
+requestObserver.onError(Status.CANCELLED
+.withDescription(clientId + ": request #" + request.getCallId() +
" cancelled")
+.asRuntimeException());
Review Comment:
Let's use asException() instead of asRuntimeException().
--
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]
[PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 opened a new pull request, #1342: URL: https://github.com/apache/ratis/pull/1342 ## What changes were proposed in this pull request? This PR adds timeout and thread interruption handling support for synchronous gRPC client requests in GrpcClientRpc. ## What is the link to the Apache JIRA RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc ## How was this patch tested? - testGrpcClientRpcSyncTimeout: Verifies that synchronous requests correctly throw TimeoutIOException when the configured timeout is reached. Uses SimpleStateMachine4Testing.blockStartTransaction() to simulate a blocking scenario. - testGrpcClientRpcSyncCancelOnInterrupt: Verifies that when a request thread is interrupted, the request is properly cancelled and InterruptedIOException is thrown. Validates that the thread exits promptly after interruption. -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3863912156 @szetszwo Could you please review this PR? Thank you very much! -- 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]
