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

SuvarnaMeenakshi · 2020-06-30T21:06:07Z

- What I did
To support namespaces, we added a Namespace class with functions to support getting data from all databases.
The Namespace dbs_get_all function , checks if the key exists in the database before executing get_all.
This additional check was causing additional time, which was causing agentx socket to disconnect.

- How I did it

Added new Namespace class variables to get the mapping of interface index and namespace(or database instance).
Use the new mapping to directly execute get_all from that specific database instance instead of checking if the key exists in the database.

- How to verify it
Make sure unit-test passes.
Make sure single-asic platform test cases are passing and no error message is observed in the syslog.
Check snmp output in multi-asic platform.

- Description for the changelog

interface index and database instance to avoid checking if a key exists every time for get_all. Signed-off-by: SuvarnaMeenakshi <[email protected]>

Signed-off-by: SuvarnaMeenakshi <[email protected]>

qiluo-msft · 2020-07-02T23:57:49Z

src/sonic_ax_impl/mibs/__init__.py


        """
        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 inst in range(Namespace.get_start_idx_of_non_host(dbs), len(dbs)):


inst [](start = 12, length = 4)

ns_index or ns ? #Closed

ns_index; basically, the index in db array.
index is not same as namespace, index 0 will be host namespace.

Then call it db_index ?

In reply to: 449314977 [](ancestors = 449314977)

updated as per comment

qiluo-msft · 2020-07-03T00:34:58Z

src/sonic_ax_impl/mibs/ieee802_1ab.py

            self.if_range.append((if_oid, ))
        self.if_range.sort()
        if not self.loc_port_data:
            logger.warning("0 - b'PORT_TABLE' is empty. No local port information could be retrieved.")

-    def _get_if_entry(self, if_name):
+    def _get_if_entry(self, if_name, db_inst):


db_inst [](start = 37, length = 7)

db_index? #Closed

updated as per comment

qiluo-msft · 2020-07-03T01:10:42Z

src/sonic_ax_impl/mibs/__init__.py

+        if len(dbs) == 1:
+            return 0
+        else:
+            return 1

    @staticmethod
    def init_namespace_sync_d_interface_tables(dbs):


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

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.

qiluo-msft · 2020-07-03T01:14:54Z

src/sonic_ax_impl/mibs/__init__.py

-
+
+    @staticmethod
+    def get_start_idx_of_non_host(dbs):


get_start_idx_of_non_host [](start = 8, length = 25)

You can return a range since all use cases are in a range. You can further implement get_non_host_dbs by this function.

A better name will be get_non_host_db_indexes #Closed

updated as per comment.

qiluo-msft

As comments

Signed-off-by: SuvarnaMeenakshi <[email protected]>

lgtm-com · 2020-07-07T21:45:54Z

This pull request introduces 1 alert when merging effc505 into 253f58e - view on LGTM.com

new alerts:

1 for Syntax error

Signed-off-by: SuvarnaMeenakshi <[email protected]>

qiluo-msft · 2020-07-07T22:15:17Z

src/sonic_ax_impl/mibs/ietf/rfc2737.py

@@ -233,9 +233,9 @@ def update_data(self):
            if not self.pubsub[i]:
                pattern = self.TRANSCEIVER_KEY_PATTERN
                self.pubsub[i] = mibs.get_redis_pubsub(self.statedb[i], self.statedb[i].STATE_DB, pattern)


~~Move to __init__() ?~~

This function is currently in sonic_ax_impl/mibs/init.py

Sorry, I mean the section is only needed once after each time db connected.

for i in range(len(self.statedb)): if not self.pubsub[i]: pattern = self.TRANSCEIVER_KEY_PATTERN self.pubsub[i] = mibs.get_redis_pubsub(self.statedb[i], self.statedb[i].STATE_DB, pattern)

In reply to: 451174442 [](ancestors = 451174442)

qiluo-msft · 2020-07-07T22:16:57Z

src/sonic_ax_impl/mibs/ietf/rfc2737.py


-    def _update_per_namespace_data(self, pubsub):
+    def _update_per_namespace_data(self, pubsub, db_instance):


db_instance [](start = 49, length = 11)

db_index #Closed

updated as per comment.

Signed-off-by: SuvarnaMeenakshi <[email protected]>

lgtm-com · 2020-07-08T04:11:39Z

This pull request introduces 1 alert when merging 61c0d91 into 253f58e - view on LGTM.com

new alerts:

1 for Unused local variable

Signed-off-by: SuvarnaMeenakshi <[email protected]>

SuvarnaMeenakshi · 2020-07-28T19:25:45Z

Will raise a new PR to fix the comment:
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

Time reduction and socket connection error is fixed in PR #140

SuvarnaMeenakshi added 2 commits June 30, 2020 13:28

Add dict in Namespace class to maintain a mapping of

ada41e2

interface index and database instance to avoid checking if a key exists every time for get_all. Signed-off-by: SuvarnaMeenakshi <[email protected]>

Remove trailing whitespace.

6e1c4a7

Signed-off-by: SuvarnaMeenakshi <[email protected]>

SuvarnaMeenakshi requested a review from qiluo-msft June 30, 2020 21:06

SuvarnaMeenakshi added 3 commits June 30, 2020 14:19

Remove dbs_get_all function.

9f23f0b

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Add function to get interface index from queue key.

220323a

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Fix whitespace.

473fd98

Signed-off-by: SuvarnaMeenakshi <[email protected]>

qiluo-msft reviewed Jul 2, 2020

View reviewed changes

qiluo-msft reviewed Jul 3, 2020

View reviewed changes

qiluo-msft requested changes Jul 3, 2020

View reviewed changes

SuvarnaMeenakshi added 4 commits July 4, 2020 15:52

Update as per review comments.

4d529ad

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Minor correction.

7c18887

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Minor fix, unit-test failure.

4267524

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Merge remote-tracking branch 'sumeenak/master' into time_fix

effc505

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Fix lgtm error.

60da036

Signed-off-by: SuvarnaMeenakshi <[email protected]>

qiluo-msft reviewed Jul 7, 2020

View reviewed changes

SuvarnaMeenakshi added 2 commits July 7, 2020 20:59

Minor update as per review comment.

90c804f

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Remove usage of dbs_get_all.

61c0d91

Signed-off-by: SuvarnaMeenakshi <[email protected]>

Fix lgtm un-used variable error.

87c4b5b

Signed-off-by: SuvarnaMeenakshi <[email protected]>

SuvarnaMeenakshi mentioned this pull request Jul 21, 2020

[Multiasic]: Provide namespace support for ipNetToMediaPhysAddress #129

Merged

SuvarnaMeenakshi closed this Jul 28, 2020

qiluo-msft mentioned this pull request Aug 5, 2020

[Namespace]: Fix SAI_ID key used in cpfcIfTable and csqIfQosGroupStatsTable implementation #138

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

SuvarnaMeenakshi commented Jun 30, 2020

qiluo-msft Jul 2, 2020 •

edited

Loading

SuvarnaMeenakshi Jul 3, 2020

qiluo-msft Jul 3, 2020

SuvarnaMeenakshi Jul 4, 2020

qiluo-msft Jul 3, 2020 •

edited

Loading

SuvarnaMeenakshi Jul 4, 2020

qiluo-msft Jul 3, 2020 •

edited

Loading

SuvarnaMeenakshi Jul 4, 2020

qiluo-msft Jul 3, 2020 •

edited

Loading

SuvarnaMeenakshi Jul 4, 2020

qiluo-msft left a comment

lgtm-com bot commented Jul 7, 2020

qiluo-msft Jul 7, 2020 •

edited

Loading

SuvarnaMeenakshi Jul 7, 2020

qiluo-msft Jul 7, 2020

qiluo-msft Jul 7, 2020 •

edited

Loading

SuvarnaMeenakshi Jul 8, 2020

lgtm-com bot commented Jul 8, 2020

SuvarnaMeenakshi commented Jul 28, 2020


		def _update_per_namespace_data(self, pubsub):
		def _update_per_namespace_data(self, pubsub, db_instance):

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

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

Conversation

SuvarnaMeenakshi commented Jun 30, 2020

qiluo-msft Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Jul 7, 2020

qiluo-msft Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Jul 8, 2020

SuvarnaMeenakshi commented Jul 28, 2020

qiluo-msft Jul 2, 2020 •

edited

Loading

qiluo-msft Jul 3, 2020 •

edited

Loading

qiluo-msft Jul 3, 2020 •

edited

Loading

qiluo-msft Jul 3, 2020 •

edited

Loading

qiluo-msft Jul 7, 2020 •

edited

Loading

qiluo-msft Jul 7, 2020 •

edited

Loading