Skip to content

Commit 1a15091

Browse files
authored
Fix multi asic connection creation (sonic-net#4109)
- What I did Create a cache for the SonicV2Connector objects which are created, because currently we are creating n interfaces * m namespace amount of connectors in case of multi asic implementation, which is very high and would lead to the show interface counters command to crash root@sonic:/home/admin# show interfaces counters Traceback (most recent call last): File "/usr/local/bin/portstat", line 168, in main() File "/usr/local/bin/portstat", line 158, in main portstat.cnstat_diff_print(cnstat_dict, {}, ratestat_dict, intf_list, use_json, print_all, errors_only, File "/usr/local/lib/python3.11/dist-packages/utilities_common/portstat.py", line 572, in cnstat_diff_print port_speed = self.get_port_speed(key) ^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.11/dist-packages/utilities_common/portstat.py", line 373, in get_port_speed self.db = multi_asic.connect_to_all_dbs_for_ns(ns) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.11/dist-packages/sonic_py_common/multi_asic.py", line 81, in connect_to_all_dbs_for_ns db.connect(db_id) File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 2069, in connect return _swsscommon.SonicV2Connector_Native_connect(self, db_name, retry_on) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RuntimeError: Unable to connect to redis - Cannot assign requested address(1): Cannot assign requested address - How I did it Cache the connectors in a dictionary - How to verify it Run show interfaces counters command Signed-off-by: gpunathilell <[email protected]>
1 parent 137d594 commit 1a15091

2 files changed

Lines changed: 79 additions & 3 deletions

File tree

tests/portstat_test.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,3 +1125,74 @@ def teardown_class(cls):
11251125
os.environ["UTILITIES_UNIT_TESTING"] = "0"
11261126
os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = ""
11271127
remove_tmp_cnstat_file()
1128+
1129+
1130+
class TestPortstatGetDbClient(object):
1131+
"""Test the get_db_client method for caching DB connections"""
1132+
1133+
def test_get_db_client_creates_new_connection(self):
1134+
"""Test that get_db_client creates a new connection when none exists"""
1135+
from unittest import mock
1136+
from utilities_common.portstat import Portstat
1137+
from utilities_common.constants import DEFAULT_NAMESPACE
1138+
1139+
portstat = Portstat(namespace=DEFAULT_NAMESPACE, display_option='')
1140+
1141+
with mock.patch('sonic_py_common.multi_asic.connect_to_all_dbs_for_ns') as mock_connect:
1142+
mock_db_client = mock.MagicMock()
1143+
mock_connect.return_value = mock_db_client
1144+
1145+
result = portstat.get_db_client('asic0')
1146+
1147+
mock_connect.assert_called_once_with('asic0')
1148+
1149+
assert result == mock_db_client
1150+
1151+
assert portstat.db_clients['asic0'] == mock_db_client
1152+
1153+
def test_get_db_client_returns_cached_connection(self):
1154+
"""Test that get_db_client returns cached connection on subsequent calls"""
1155+
from unittest import mock
1156+
from utilities_common.portstat import Portstat
1157+
from utilities_common.constants import DEFAULT_NAMESPACE
1158+
1159+
portstat = Portstat(namespace=DEFAULT_NAMESPACE, display_option='')
1160+
1161+
with mock.patch('sonic_py_common.multi_asic.connect_to_all_dbs_for_ns') as mock_connect:
1162+
mock_db_client = mock.MagicMock()
1163+
mock_connect.return_value = mock_db_client
1164+
result1 = portstat.get_db_client('asic0')
1165+
result2 = portstat.get_db_client('asic0')
1166+
mock_connect.assert_called_once_with('asic0')
1167+
1168+
assert result1 == result2
1169+
assert result1 == mock_db_client
1170+
1171+
def test_get_db_client_multiple_namespaces(self):
1172+
"""Test that get_db_client handles multiple namespaces correctly"""
1173+
from unittest import mock
1174+
from utilities_common.portstat import Portstat
1175+
from utilities_common.constants import DEFAULT_NAMESPACE
1176+
1177+
portstat = Portstat(namespace=DEFAULT_NAMESPACE, display_option='')
1178+
1179+
with mock.patch('sonic_py_common.multi_asic.connect_to_all_dbs_for_ns') as mock_connect:
1180+
mock_db_client_asic0 = mock.MagicMock()
1181+
mock_db_client_asic1 = mock.MagicMock()
1182+
1183+
def side_effect(ns):
1184+
if ns == 'asic0':
1185+
return mock_db_client_asic0
1186+
elif ns == 'asic1':
1187+
return mock_db_client_asic1
1188+
1189+
mock_connect.side_effect = side_effect
1190+
result_asic0 = portstat.get_db_client('asic0')
1191+
result_asic1 = portstat.get_db_client('asic1')
1192+
assert mock_connect.call_count == 2
1193+
mock_connect.assert_any_call('asic0')
1194+
mock_connect.assert_any_call('asic1')
1195+
assert result_asic0 == mock_db_client_asic0
1196+
assert result_asic1 == mock_db_client_asic1
1197+
assert portstat.db_clients['asic0'] == mock_db_client_asic0
1198+
assert portstat.db_clients['asic1'] == mock_db_client_asic1

utilities_common/portstat.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ def __init__(self, namespace, display_option):
189189
if device_info.is_supervisor():
190190
self.db = SonicV2Connector(use_unix_socket_path=False)
191191
self.db.connect(self.db.CHASSIS_STATE_DB, False)
192-
192+
self.db_clients = {}
193193
self.sorted = natsorted
194194

195195
def get_cnstat_dict(self):
@@ -394,7 +394,7 @@ def get_port_speed(self, port_name):
394394
state_db_table_id = PORT_STATE_TABLE_PREFIX + port_name
395395
app_db_table_id = PORT_STATUS_TABLE_PREFIX + port_name
396396
for ns in self.multi_asic.get_ns_list_based_on_options():
397-
self.db = multi_asic.connect_to_all_dbs_for_ns(ns)
397+
self.db = self.get_db_client(ns)
398398
speed = self.db.get(self.db.STATE_DB, state_db_table_id, PORT_SPEED_FIELD)
399399
oper_status = self.db.get(self.db.APPL_DB, app_db_table_id, PORT_OPER_STATUS_FIELD)
400400
if speed is None or speed == STATUS_NA or oper_status != "up":
@@ -403,6 +403,11 @@ def get_port_speed(self, port_name):
403403
return int(speed)
404404
return STATUS_NA
405405

406+
def get_db_client(self, ns):
407+
if not self.db_clients.get(ns):
408+
self.db_clients[ns] = multi_asic.connect_to_all_dbs_for_ns(ns)
409+
return self.db_clients[ns]
410+
406411
def get_port_state(self, port_name):
407412
"""
408413
Get the port state
@@ -416,7 +421,7 @@ def get_port_state(self, port_name):
416421

417422
full_table_id = PORT_STATUS_TABLE_PREFIX + port_name
418423
for ns in self.multi_asic.get_ns_list_based_on_options():
419-
self.db = multi_asic.connect_to_all_dbs_for_ns(ns)
424+
self.db = self.get_db_client(ns)
420425
admin_state = self.db.get(self.db.APPL_DB, full_table_id, PORT_ADMIN_STATUS_FIELD)
421426
oper_state = self.db.get(self.db.APPL_DB, full_table_id, PORT_OPER_STATUS_FIELD)
422427

0 commit comments

Comments
 (0)