[Namespace]: Fix SNMP AgentX socket connection timeout when using Namespace.get_all() #140

SuvarnaMeenakshi · 2020-07-10T06:43:44Z

Signed-off-by: SuvarnaMeenakshi sumeenak@microsoft.com

- 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 and does db connect before executing get_all.
Both of this operation causes additional time, which was causing agentx socket to disconnect.

- How I did it

Remove the additional check that was added to see if the key exists before doing a get all.
Remove db connect from dbs_get_all , as this was adding additional delay in dbs_get_all function, which is called during update_data. Instead, connect to all dbs during initialization, and connect to required dbs before invoking sync_d functions to ensure connection persists periodically.

- How to verify it
For single-asic platform and multi-asic, execute snmp queries to verify that the agentx socket does not disconnect and time issue is seen.

- Description for the changelog

to reduce time taken. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

src/sonic_ax_impl/mibs/__init__.py

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

abdosi

LGTM

sync_d functions to reduce time taken in dbs_get_all. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

The Namespace functions will be used for both single and multi-asic platforms. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

qiluo-msft · 2020-07-10T23:56:20Z

src/sonic_ax_impl/mibs/__init__.py

+        # If there are multiple namespaces, _hash might not be 
+        # present in all namespace, ignore if not present in a
+        # specfic namespace.
+        if (len(dbs) > 1) : kwargs['blocking'] = False


kwargs['blocking'] = False [](start = 28, length = 26)

Please move it in next line. #Closed

fixed as per comment.

qiluo-msft · 2020-07-10T23:56:33Z

src/sonic_ax_impl/mibs/__init__.py

+        # If there are multiple namespaces, _hash might not be 
+        # present in all namespace, ignore if not present in a
+        # specfic namespace.
+        if (len(dbs) > 1) : kwargs['blocking'] = False


