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.

Sign up for GitHub

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

Jump to bottom

Fix epoll and socket resource leak issue. #651

Merged
merged 2 commits into from
Jul 23, 2022
Merged

Fix epoll and socket resource leak issue. #651

merged 2 commits into from
Jul 23, 2022

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Jul 21, 2022 •
edited
Loading

Why I did it

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

The reason of this issue is in SWIG any method return a new object need decorate with %newobject, so SWIG will generate code to release C++ object when python wrapper object released:
https://www.swig.org/Doc4.0/SWIGDocumentation.html#Customization_ownership

How I did it

Update swsscommon.i to decorate return new object methods with %newobject

How to verify it

Pass all test case.

Run following code in python and validate there is no epoll and socket leak:

from swsscommon.swsscommon import SonicDBConfig
from swsscommon.swsscommon import SonicV2Connector
import gc

SonicDBConfig.load_sonic_global_db_config()
SonicDBConfig.get_ns_list()
db = SonicV2Connector(use_unix_socket_path=True, namespace='')
db.connect("CONFIG_DB")
db.get_redis_client("CONFIG_DB")
client = db.get_redis_client("CONFIG_DB")
client.pubsub()
client.pubsub()
client.pubsub()
client.newConnector(0)
client.newConnector(0)
client.newConnector(0)

gc.collect()

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 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)

liuh-80
liuh-80 commented Jul 21, 2022
View reviewed changes
pyext/swsscommon.i Outdated
@@ -158,6 +158,14 @@ T castSelectableObj(swss::Selectable *temp)
%template(hgetall) hgetall<std::map<std::string, std::string>>;
}

%extend swss::PubSub {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WIll delete this code.
Also will check if other method need decorate with %newobject

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, these 2 methods are all methods return new object in sonic-swss-common.

@liuh-80 liuh-80 mentioned this pull request Jul 21, 2022
Merged
5 tasks
@liuh-80 liuh-80 marked this pull request as ready for review July 21, 2022 07:23
@liuh-80 liuh-80 requested a review from qiluo-msft July 21, 2022 07:30
@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 21, 2022

sairedis build failed because a known issue. will retry till it success.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 21, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abdosi
Copy link
Contributor

abdosi commented Jul 21, 2022 •
edited
Loading

can we add UT that can cover this scenario.

@prgeor prgeor added the Bug label Jul 21, 2022
@qiluo-msft
Copy link
Contributor

I see the manual testcase in "How to verify it" section. Is it possible to automate the test as unit test or sonic-mgmt test?

@liuh-80 liuh-80 merged commit d72e5ea into sonic-net:master Jul 23, 2022
lukasstockner pushed a commit to genesiscloud/sonic-swss-common that referenced this pull request Nov 10, 2022
@liuh-80 @lukasstockner
Fix epoll and socket resource leak issue. (sonic-net#651)
a823c48 
#### Why I did it
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

The reason of this issue is in SWIG any method return a new object need decorate with %newobject, so SWIG will generate code to release C++ object when python wrapper object released:
https://www.swig.org/Doc4.0/SWIGDocumentation.html#Customization_ownership


#### How I did it
Update swsscommon.i to decorate return new object methods with %newobject

#### How to verify it
Pass all test case.

Run following code in python and validate there is no epoll and socket leak:

from swsscommon.swsscommon import SonicDBConfig
from swsscommon.swsscommon import SonicV2Connector
import gc

SonicDBConfig.load_sonic_global_db_config()
SonicDBConfig.get_ns_list()
db = SonicV2Connector(use_unix_socket_path=True, namespace='')
db.connect("CONFIG_DB")
db.get_redis_client("CONFIG_DB")
client = db.get_redis_client("CONFIG_DB")
client.pubsub()
client.pubsub()
client.pubsub()
client.newConnector(0)
client.newConnector(0)
client.newConnector(0)

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)
kpetremann pushed a commit to criteo-forks/sonic-swss-common that referenced this pull request Jan 19, 2023
@liuh-80 @kpetremann
Fix epoll and socket resource leak issue. (sonic-net#651)
67a2692 
#### Why I did it
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

The reason of this issue is in SWIG any method return a new object need decorate with %newobject, so SWIG will generate code to release C++ object when python wrapper object released:
https://www.swig.org/Doc4.0/SWIGDocumentation.html#Customization_ownership


#### How I did it
Update swsscommon.i to decorate return new object methods with %newobject

#### How to verify it
Pass all test case.

Run following code in python and validate there is no epoll and socket leak:

from swsscommon.swsscommon import SonicDBConfig
from swsscommon.swsscommon import SonicV2Connector
import gc

SonicDBConfig.load_sonic_global_db_config()
SonicDBConfig.get_ns_list()
db = SonicV2Connector(use_unix_socket_path=True, namespace='')
db.connect("CONFIG_DB")
db.get_redis_client("CONFIG_DB")
client = db.get_redis_client("CONFIG_DB")
client.pubsub()
client.pubsub()
client.pubsub()
client.newConnector(0)
client.newConnector(0)
client.newConnector(0)

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)
@kpetremann kpetremann mentioned this pull request Jan 19, 2023
Merged
7 tasks
liat-grozovik pushed a commit that referenced this pull request Feb 8, 2023
@kpetremann @liuh-80
[202205] Fix epoll and socket resource leak issue. (#651) (#741)
b9d4284 
- Why I did it
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

The reason of this issue is in SWIG any method return a new object need decorate with %newobject, so SWIG will generate code to release C++ object when python wrapper object released:
https://www.swig.org/Doc4.0/SWIGDocumentation.html#Customization_ownership

- How I did it
Update swsscommon.i to decorate return new object methods with %newobject

- How to verify it
Pass all test case.

Run following code in python and validate there is no epoll and socket leak:

from swsscommon.swsscommon import SonicDBConfig
from swsscommon.swsscommon import SonicV2Connector
import gc

SonicDBConfig.load_sonic_global_db_config()
SonicDBConfig.get_ns_list()
db = SonicV2Connector(use_unix_socket_path=True, namespace='')
db.connect("CONFIG_DB")
db.get_redis_client("CONFIG_DB")
client = db.get_redis_client("CONFIG_DB")
client.pubsub()
client.pubsub()
client.pubsub()
client.newConnector(0)
client.newConnector(0)
client.newConnector(0)

gc.collect()

Co-authored-by: Hua Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prgeor prgeor prgeor approved these changes

@kcudnik kcudnik kcudnik approved these changes

@qiluo-msft qiluo-msft qiluo-msft approved these changes

Assignees
No one assigned
Labels
Bug Cherry Pick Conflict_202205 Included in 202205 Branch Request for 202205 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@liuh-80 @abdosi @qiluo-msft @kcudnik @prgeor @yxieca @liat-grozovik