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.

Sign up for GitHub

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

[Namespace]: Reduce time taken during get_all from all namespaces #137

Closed
wants to merge 13 commits into from
Closed

[Namespace]: Reduce time taken during get_all from all namespaces #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ada41e2
Add dict in Namespace class to maintain a mapping of
SuvarnaMeenakshi Jun 30, 2020
6e1c4a7
Remove trailing whitespace.
SuvarnaMeenakshi Jun 30, 2020
9f23f0b
Remove dbs_get_all function.
SuvarnaMeenakshi Jun 30, 2020
220323a
Add function to get interface index from queue key.
SuvarnaMeenakshi Jun 30, 2020
473fd98
Fix whitespace.
SuvarnaMeenakshi Jul 1, 2020
4d529ad
Update as per review comments.
SuvarnaMeenakshi Jul 4, 2020
7c18887
Minor correction.
SuvarnaMeenakshi Jul 4, 2020
4267524
Minor fix, unit-test failure.
SuvarnaMeenakshi Jul 4, 2020
effc505
Merge remote-tracking branch 'sumeenak/master' into time_fix
SuvarnaMeenakshi Jul 7, 2020
60da036
Fix lgtm error.
SuvarnaMeenakshi Jul 7, 2020
90c804f
Minor update as per review comment.
SuvarnaMeenakshi Jul 8, 2020
61c0d91
Remove usage of dbs_get_all.
SuvarnaMeenakshi Jul 8, 2020
87c4b5b
Fix lgtm un-used variable error.
SuvarnaMeenakshi Jul 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 61 additions & 42 deletions src/sonic_ax_impl/mibs/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ def queue_table(sai_id):
def queue_key(port_index, queue_index):
return str(port_index) + ':' + str(queue_index)

def get_port_index_from_queue_key(queue_key):
return int(queue_key.split(':')[0])