( [](start = 11, length = 1)

you can remove '()' #Closed

Fixed as per comment.

qiluo-msft · 2020-07-11T00:00:41Z

src/sonic_ax_impl/mibs/__init__.py

+        # If there are multiple namespaces, _hash might not be 
+        # present in all namespace, ignore if not present in a
+        # specfic namespace.
+        if (len(dbs) > 1) : kwargs['blocking'] = False


kwargs [](start = 28, length = 6)

This parameter is passed as reference, and it is mutable.
If you change it inside function, it will have side-effect to the caller's scope. Could you do not change it? #Closed

@qiluo-msft We were initially updating all the function call arguments and the changes were spread in multiple places. This approach looks better where only for Multi-Asic we are making blocking as False and for single asic behavior remains unchanged.
Also caller should not be impacted by this as it does not get used in caller scope and just passes the value to the library.

As per suggestion, instead of modifying kwargs directly, did a shallow copy to change.

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

do a shallow copy instead. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

…espace.get_all() (#140) * [Namespace]: Remove key exists check in dbs_get_all to reduce time taken. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Fix as per review comment. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Minor fix. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Connect to all required namespace dbs during init and periodic sync_d functions to reduce time taken in dbs_get_all. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Change test_mibs to use generic wrapper function added. The Namespace functions will be used for both single and multi-asic platforms. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Fix review comment. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Fix as per review comment. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Remove whitespace. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com> * Fix review comment. Avoid modifying mutable object kwargs, do a shallow copy instead. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

[Namespace]: Remove key exists check in dbs_get_all

16f36d8

to reduce time taken. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi requested review from qiluo-msft and abdosi July 10, 2020 06:46

abdosi reviewed Jul 10, 2020

View reviewed changes

src/sonic_ax_impl/mibs/__init__.py Outdated Show resolved Hide resolved

abdosi reviewed Jul 10, 2020

View reviewed changes

src/sonic_ax_impl/mibs/__init__.py Outdated Show resolved Hide resolved

SuvarnaMeenakshi added 2 commits July 10, 2020 11:29

Fix as per review comment.

af7d4f4

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Minor fix.

7cfc43d

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

abdosi approved these changes Jul 10, 2020

View reviewed changes

abdosi previously approved these changes Jul 10, 2020

View reviewed changes

SuvarnaMeenakshi added 2 commits July 10, 2020 13:58

Connect to all required namespace dbs during init and periodic

e264498

sync_d functions to reduce time taken in dbs_get_all. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Change test_mibs to use generic wrapper function added.

a10f55f

The Namespace functions will be used for both single and multi-asic platforms. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

SuvarnaMeenakshi dismissed abdosi’s stale review via a10f55f July 10, 2020 21:00

abdosi changed the title ~~[Namespace]: Remove exists check in Namespace get_all~~ [Namespace]: Fix SNMP Agent X socket connection timeout when using Namespace.get_all() Jul 10, 2020

qiluo-msft changed the title ~~[Namespace]: Fix SNMP Agent X socket connection timeout when using Namespace.get_all()~~ [Namespace]: Fix SNMP AgentX socket connection timeout when using Namespace.get_all() Jul 10, 2020

qiluo-msft reviewed Jul 10, 2020

View reviewed changes

qiluo-msft reviewed Jul 11, 2020

View reviewed changes

SuvarnaMeenakshi added 4 commits July 10, 2020 17:11

Fix review comment.

2fd0960

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Fix as per review comment.

7e9eb50

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Remove whitespace.

9c4efd6

Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

Fix review comment. Avoid modifying mutable object kwargs,

fbba260

do a shallow copy instead. Signed-off-by: SuvarnaMeenakshi <sumeenak@microsoft.com>

qiluo-msft approved these changes Jul 11, 2020

View reviewed changes

abdosi added the Request for 201911 Branch label Jul 11, 2020

abdosi merged commit 1d210d9 into sonic-net:master Jul 11, 2020

abdosi added the Included in 201911 Branch label Jul 11, 2020

abdosi mentioned this pull request Jul 11, 2020

[201911]Cherry-pick Master PR's to 201911 #143

Closed

SuvarnaMeenakshi mentioned this pull request Jul 20, 2020

[Namespace]: Fix to reduce time and fix counters related MIB #139

Closed

SuvarnaMeenakshi mentioned this pull request Jul 28, 2020

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Namespace]: Fix SNMP AgentX socket connection timeout when using Namespace.get_all() #140

[Namespace]: Fix SNMP AgentX socket connection timeout when using Namespace.get_all() #140

SuvarnaMeenakshi commented Jul 10, 2020 •

edited by abdosi

abdosi left a comment

qiluo-msft Jul 10, 2020 •

edited

SuvarnaMeenakshi Jul 11, 2020

qiluo-msft Jul 10, 2020 •

edited

SuvarnaMeenakshi Jul 11, 2020

qiluo-msft Jul 11, 2020 •

edited

abdosi Jul 11, 2020

SuvarnaMeenakshi Jul 11, 2020

[Namespace]: Fix SNMP AgentX socket connection timeout when using Namespace.get_all() #140

[Namespace]: Fix SNMP AgentX socket connection timeout when using Namespace.get_all() #140

Conversation

SuvarnaMeenakshi commented Jul 10, 2020 • edited by abdosi

abdosi left a comment

Choose a reason for hiding this comment

qiluo-msft Jul 10, 2020 • edited

Choose a reason for hiding this comment

SuvarnaMeenakshi Jul 11, 2020

Choose a reason for hiding this comment

qiluo-msft Jul 10, 2020 • edited

Choose a reason for hiding this comment

SuvarnaMeenakshi Jul 11, 2020

Choose a reason for hiding this comment

qiluo-msft Jul 11, 2020 • edited

Choose a reason for hiding this comment

abdosi Jul 11, 2020

Choose a reason for hiding this comment

SuvarnaMeenakshi Jul 11, 2020

Choose a reason for hiding this comment

SuvarnaMeenakshi commented Jul 10, 2020 •

edited by abdosi

qiluo-msft Jul 10, 2020 •

edited

qiluo-msft Jul 10, 2020 •

edited

qiluo-msft Jul 11, 2020 •

edited