Add a new API connect_host to SonicV2Connector #1003
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Enhance test_redis_ut.py.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@arlakshm @vmittal-msft this is needed for the aggregate Voq counters feature |
|
@qiluo-msft @liuh-80 : can you please review this. |
|
LGTM. please rebase. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@qiluo-msft please help in merging this PR |
| db = SonicV2Connector(use_unix_socket_path=True, namespace='', decode_responses=True) | ||
| db = SonicV2Connector(use_unix_socket_path=False, decode_responses=True) | ||
| db = SonicV2Connector(host="127.0.0.1", decode_responses=True) | ||
| db = SonicV2Connector() |
There was a problem hiding this comment.
Looks like in the queuestat: https://github.com/sonic-net/sonic-swss-common/blob/master/common/dbconnector.cpp#L655-L664 can be used. But it will need significant adjustments in the unit test in our case.
The infra for mimicking SonicV2Connector is already in place, it may not be easy to do that with DBConnector
https://github.com/sonic-net/sonic-utilities/blob/master/tests/mock_tables/dbconnector.py#L231-L240
There was a problem hiding this comment.
We should first discuss which design fit your needs more properly. The effort of test adjustments and how to mimicking should not revert the design decision. Sorry if you already spend time on testcode, but we need to be careful when extending interfaces in swss-common repo.
This feature has been tracked in #1543
HLD PR: 1957
What did I do?
A new API is being added to sonicv2connector.cpp which can take the db name and host IP as an argument and connect us to the redis instance. The existing API needs the db_name and the host_ip and port (or unix_socket) is decoded using database_config.json. This API is tailored for use cases when you want to connect to the namespace redis instances from the same device. Our use case of aggregating VOQ counters across NPUs involves connecting to a redis instances over midplane IP hence the new API is needed.
How did I test it?
Existing unit test
test_redis_ut.pyis modified to test this. Also verified on Arista chassis (DCS-7804-CH) which had 7800R3A-36D2-LC linecards with all changes related to this feature as present here sonic-net/SONiC#1957