def transceiver_info_table(port_name):
"""
:param: port_name: port name
Expand Down Expand Up @@ -469,7 +472,7 @@ class RedisOidTreeUpdater(MIBUpdater):
def __init__(self, prefix_str):
super().__init__()

self.db_conn = Namespace.init_namespace_dbs()
self.db_conn = Namespace.init_namespace_dbs()
if prefix_str.startswith('.'):
prefix_str = prefix_str[1:]
self.prefix_str = prefix_str
Expand All @@ -495,16 +498,18 @@ def update_data(self):
self.oid_list = []
self.oid_map = {}

keys = Namespace.dbs_keys(self.db_conn, SNMP_OVERLAY_DB, self.prefix_str + '*')
Namespace.connect_all_dbs(self.db_conn, SNMP_OVERLAY_DB)
keys = Namespace.dbs_keys_namespace(self.db_conn, SNMP_OVERLAY_DB, self.prefix_str + '*')
# TODO: fix db_conn.keys to return empty list instead of None if there is no match
if keys is None:
keys = []
keys = {}

for key in keys:
db_index = keys[key]
key = key.decode()
oid = oid2tuple(key, dot_prefix=False)
self.oid_list.append(oid)
value = Namespace.dbs_get_all(self.db_conn, SNMP_OVERLAY_DB, key)
value = self.db_conn[db_index].get_all(SNMP_OVERLAY_DB, key)
if value[b'type'] in [b'COUNTER_32', b'COUNTER_64']:
self.oid_map[oid] = int(value[b'data'])
else:
Expand All @@ -520,7 +525,7 @@ def get_oidvalue(self, oid):
class Namespace:
@staticmethod
def init_namespace_dbs():
db_conn= []
db_conn = []
SonicDBConfig.load_sonic_global_db_config()
for namespace in SonicDBConfig.get_ns_list():
db = SonicV2Connector(use_unix_socket_path=True, namespace=namespace)
Expand All @@ -538,7 +543,7 @@ def dbs_keys(dbs, db_name, pattern='*'):
"""
db keys function execute on global and all namespace DBs.
"""
result_keys=[]
result_keys = []
for db_conn in dbs:
db_conn.connect(db_name)
keys = db_conn.keys(db_name, pattern)
Expand All @@ -547,33 +552,35 @@ def dbs_keys(dbs, db_name, pattern='*'):
return result_keys

@staticmethod
def dbs_get_all(dbs, db_name, _hash, *args, **kwargs):
def dbs_keys_namespace(dbs, db_name, pattern='*'):
"""
db get_all function executed on global and all namespace DBs.
dbs_keys_namespace function execute on global
and all namespace DBs. Provides a map of keys
and namespace(db index).
"""
result = {}
for db_conn in dbs:
db_conn.connect(db_name)
if(db_conn.exists(db_name, _hash)):
ns_result = db_conn.get_all(db_name, _hash, *args, **kwargs)
if ns_result is not None:
result.update(ns_result)
return result
result_keys = {}
for db_index in range(len(dbs)):
keys = dbs[db_index].keys(db_name, pattern)
if keys is not None:
keys_ns = dict.fromkeys(keys, db_index)
result_keys.update(keys_ns)
return result_keys

@staticmethod
def get_non_host_dbs(dbs):
def get_non_host_db_indexes(dbs):
"""
From the list of all dbs, return the list of dbs
which will have interface related tables.
For single namespace db, return the single db.
For multiple namespace dbs, return all dbs except the
host namespace db which is the first db in the list.
For multiple namespace dbs, return db index ofall dbs
except the host namespace db which is the first db
in the list.
"""
if len(dbs) == 1:
return dbs
start_index = 0
else:
return dbs[1:]

start_index = 1
return range(start_index, len(dbs))

@staticmethod
def init_namespace_sync_d_interface_tables(dbs):
Copy link
Contributor

@qiluo-msft qiluo-msft Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

init_namespace_sync_d [](start = 8, length = 21)

I see a common pattern in all the functions like init_namespace_sync_d*. They just call the single db function and merge all returned items respectively.

How about abstract this high level function and return a dictionary of list, in this case the type is db -> { if_name_map, if_alias_map, if_id_map, oid_sai_map, oid_name_map, if_oid_namespace }

This comment is also true on dbs_keys_namespace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the MIB implementations, we generally use a single dict to look up the if_index etc. If we have a dict of lists, then we will have to loop through this dict for each instance of the db and look for the if_index, which will make the code look bulky in the places where these functions are used.

Expand All @@ -582,26 +589,29 @@ def init_namespace_sync_d_interface_tables(dbs):
if_id_map = {}
oid_sai_map = {}
oid_name_map = {}
if_oid_namespace = {}

"""
all_ns_db - will have db_conn to all namespace DBs and
global db. First db in the list is global db.
Ignore first global db to get interface tables if there
are multiple namespaces.
"""
for db_conn in Namespace.get_non_host_dbs(dbs):
for db_index in Namespace.get_non_host_db_indexes(dbs):
if_name_map_ns, \
if_alias_map_ns, \
if_id_map_ns, \
oid_sai_map_ns, \
oid_name_map_ns = init_sync_d_interface_tables(db_conn)
oid_name_map_ns = init_sync_d_interface_tables(dbs[db_index])
if_name_map.update(if_name_map_ns)
if_alias_map.update(if_alias_map_ns)
if_id_map.update(if_id_map_ns)
oid_sai_map.update(oid_sai_map_ns)
oid_name_map.update(oid_name_map_ns)
if_oid_namespace_ns = dict.fromkeys(oid_name_map_ns.keys(), db_index)
if_oid_namespace.update(if_oid_namespace_ns)

return if_name_map, if_alias_map, if_id_map, oid_sai_map, oid_name_map
return if_name_map, if_alias_map, if_id_map, oid_sai_map, oid_name_map, if_oid_namespace

@staticmethod
def init_namespace_sync_d_lag_tables(dbs):
Expand All @@ -610,53 +620,62 @@ def init_namespace_sync_d_lag_tables(dbs):
if_name_lag_name_map = {}
oid_lag_name_map = {}
lag_sai_map = {}
oid_lag_namespace = {}

"""
all_ns_db - will have db_conn to all namespace DBs and
global db. First db in the list is global db.
Ignore first global db to get lag tables if
there are multiple namespaces.
"""
for db_conn in Namespace.get_non_host_dbs(dbs):
for db_index in Namespace.get_non_host_db_indexes(dbs):
lag_name_if_name_map_ns, \
if_name_lag_name_map_ns, \
oid_lag_name_map_ns, \
lag_sai_map_ns = init_sync_d_lag_tables(db_conn)
lag_sai_map_ns = init_sync_d_lag_tables(dbs[db_index])
lag_name_if_name_map.update(lag_name_if_name_map_ns)
if_name_lag_name_map.update(if_name_lag_name_map_ns)
oid_lag_name_map.update(oid_lag_name_map_ns)
oid_lag_namespace_ns = dict.fromkeys(oid_lag_name_map_ns.keys(), db_index)
oid_lag_namespace.update(oid_lag_namespace_ns)
lag_sai_map.update(lag_sai_map_ns)

return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map
return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map, oid_lag_namespace

@staticmethod
def init_namespace_sync_d_rif_tables(dbs):
rif_port_map = {}
port_rif_map = {}
port_namespace = {}

for db_conn in Namespace.get_non_host_dbs(dbs):
for db_index in Namespace.get_non_host_db_indexes(dbs):
rif_port_map_ns, \
port_rif_map_ns = init_sync_d_rif_tables(db_conn)
port_rif_map_ns = init_sync_d_rif_tables(dbs[db_index])
rif_port_map.update(rif_port_map_ns)
port_rif_map.update(port_rif_map_ns)
port_namespace_ns = dict.fromkeys(rif_port_map_ns.keys(), db_index)
port_namespace.update(port_namespace_ns)

return rif_port_map, port_rif_map
return rif_port_map, port_rif_map, port_namespace

@staticmethod
def init_namespace_sync_d_vlan_tables(dbs):
vlan_name_map = {}
oid_sai_map = {}
oid_name_map = {}
vlan_oid_namespace = {}

for db_conn in Namespace.get_non_host_dbs(dbs):
for db_index in Namespace.get_non_host_db_indexes(dbs):
vlan_name_map_ns, \
oid_sai_map_ns, \
oid_name_map_ns = init_sync_d_vlan_tables(db_conn)
oid_name_map_ns = init_sync_d_vlan_tables(dbs[db_index])
vlan_name_map.update(vlan_name_map_ns)
oid_sai_map.update(oid_sai_map_ns)
oid_name_map.update(oid_name_map_ns)
vlan_oid_namespace_ns = dict.fromkeys(oid_name_map.keys(), db_index)
vlan_oid_namespace.update(vlan_oid_namespace_ns)

return vlan_name_map, oid_sai_map, oid_name_map
return vlan_name_map, oid_sai_map, oid_name_map, vlan_oid_namespace

@staticmethod
def init_namespace_sync_d_queue_tables(dbs):
Expand All @@ -670,10 +689,10 @@ def init_namespace_sync_d_queue_tables(dbs):
Ignore first global db to get queue tables if there
are multiple namespaces.
"""
for db_conn in Namespace.get_non_host_dbs(dbs):
for db_index in Namespace.get_non_host_db_indexes(dbs):
port_queues_map_ns, \
queue_stat_map_ns, \
port_queue_list_map_ns = init_sync_d_queue_tables(db_conn)
port_queue_list_map_ns = init_sync_d_queue_tables(dbs[db_index])
port_queues_map.update(port_queues_map_ns)
queue_stat_map.update(queue_stat_map_ns)
port_queue_list_map.update(port_queue_list_map_ns)
Expand All @@ -686,15 +705,15 @@ def dbs_get_bridge_port_map(dbs, db_name):
get_bridge_port_map from all namespace DBs
"""
if_br_oid_map = {}
for db_conn in Namespace.get_non_host_dbs(dbs):
if_br_oid_map_ns = port_util.get_bridge_port_map(db_conn)
for db_index in Namespace.get_non_host_db_indexes(dbs):
if_br_oid_map_ns = port_util.get_bridge_port_map(dbs[db_index])
if_br_oid_map.update(if_br_oid_map_ns)
return if_br_oid_map

@staticmethod
def dbs_get_vlan_id_from_bvid(dbs, bvid):
for db_conn in Namespace.get_non_host_dbs(dbs):
db_conn.connect('ASIC_DB')
vlan_obj = db_conn.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid)
for db_index in Namespace.get_non_host_db_indexes(dbs):
dbs[db_index].connect('ASIC_DB')
vlan_obj = dbs[db_index].keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid)
if vlan_obj is not None:
return port_util.get_vlan_id_from_bvid(db_conn, bvid)
return port_util.get_vlan_id_from_bvid(dbs[db_index], bvid)
Loading