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

[ycable] cleanup logic for creating grpc future ready #289

Merged
merged 7 commits into from
Sep 7, 2022

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Sep 2, 2022

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
This PR attempts to remove this call
channel_ready = grpc.channel_ready_future for the ycabled gRPC implementation. The reason to do this is incase of channel not being connected ycabled tries to reattempt the channel creation again with each RPC request from linkmgrd.
This is not the right way to maintain the channels/stubs, and actually adds unrequired overhead for ycabled. This PR supports creating channel/stub in ycabled in correct manner.

Description

Motivation and Context

Required to reduce CPU usage for ycabled

How Has This Been Tested?

Deploying the changes on Arista testbed and UT

Additional Information (Optional)

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 2, 2022

This pull request introduces 2 alerts when merging 4229dd6 into 7c0a326 - view on LGTM.com

new alerts:

  • 2 for Variable defined multiple times

target_name = kvp.get("grpc_ssl_credential", None)
if credential is None or target_name is None:
return (None, None)
def create_channel(type,level, kvp, soc_ip, port):
Copy link

Choose a reason for hiding this comment

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

add a space before level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@prgeor prgeor added the Y-Cable label Sep 2, 2022
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Sep 6, 2022

This pull request introduces 2 alerts when merging 2681dbf into ce3b6db - view on LGTM.com

new alerts:

  • 2 for Variable defined multiple times

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 requested a review from lolyu September 7, 2022 00:55
@vdahiya12 vdahiya12 merged commit 3acb171 into sonic-net:master Sep 7, 2022
yxieca pushed a commit that referenced this pull request Sep 8, 2022
Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
This PR attempts to remove this call
channel_ready = grpc.channel_ready_future for the ycabled gRPC implementation. The reason to do this is incase of channel not being connected ycabled tries to reattempt the channel creation again with each RPC request from linkmgrd.
This is not the right way to maintain the channels/stubs, and actually adds unrequired overhead for ycabled. This PR supports creating channel/stub in ycabled in correct manner.

Description
Motivation and Context
Required to reduce CPU usage for ycabled

How Has This Been Tested?
Deploying the changes on Arista testbed and UT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants