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

Multiprocessing Manager resources (Queue) to be freed up during task stop #15212

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

sg893052
Copy link
Contributor

Multiprocessing Manager resources (Queue) to be freed up during task stop

Why I did it

Fix for issue #14964

Work item tracking
  • Microsoft ADO (number only):

How I did it

Free up Multiprocessing Manager resource at task stop request
[self.mpmgr.shutdown() in task_stop]

How to verify it

time systemctl stop system-health.service

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@sg893052
Copy link
Contributor Author

@adyeung @Junchao-Mellanox @vaibhavhd
FYI: This PR will fix the #14964

self.mpmgr.shutdown()

# Wait for the process to exit
self._task_process.join(self._stop_timeout_secs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should monitor_system_bus task and monitor_statedb_table task be join here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the end of the system_service() function, there exists a call to monitor_system_bus.task_stop() and monitor_statedb_table.task_stop(), which are responsible for shutting down their respective tasks.

if self._task_process.is_alive():
os.kill(self._task_process.pid, signal.SIGKILL)

if self._task_process.is_alive():
Copy link
Collaborator

Choose a reason for hiding this comment

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

log something here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added logs

Multiprocessing Manager resources (Queue) to be freed up during task stop
@bingwang-ms
Copy link
Contributor

@Junchao-Mellanox Can you please check and review? Thanks

@vaibhavhd
Copy link
Contributor

@Junchao-Mellanox please check the latest comments here. We need to take this fix in sooner.

@sg893052, please update the test description to indicate that the shutdown path is no longer hung/delayed. Please test that path if not already tested.

@vaibhavhd
Copy link
Contributor

ADO: 22090981

@qiluo-msft qiluo-msft changed the title Fix for issue#14964 Multiprocessing Manager resources (Queue) to be freed up during task stop Jun 19, 2023
@yxieca yxieca merged commit ed700de into sonic-net:master Jun 19, 2023
@mssonicbld
Copy link
Collaborator

@sg893052 PR conflicts with 202211 branch

@mssonicbld
Copy link
Collaborator

@sg893052 PR conflicts with 202205 branch

@yxieca
Copy link
Contributor

yxieca commented Jun 19, 2023

@sg893052 please help create separate PR for 202205/202211 branches. Thanks!

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #15595

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Jun 23, 2023
Multiprocessing Manager resources (Queue) to be freed up during task stop
@vaibhavhd
Copy link
Contributor

@sg893052 please help create separate PR for 202205/202211 branches. Thanks!

Hi @sg893052 , pinging you on this request. The issue is blocking 202205 warm upgrades. Please help to create cherry pick PRs.

@sg893052
Copy link
Contributor Author

Cherry-pick PR to 202205: #15662
Cherry-pick PR to 202211: #15663

@mssonicbld
Copy link
Collaborator

@sg893052 PR conflicts with 202205 branch

@sg893052
Copy link
Contributor Author

@sg893052 PR conflicts with 202205 branch
This PR is already merged to 202205 branch using #15662

sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Multiprocessing Manager resources (Queue) to be freed up during task stop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants