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.

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

[snmp]: Remove redundant connections to dbs #264

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SuvarnaMeenakshi
Copy link
Contributor

Signed-off-by: Suvarna Meenakshi [email protected]

- What I did
init_namespace_dbs() function establishes connections to all required DBs in all namespaces.
There are various connections done in periodic functions like reinit_data() or update_data() which creates many connections and increases the number of open fds for sonic_ax_impl processes eventually leading to error logs like:

RR snmp#snmp-subagent [ax_interface] ERROR: MIBUpdater.start() caught an unexpected exception during update_data()#012Traceback (most recent call last):#012  File "/usr/local/lib/python3.7/dist-packages/ax_interface/mib.py", line 37, in start#012    self.reinit_data()#012  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py", line 38, in reinit_data#012    self.oid_name_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_interface_tables, self.db_conn)#012  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 669, in get_sync_d_from_all_namespace#012    ns_tuple = per_namespace_func(db_conn)#012  File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 268, in init_sync_d_interface_tables#012    if_name_map_util, if_id_map_util = port_util.get_interface_oid_map(db_conn, blocking=False)#012  File "/usr/local/lib/python3.7/dist-packages/swsssdk/port_util.py", line 73, in get_interface_oid_map#012    db.connect('COUNTERS_DB')#012  File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1690, in connect#012    return _swsscommon.SonicV2Connector_Native_connect(self, db_name, retry_on)#012RuntimeError: Unable to connect to redis (unix-socket): Cannot assign requested address

- How I did it
Remove establishing connections in periodic functions when the connection is already established during init.

- How to verify it

- Description for the changelog

db connect is done to all required DBs in the initialization of
each MIB class

Signed-off-by: Suvarna Meenakshi <[email protected]>
@qiluo-msft
Copy link
Contributor

def __init__(self):

This is not "periodic functions", right?


Refers to: src/sonic_ax_impl/mibs/ieee802_1ab.py:152 in d845a95. [](commit_id = d845a95, deletion_comment = False)

@qiluo-msft
Copy link
Contributor

def __init__(self):

Not "periodic functions"?


Refers to: src/sonic_ax_impl/mibs/ieee802_1ab.py:500 in d845a95. [](commit_id = d845a95, deletion_comment = False)

@@ -232,9 +232,6 @@ def init_mgmt_interface_tables(db_conn):
:return: tuple of mgmt name to oid map and mgmt name to alias map
"""

db_conn.connect(CONFIG_DB)
Copy link
Contributor

Choose a reason for hiding this comment

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

connect

The expectation of SonicV2Connector is that connect to the same DB multiple time will be harmless and no resource leak. We need to dig deeper and check if the expectation is broken.

Copy link

Choose a reason for hiding this comment

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

Redis DB is not meant to handle too many connections ...there is a limit to number of connections. Each process should establish connection once in its life time and then use the same connection to do any queries ...any number of times. Too many connections request may result in hitting this error

@SuvarnaMeenakshi
Copy link
Contributor Author

def __init__(self):

This is not "periodic functions", right?

Refers to: src/sonic_ax_impl/mibs/ieee802_1ab.py:152 in d845a95. [](commit_id = d845a95, deletion_comment = False)

Yes it is not periodic, but in places where we do "init_namespace_dbs()", we do not need to connect again as "init_namespace_dbs" also connects to all required DBs in all namespaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants