Skip to content

Commit 1c633dd

Browse files
zbud-msftliuh-80liuh-80liushilongbuaazjswhhh
authored
Test pipeline (#12)
* Fix memory leak issue in ConfigDBConnector. (sonic-net#655) #### Why I did it Fix memory leak issue in ConfigDBConnector: [chassis] Too many open files error and unable to connect to redis socket error sonic-net/sonic-buildimage#10870 The reason of this issue is DBConnector::pubsub() will return a pointer, and following code call this method but never release the returned pointer: ``` void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bool retry_on) { m_db_name = db_name; m_key_separator = m_table_name_separator = get_db_separator(db_name); SonicV2Connector_Native::connect(m_db_name, retry_on); if (wait_for_init) { auto& client = get_redis_client(m_db_name); auto pubsub = client.pubsub(); <== this pointer not delete later. ``` Also change DBConnector::pubsub() to deprecated for none SWIG scenario. #### How I did it Change DBConnector::pubsub() to return a smart pointer. #### How to verify it Pass all test case. Run following code in python and validate there is no epoll and socket leak: ``` import gc from swsscommon import swsscommon config_db = swsscommon.ConfigDBConnector_Native() config_db.connect() config_db.connect() config_db.connect() gc.collect() ``` #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [x] 202111 #### Description for the changelog Fix epoll and socket resurce leak issue: [chassis] Too many open files error and unable to connect to redis socket error sonic-net/sonic-buildimage#10870 #### Link to config_db schema for YANG module changes #### A picture of a cute animal (not mandatory but encouraged) Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net> * Transfer organization from Azure to sonic-net (sonic-net#656) Transfer organization from Azure to sonic-net * Add docker-mux related table names (sonic-net#627) This PR is to add table name definitions for database entries used by docker mux processes. Sign-off: Jing Zhang zhangjing@microsoft.com * Add libzmq dependency * Add test logs * Add libzmq3-dev dependency * Add libboost-serialization and uuid-dev dependencies * Add boost and uuid * Add installation before building docker Co-authored-by: Hua Liu <58683130+liuh-80@users.noreply.github.com> Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net> Co-authored-by: Liu Shilong <shilongliu@microsoft.com> Co-authored-by: Jing Zhang <zjsw1206@gmail.com> Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>
1 parent 65dc632 commit 1c633dd

File tree

6 files changed

+38
-5
lines changed

6 files changed

+38
-5
lines changed

.azure-pipelines/build-docker-sonic-vs-template.yml

+3
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ jobs:
6262
displayName: "Download sonic-buildimage docker-sonic-vs"
6363
- script: |
6464
set -ex
65+
66+
sudo apt-get install -y uuid-dev libboost-serialization-dev
67+
6568
echo $(Build.DefinitionName).$(Build.BuildNumber)
6669
6770
docker load < $(Build.ArtifactStagingDirectory)/download/target/docker-sonic-vs.gz

azure-pipelines.yml

+4-4
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@ resources:
3030
repositories:
3131
- repository: sonic-sairedis
3232
type: github
33-
name: Azure/sonic-sairedis
34-
endpoint: build
33+
name: sonic-net/sonic-sairedis
34+
endpoint: sonic-net
3535
- repository: sonic-swss
3636
type: github
37-
name: Azure/sonic-swss
38-
endpoint: build
37+
name: sonic-net/sonic-swss
38+
endpoint: sonic-net
3939

4040
parameters:
4141
- name: debian_version

common/configdb.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bo
2424
if (wait_for_init)
2525
{
2626
auto& client = get_redis_client(m_db_name);
27-
auto pubsub = client.pubsub();
27+
auto pubsub = make_shared<PubSub>(&client);
2828
auto initialized = client.get(INIT_INDICATOR);
2929
if (!initialized || initialized->empty())
3030
{

common/dbconnector.h

+3
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ class DBConnector : public RedisContext
174174
/* Create new context to DB */
175175
DBConnector *newConnector(unsigned int timeout) const;
176176

177+
#ifndef SWIG
178+
__attribute__((deprecated))
179+
#endif
177180
PubSub *pubsub();
178181

179182
int64_t del(const std::string &key);

common/schema.h

+11
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ namespace swss {
110110
#define APP_MUX_CABLE_COMMAND_TABLE_NAME "MUX_CABLE_COMMAND_TABLE"
111111
#define APP_MUX_CABLE_RESPONSE_TABLE_NAME "MUX_CABLE_RESPONSE_TABLE"
112112

113+
#define APP_FORWARDING_STATE_COMMAND_TABLE_NAME "FORWARDING_STATE_COMMAND"
114+
#define APP_FORWARDING_STATE_RESPONSE_TABLE_NAME "FORWARDING_STATE_RESPONSE"
115+
116+
#define APP_PEER_PORT_TABLE_NAME "PORT_TABLE_PEER"
117+
#define APP_PEER_HW_FORWARDING_STATE_TABLE_NAME "HW_FORWARDING_STATE_PEER"
118+
113119
#define APP_SYSTEM_PORT_TABLE_NAME "SYSTEM_PORT_TABLE"
114120

115121
#define APP_MACSEC_PORT_TABLE_NAME "MACSEC_PORT_TABLE"
@@ -445,6 +451,11 @@ namespace swss {
445451
#define STATE_HW_MUX_CABLE_TABLE_NAME "HW_MUX_CABLE_TABLE"
446452
#define STATE_MUX_LINKMGR_TABLE_NAME "MUX_LINKMGR_TABLE"
447453
#define STATE_MUX_METRICS_TABLE_NAME "MUX_METRICS_TABLE"
454+
#define STATE_MUX_CABLE_INFO_TABLE_NAME "MUX_CABLE_INFO"
455+
#define STATE_LINK_PROBE_STATS_TABLE_NAME "LINK_PROBE_STATS"
456+
457+
#define STATE_PEER_MUX_METRICS_TABLE_NAME "MUX_METRICS_TABLE_PEER"
458+
#define STATE_PEER_HW_FORWARDING_STATE_TABLE_NAME "HW_MUX_CABLE_TABLE_PEER"
448459

449460
#define STATE_SYSTEM_NEIGH_TABLE_NAME "SYSTEM_NEIGH_TABLE"
450461

tests/test_redis_ut.py

+16
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ def test_ConfigDBConnector():
381381
def test_ConfigDBConnectorSeparator():
382382
db = swsscommon.DBConnector("APPL_DB", 0, True)
383383
config_db = ConfigDBConnector()
384+
# set wait for init to True to cover wait_for_init code.
384385
config_db.db_connect("APPL_DB", False, False)
385386
config_db.get_redis_client(config_db.APPL_DB).flushdb()
386387
config_db.set_entry("TEST_PORT", "Ethernet222", {"alias": "etp2x"})
@@ -696,3 +697,18 @@ def test_SonicV2Connector():
696697
db.set("TEST_DB", "test_key", "field1", 1)
697698
value = db.get("TEST_DB", "test_key", "field1")
698699
assert value == "1"
700+
701+
def test_ConfigDBWaitInit():
702+
config_db = ConfigDBConnector()
703+
config_db.connect(wait_for_init=False)
704+
client = config_db.get_redis_client(config_db.CONFIG_DB)
705+
suc = client.set(config_db.INIT_INDICATOR, 1)
706+
assert suc
707+
708+
# set wait for init to True to cover wait_for_init code.
709+
config_db = ConfigDBConnector()
710+
config_db.db_connect(config_db.CONFIG_DB, True, False)
711+
712+
config_db.set_entry("TEST_PORT", "Ethernet111", {"alias": "etp1x"})
713+
allconfig = config_db.get_config()
714+
assert allconfig["TEST_PORT"]["Ethernet111"]["alias"] == "etp1x"

0 commit comments

Comments
 (0)