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

🐛addresses blocking Thread/Process pool executors when shutting down #2397

Merged
merged 32 commits into from
Jun 24, 2021

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Jun 22, 2021

What do these changes do?

While debugging the below error, it was observed that when closing a ThreadPoolExecutor or a ProcessPoolExecutor, both will wait for threads/processes before closing.

  _     ._   __/__   _ _  _  _ _/_   Recorded: 03:31:46  Samples:  1,
 /_//_/// /_\ / //_// / //_'/ //     Duration: 5.901     CPU time: 5.870,
/   _/                      v3.4.2,
,
Program: /home/scu/.venv/bin/simcore-service-webserver --config server-docker-prod.yaml,
,
5.898 instrumented  servicelib/monitor_slow_callbacks.py:23,
└─ 5.898 instrumented  aiodebug/log_slow_callbacks.py:16,
   └─ 5.898 _run  asyncio/events.py:143,
      └─ 5.898 metrics_handler  simcore_service_webserver/diagnostics_monitoring.py:32,
         └─ 5.898 __exit__  concurrent/futures/_base.py:610,
            └─ 5.898 shutdown  concurrent/futures/thread.py:146,
               └─ 5.898 join  threading.py:1024,
                  └─ 5.898 _wait_for_tstate_lock  threading.py:1062,
                     └─ 5.898 lock.acquire  ../../<built-in>:0,

Where possible pools where created and reused, in all other cases a non blocking shutdown was used via contextmanger.

Related issue/s

There is an issue with cypython where a ProcessPoolExecutor which raises an error on shutdown(wait=False). Will use a shard ProcessPoolExecutor where it was requested crated per request.

How to test

Checklist

@codecov
Copy link

codecov bot commented Jun 22, 2021

Codecov Report

Merging #2397 (52aaf81) into master (3ab3bf6) will decrease coverage by 0.1%.
The diff coverage is 67.5%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2397     +/-   ##
========================================
- Coverage    74.2%   74.0%   -0.2%     
========================================
  Files         518     520      +2     
  Lines       20320   20344     +24     
  Branches     2012    2016      +4     
========================================
- Hits        15088   15069     -19     
- Misses       4711    4758     +47     
+ Partials      521     517      -4     
Flag Coverage Δ
integrationtests 66.1% <42.8%> (-1.3%) ⬇️
unittests 67.7% <65.0%> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/service-library/src/servicelib/utils.py 67.9% <ø> (ø)
...imcore_service_webserver/diagnostics_monitoring.py 89.8% <0.0%> (+1.1%) ⬆️
...catalog/src/simcore_service_catalog/utils/pools.py 42.8% <42.8%> (ø)
...src/simcore_service_catalog/api/routes/services.py 32.5% <50.0%> (ø)
.../service-library/src/servicelib/archiving_utils.py 61.6% <100.0%> (-28.0%) ⬇️
packages/service-library/src/servicelib/pools.py 100.0% <100.0%> (ø)
...vice_webserver/exporter/formatters/formatter_v2.py 88.2% <100.0%> (ø)
.../simcore_service_webserver/projects/projects_db.py 90.2% <100.0%> (-0.1%) ⬇️
...e_webserver/exporter/formatters/sds/xlsx/writer.py 38.8% <0.0%> (-61.2%) ⬇️
...ce_webserver/exporter/formatters/sds/text_files.py 68.4% <0.0%> (-31.6%) ⬇️
... and 7 more

@GitHK GitHK self-assigned this Jun 22, 2021
@GitHK GitHK added a:catalog catalog service a:webserver issue related to the webserver service bug buggy, it does not work as expected a:services-library issues on packages/service-libs labels Jun 22, 2021
@GitHK GitHK added this to the Marmoset milestone Jun 22, 2021
@GitHK GitHK requested review from pcrespov, sanderegg, mguidon and colinRawlings and removed request for sanderegg June 22, 2021 08:25
@GitHK GitHK marked this pull request as ready for review June 22, 2021 08:26
@GitHK GitHK changed the title 🐛addresses blocking pools when shutting down 🐛addresses blocking Thread/Process pool executors when shutting down Jun 22, 2021
@GitHK GitHK requested a review from pcrespov June 22, 2021 10:33
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

Sorry, I think that

a) one thread/process pool per app
b) thread/process poll lifetime = app lifetime

As a reference see http client (https://github.com/ITISFoundation/osparc-simcore/blob/master/packages/service-library/src/servicelib/client_session.py)

@GitHK GitHK requested review from sanderegg and pcrespov June 23, 2021 08:59
@GitHK GitHK requested a review from pcrespov June 23, 2021 12:13
@GitHK GitHK merged commit cee7479 into ITISFoundation:master Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:catalog catalog service a:services-library issues on packages/service-libs a:webserver issue related to the webserver service bug buggy, it does not work as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants