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

add async notification support in active-active topo; refactor code for ycable tasks for change events #327

Merged
merged 20 commits into from
May 26, 2023

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Jan 3, 2023

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
This PR adds an enhancement to ycabled when if and whenever a notification arrives from SoC or server that it is going to be serviced. The notification can come at anytime and hence it must be listened to or awaited by the client at all times. For this we use asyncio lib and avail the async functionality in python.

the added code defines a GracefulRestartClient class that can be used to send requests to a gRPC server for graceful restart. The class has three async methods: send_request, receive_response, and notify_graceful_restart_start.

send_request method retrieves tor from request_queue, creates a request with the ToR, and sends it to the server using the stub's NotifyGracefulRestartStart method. It then reads the response from the server and prints the details of the response.

receive_response method retrieves response from response_queue, prints/puts the the response in DB, sleeps for the period mentioned in the response, and then puts the tor of the response back in the request_queue

We follow the existing ycabled pattern and instantiate the task_worker which contains all these tasks in a seperate thread

Description

Motivation and Context

How Has This Been Tested?

UT and using this server to validate

import asyncio
import grpc
import os
from concurrent import futures
from proto_out import linkmgr_grpc_driver_pb2 as linkmgr_pb2
from proto_out import linkmgr_grpc_driver_pb2_grpc as linkmgr_pb2_grpc os.environ["GRPC_TRACE"] = "http" class GracefulRestartServicer(linkmgr_pb2_grpc.GracefulRestartServicer):     async def NotifyGracefulRestartStart(self, request, context):
        # Dummy values
        msgtype = linkmgr_pb2.SERVICE_BEGIN
        notifytype = linkmgr_pb2.CONTROL_PLANE
        guid = "abc123"
        period = 60         # Yield a stream of responses
        for i in range(5):
            response = linkmgr_pb2.GracefulAdminResponse(
                msgtype=msgtype,
                notifytype=notifytype,
                guid=guid,
                period=period
            )
            yield response
            await asyncio.sleep(1)  # delay between responses async def serve():
    server = grpc.aio.server()
    linkmgr_pb2_grpc.add_GracefulRestartServicer_to_server(
        GracefulRestartServicer(), server)
    server.add_insecure_port('[::]:50052')
    await server.start()
    await server.wait_for_termination() if __name__ == '__main__':
    asyncio.run(serve())

   

Additional Information (Optional)

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 marked this pull request as draft January 4, 2023 16:48
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 marked this pull request as ready for review January 13, 2023 19:49
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@prgeor
Copy link
Collaborator

prgeor commented Mar 7, 2023

@vdahiya12 merge conflict

@prgeor prgeor self-assigned this Mar 7, 2023
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>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@prgeor
Copy link
Collaborator

prgeor commented Apr 10, 2023

@vdahiya12 description says "GracefulRestartClient" class, i didnot find in the code

Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 changed the title add async notification support in active-active topo add async notification support in active-active topo; refactor code for ycable tasks for change events Apr 12, 2023
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 requested a review from zjswhhh April 18, 2023 17:33
Copy link

@zjswhhh zjswhhh left a comment

Choose a reason for hiding this comment

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

If I understand it correctly, the graceful restart notification is implemented by looping below?
fetch ToR side from request queue -> send request to server -> process response -> sleep if there is a restart period -> put ToR in request queue -> ...


await self.response_queue.put(response)

async def process_response(self):
Copy link

@zjswhhh zjswhhh Apr 20, 2023

Choose a reason for hiding this comment

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

Will response ever be written to DBs for other processes to consume? Or the only action item is to sleep gRPC client for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently the plan is to log it, if its going to be used later on for more purposes, we will put it to DB

if response is not None:
await asyncio.sleep(response.period)
else:
await asyncio.sleep(20)
Copy link

Choose a reason for hiding this comment

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

Will server respond when there isn't a graceful restart?

How is 20s determined here? If ToR doesn't get graceful restart notification before the scheduled restart, will SoC go ahead and restart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline, as mentioned 20seconds sleep is when there is no soc connectivity, otherwise once the notification for service start comes, we would sleep for advertised period.

zjswhhh
zjswhhh previously approved these changes Apr 21, 2023
Copy link

@zjswhhh zjswhhh left a comment

Choose a reason for hiding this comment

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

Discussed offline. @prgeor can you also review this PR?

vdahiya12 added a commit that referenced this pull request May 23, 2023
…355)

This change onboards refactor of various redis DB opens in y_cable_table-helper.py file and is required to onboard y_cable_async_client PR #327

Basically it has changes for all the table to be now exclusively opened once per class instance .
It also accommodates refactor for getting async channels open given the params to open it.

Description
Motivation and Context
refactor ycabled

How Has This Been Tested?
UT and deploy changes on test DUT
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
@vdahiya12 vdahiya12 merged commit d6b2a02 into sonic-net:master May 26, 2023
3 checks passed
yxieca pushed a commit that referenced this pull request May 26, 2023
…355)

This change onboards refactor of various redis DB opens in y_cable_table-helper.py file and is required to onboard y_cable_async_client PR #327

Basically it has changes for all the table to be now exclusively opened once per class instance .
It also accommodates refactor for getting async channels open given the params to open it.

Description
Motivation and Context
refactor ycabled

How Has This Been Tested?
UT and deploy changes on test DUT
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
yxieca pushed a commit that referenced this pull request May 26, 2023
…or ycable tasks for change events (#327)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com
This PR adds an enhancement to ycabled when if and whenever a notification arrives from SoC or server that it is going to be serviced. The notification can come at anytime and hence it must be listened to or awaited by the client at all times. For this we use asyncio lib and avail the async functionality in python.

the added code defines a GracefulRestartClient class that can be used to send requests to a gRPC server for graceful restart. The class has three async methods: send_request, receive_response, and notify_graceful_restart_start.

send_request method retrieves tor from request_queue, creates a request with the ToR, and sends it to the server using the stub's NotifyGracefulRestartStart method. It then reads the response from the server and prints the details of the response.

receive_response method retrieves response from response_queue, prints/puts the the response in DB, sleeps for the period mentioned in the response, and then puts the tor of the response back in the request_queue

We follow the existing ycabled pattern and instantiate the task_worker which contains all these tasks in a seperate thread

Description
Motivation and Context
How Has This Been Tested?
UT and using this server to validate
yxieca added a commit that referenced this pull request Jun 6, 2023
…r code for ycable tasks for change events (#327)"

This reverts commit 2466680.
yxieca added a commit that referenced this pull request Jul 7, 2023
… refactor code for ycable tasks for change events (#327)""

This reverts commit 5324554.
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.

None yet

4 participants