Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding 'methodconfig' for all services in channel to allow retry #3343

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Aug 8, 2024

Description

Implement gRPC retries. It should improve the stability of the gRPC interface.

Issue linked

It should also close a lot of issues:

Close #3342 and related.

Checklist

@germa89 germa89 requested a review from a team as a code owner August 8, 2024 20:16
@germa89 germa89 requested review from clatapie and pyansys-ci-bot and removed request for a team August 8, 2024 20:16
@germa89 germa89 self-assigned this Aug 8, 2024
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added the new feature Request or proposal for a new feature label Aug 8, 2024
@germa89
Copy link
Collaborator Author

germa89 commented Aug 8, 2024

Way to test this, using the model from #2479 connecting to v25.1.0 docker image.

Then run:

python  compute_for_multiple_mesh_size_wb.py > log.log 2>&1

and then:

$ tail -n 600 -f log.log | grep retrying      
I0808 22:01:08.049285000 6143717376 retry_filter_legacy_call_data.cc:2001] chand=0x137076210 calld=0x156e982d0: retrying failed call in 30 ms
I0808 22:01:08.082253000 6143717376 retry_filter_legacy_call_data.cc:2001] chand=0x137076210 calld=0x156e982d0: retrying failed call in 73 ms
I0808 22:01:08.158568000 6143717376 retry_filter_legacy_call_data.cc:2001] chand=0x137076210 calld=0x156e982d0: retrying failed call in 310 ms

Copy link

codecov bot commented Aug 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.96%. Comparing base (93dd176) to head (4a4ae66).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3343      +/-   ##
==========================================
- Coverage   87.13%   86.96%   -0.17%     
==========================================
  Files          55       55              
  Lines        9816    10959    +1143     
==========================================
+ Hits         8553     9531     +978     
- Misses       1263     1428     +165     

@germa89
Copy link
Collaborator Author

germa89 commented Aug 9, 2024

I saw this:

<_InactiveRpcError of RPC that terminated with:
        status = StatusCode.UNAVAILABLE
        details = "failed to connect to all addresses; last error: UNAVAILABLE: ipv4:127.0.0.1:50062: FD shutdown"
        debug_error_string = "UNKNOWN:Error received from peer  {created_time:"2024-08-08T22:33:17.948629+02:00", grpc_status:14, grpc_message:"failed to connect to all addresses; last error: UNAVAILABLE: ipv4:127.0.0.1:50062: FD shutdown"}"

strange

@germa89
Copy link
Collaborator Author

germa89 commented Aug 14, 2024

Pinging @greschd in case he has any experience with this topic.

@germa89
Copy link
Collaborator Author

germa89 commented Aug 20, 2024

Repinging @greschd and @clatapie

@germa89
Copy link
Collaborator Author

germa89 commented Aug 27, 2024

I guess it is time for... @pyansys-ci-bot LGTM.

@germa89 germa89 enabled auto-merge (squash) August 27, 2024 15:26
Copy link
Contributor

@pyansys-ci-bot pyansys-ci-bot left a comment

Choose a reason for hiding this comment

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

✅ Approving this PR because germa89 said so in here 😬

LGTM

@germa89 germa89 changed the title feat: Adding 'methodconfig' for all services in channel that allow to retry feat: Adding 'methodconfig' for all services in channel to allow to retry Aug 27, 2024
@germa89 germa89 changed the title feat: Adding 'methodconfig' for all services in channel to allow to retry feat: Adding 'methodconfig' for all services in channel to allow retry Aug 27, 2024
@germa89 germa89 merged commit 0c0f679 into main Aug 27, 2024
55 of 56 checks passed
@germa89 germa89 deleted the feat/gRPC-retry-calls branch August 27, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Request or proposal for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement client side retry
2 participants