Skip to content

Commit

Permalink
Test pipeline (#12)
Browse files Browse the repository at this point in the history
* 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 [email protected]

* 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 <[email protected]>
Co-authored-by: liuh-80 <azureuser@liuh-dev-vm-02.5fg3zjdzj2xezlx1yazx5oxkzd.hx.internal.cloudapp.net>
Co-authored-by: Liu Shilong <[email protected]>
Co-authored-by: Jing Zhang <[email protected]>
Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>
  • Loading branch information
6 people authored Aug 3, 2022
1 parent 65dc632 commit 1c633dd
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 5 deletions.
3 changes: 3 additions & 0 deletions .azure-pipelines/build-docker-sonic-vs-template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ jobs:
displayName: "Download sonic-buildimage docker-sonic-vs"
- script: |
set -ex
sudo apt-get install -y uuid-dev libboost-serialization-dev
echo $(Build.DefinitionName).$(Build.BuildNumber)
docker load < $(Build.ArtifactStagingDirectory)/download/target/docker-sonic-vs.gz
Expand Down
8 changes: 4 additions & 4 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ resources:
repositories:
- repository: sonic-sairedis
type: github
name: Azure/sonic-sairedis
endpoint: build
name: sonic-net/sonic-sairedis
endpoint: sonic-net
- repository: sonic-swss
type: github
name: Azure/sonic-swss
endpoint: build
name: sonic-net/sonic-swss
endpoint: sonic-net

parameters:
- name: debian_version
Expand Down
2 changes: 1 addition & 1 deletion common/configdb.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ void ConfigDBConnector_Native::db_connect(string db_name, bool wait_for_init, bo
if (wait_for_init)
{
auto& client = get_redis_client(m_db_name);
auto pubsub = client.pubsub();
auto pubsub = make_shared<PubSub>(&client);
auto initialized = client.get(INIT_INDICATOR);
if (!initialized || initialized->empty())
{
Expand Down
3 changes: 3 additions & 0 deletions common/dbconnector.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ class DBConnector : public RedisContext
/* Create new context to DB */
DBConnector *newConnector(unsigned int timeout) const;

#ifndef SWIG
__attribute__((deprecated))
#endif
PubSub *pubsub();

int64_t del(const std::string &key);
Expand Down
11 changes: 11 additions & 0 deletions common/schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ namespace swss {
#define APP_MUX_CABLE_COMMAND_TABLE_NAME "MUX_CABLE_COMMAND_TABLE"
#define APP_MUX_CABLE_RESPONSE_TABLE_NAME "MUX_CABLE_RESPONSE_TABLE"

#define APP_FORWARDING_STATE_COMMAND_TABLE_NAME "FORWARDING_STATE_COMMAND"
#define APP_FORWARDING_STATE_RESPONSE_TABLE_NAME "FORWARDING_STATE_RESPONSE"

#define APP_PEER_PORT_TABLE_NAME "PORT_TABLE_PEER"
#define APP_PEER_HW_FORWARDING_STATE_TABLE_NAME "HW_FORWARDING_STATE_PEER"

#define APP_SYSTEM_PORT_TABLE_NAME "SYSTEM_PORT_TABLE"

#define APP_MACSEC_PORT_TABLE_NAME "MACSEC_PORT_TABLE"
Expand Down Expand Up @@ -445,6 +451,11 @@ namespace swss {
#define STATE_HW_MUX_CABLE_TABLE_NAME "HW_MUX_CABLE_TABLE"
#define STATE_MUX_LINKMGR_TABLE_NAME "MUX_LINKMGR_TABLE"
#define STATE_MUX_METRICS_TABLE_NAME "MUX_METRICS_TABLE"
#define STATE_MUX_CABLE_INFO_TABLE_NAME "MUX_CABLE_INFO"
#define STATE_LINK_PROBE_STATS_TABLE_NAME "LINK_PROBE_STATS"

#define STATE_PEER_MUX_METRICS_TABLE_NAME "MUX_METRICS_TABLE_PEER"
#define STATE_PEER_HW_FORWARDING_STATE_TABLE_NAME "HW_MUX_CABLE_TABLE_PEER"

#define STATE_SYSTEM_NEIGH_TABLE_NAME "SYSTEM_NEIGH_TABLE"

Expand Down
16 changes: 16 additions & 0 deletions tests/test_redis_ut.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ def test_ConfigDBConnector():
def test_ConfigDBConnectorSeparator():
db = swsscommon.DBConnector("APPL_DB", 0, True)
config_db = ConfigDBConnector()
# set wait for init to True to cover wait_for_init code.
config_db.db_connect("APPL_DB", False, False)
config_db.get_redis_client(config_db.APPL_DB).flushdb()
config_db.set_entry("TEST_PORT", "Ethernet222", {"alias": "etp2x"})
Expand Down Expand Up @@ -696,3 +697,18 @@ def test_SonicV2Connector():
db.set("TEST_DB", "test_key", "field1", 1)
value = db.get("TEST_DB", "test_key", "field1")
assert value == "1"

def test_ConfigDBWaitInit():
config_db = ConfigDBConnector()
config_db.connect(wait_for_init=False)
client = config_db.get_redis_client(config_db.CONFIG_DB)
suc = client.set(config_db.INIT_INDICATOR, 1)
assert suc

# set wait for init to True to cover wait_for_init code.
config_db = ConfigDBConnector()
config_db.db_connect(config_db.CONFIG_DB, True, False)

config_db.set_entry("TEST_PORT", "Ethernet111", {"alias": "etp1x"})
allconfig = config_db.get_config()
assert allconfig["TEST_PORT"]["Ethernet111"]["alias"] == "etp1x"

0 comments on commit 1c633dd

Please sign in to comment.