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

Optimize catalog services endpoint #2313

Merged

Conversation

sanderegg
Copy link
Member

@sanderegg sanderegg commented May 3, 2021

What do these changes do?

  • reduces the number of calls to the database
  • optimizes validation through ProcessPoolExecutor
  • catalog to gzip responses when over 500 bytes

gained a few 100ms in the operation, hopefully might help in the 20 users access osparc from the CI case

Related issue/s

How to test

Checklist

@sanderegg sanderegg self-assigned this May 3, 2021
@sanderegg sanderegg added a:catalog catalog service a:database associated to postgres service and postgres-database package labels May 3, 2021
@sanderegg sanderegg added this to the Schwarznasenschaf milestone May 3, 2021
@codecov
Copy link

codecov bot commented May 3, 2021

Codecov Report

Merging #2313 (bba4593) into master (c74f23a) will decrease coverage by 3.3%.
The diff coverage is 24.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2313     +/-   ##
========================================
- Coverage    71.7%   68.4%   -3.4%     
========================================
  Files         506     506             
  Lines       19912   19927     +15     
  Branches     1949    1947      -2     
========================================
- Hits        14293   13633    -660     
- Misses       5143    5858    +715     
+ Partials      476     436     -40     
Flag Coverage Δ
integrationtests 75.3% <ø> (+13.3%) ⬆️
unittests 67.1% <24.0%> (-0.1%) ⬇️

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

Impacted Files Coverage Δ
...mcore_service_catalog/api/dependencies/database.py 68.7% <0.0%> (-4.6%) ⬇️
...src/simcore_service_catalog/api/routes/services.py 32.5% <15.7%> (+6.0%) ⬆️
...imcore_service_catalog/db/repositories/services.py 30.3% <25.0%> (-0.9%) ⬇️
.../simcore_service_catalog/db/repositories/groups.py 42.8% <28.5%> (-5.5%) ⬇️
...og/src/simcore_service_catalog/core/application.py 86.3% <100.0%> (+0.6%) ⬆️
...es/sidecar/src/simcore_service_sidecar/mpi_lock.py 29.0% <0.0%> (-71.0%) ⬇️
...simcore_service_webserver/computation_subscribe.py 30.0% <0.0%> (-61.3%) ⬇️
..._director_v2/modules/db/repositories/comp_tasks.py 40.5% <0.0%> (-55.5%) ⬇️
...c/simcore_service_webserver/users_to_groups_api.py 46.1% <0.0%> (-53.9%) ⬇️
...ore_service_director_v2/api/routes/computations.py 30.3% <0.0%> (-53.4%) ⬇️
... and 37 more

@sanderegg sanderegg force-pushed the optimize_catalog_services_endpoint branch from 4bec6bc to de52810 Compare May 3, 2021 16:35
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.

This is definitively an improvement! :-)
Please consider the suggested changes . There are important changes and I also think we should start adding more testing to the catalog

if service_in_db.owner:
service.owner = await groups_repository.get_user_email_from_gid(
service_in_db.owner
with ProcessPoolExecutor() as pool:
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we have an executor pool per application instead of per request? Did you test the difference?

Copy link
Member Author

@sanderegg sanderegg May 4, 2021

Choose a reason for hiding this comment

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

yes. saw nothing, no measureable improvement

@pcrespov pcrespov changed the title Optimize catalog services endpoint WIP: Optimize catalog services endpoint May 4, 2021
@sanderegg sanderegg force-pushed the optimize_catalog_services_endpoint branch from de15043 to aa320a1 Compare May 4, 2021 13:07
@sanderegg sanderegg marked this pull request as ready for review May 4, 2021 15:04
@sanderegg sanderegg changed the title WIP: Optimize catalog services endpoint Optimize catalog services endpoint May 4, 2021
@sanderegg sanderegg force-pushed the optimize_catalog_services_endpoint branch from 138e4f4 to 778fbdf Compare May 5, 2021 06:56
@sanderegg sanderegg merged commit 82c0f3c into ITISFoundation:master May 5, 2021
@sanderegg sanderegg deleted the optimize_catalog_services_endpoint branch May 5, 2021 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:catalog catalog service a:database associated to postgres service and postgres-database package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants