You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "kaijchen (via GitHub)" <gi...@apache.org> on 2023/03/07 12:15:40 UTC
[GitHub] [ratis] kaijchen opened a new pull request, #847: RATIS-1810. Intermittent failure in TestRaftServerWithGrpc#testRaftClientMetrics
kaijchen opened a new pull request, #847:
URL: https://github.com/apache/ratis/pull/847
## What changes were proposed in this pull request?
Address flaky test TestRaftServerWithGrpc#testRaftClientMetrics.
The metrics are updated asynchronously, sometimes it hasn't been updated before assertions.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1810
## How was this patch tested?
100x TestRaftServerWithGrpc#testRaftClientMetrics in CI.
Before: https://github.com/kaijchen/ratis/actions/runs/4345085390
After: https://github.com/kaijchen/ratis/actions/runs/4353918913
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a diff in pull request #847: RATIS-1810. Intermittent failure in TestRaftServerWithGrpc#testRaftClientMetrics
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #847:
URL: https://github.com/apache/ratis/pull/847#discussion_r1130244158
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -890,15 +890,14 @@ public CompletableFuture<RaftClientReply> submitClientRequestAsync(
}
final RaftClientRequest.Type type = request.getType();
- replyFuture.whenComplete((clientReply, exception) -> {
+ return replyFuture.whenComplete((clientReply, exception) -> {
if (clientReply.isSuccess()) {
Review Comment:
BTW, it should stop the timer even if it has failed. Could you also change it?
```java
- if (clientReply.isSuccess()) {
- timerContext.ifPresent(Timekeeper.Context::stop);
- }
+ timerContext.ifPresent(Timekeeper.Context::stop);
```
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #847: RATIS-1810. Intermittent failure in TestRaftServerWithGrpc#testRaftClientMetrics
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #847:
URL: https://github.com/apache/ratis/pull/847
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] kaijchen commented on pull request #847: RATIS-1810. Intermittent failure in TestRaftServerWithGrpc#testRaftClientMetrics
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #847:
URL: https://github.com/apache/ratis/pull/847#issuecomment-1458076329
Cc @szetszwo, although this patch fixes the problem, it may hurt performance slightly.
Alternatively, should we change the test to wait before assertions?
Do you have a better idea?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] kaijchen commented on a diff in pull request #847: RATIS-1810. Intermittent failure in TestRaftServerWithGrpc#testRaftClientMetrics
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #847:
URL: https://github.com/apache/ratis/pull/847#discussion_r1130298513
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -890,15 +890,14 @@ public CompletableFuture<RaftClientReply> submitClientRequestAsync(
}
final RaftClientRequest.Type type = request.getType();
- replyFuture.whenComplete((clientReply, exception) -> {
+ return replyFuture.whenComplete((clientReply, exception) -> {
if (clientReply.isSuccess()) {
Review Comment:
Thanks for reviewing, updated.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org