Skip to content

RATIS-1888. Handle exception of readIndexAsync in gRPC readIndex impl#920

Merged
szetszwo merged 4 commits intoapache:masterfrom
tisonkun:RATIS-1888
Sep 15, 2023
Merged

RATIS-1888. Handle exception of readIndexAsync in gRPC readIndex impl#920
szetszwo merged 4 commits intoapache:masterfrom
tisonkun:RATIS-1888

Conversation

@tisonkun
Copy link
Member

@tisonkun tisonkun commented Sep 15, 2023

What changes were proposed in this pull request?

@szetszwo I noticed that we once made a GrpcUtil.asyncCall (298c1a2) so I reuse the code with a few modification to adapt the readIndex type signature.

I check all rpcs defined in Grpc.proto and the readIndex rpc is the only one remaining having this issue.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-1888

How was this patch tested?

IMO, it's trivial and too detailed to test. But if we have some test infra I'm glad to work one.

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Member Author

tisonkun commented Sep 15, 2023

Tests failed on:

TestRaftReconfigurationWithGrpc>RaftReconfigurationBaseTest.testBootstrapReconf:389->RaftReconfigurationBaseTest.lambda$testBootstrapReconf$10:389->RaftReconfigurationBaseTest.runTestBootstrapReconf:429 » Timeout Timed out waiting for condition.

Doesn't seem related.

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 the change looks good.

@szetszwo
Copy link
Contributor

Doesn't seem related.

Agree. Just have restarted the failed job.

@szetszwo szetszwo merged commit 979b38a into apache:master Sep 15, 2023
@tisonkun tisonkun deleted the RATIS-1888 branch September 15, 2023 16:57
@SzyWilliam
Copy link
Member

@tisonkun Thanks a lot for the patch! It looks really concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants