Skip to content

garbage collector system overhaul and extras #1724

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

Merged
merged 68 commits into from
Sep 15, 2020

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Aug 20, 2020

Changes overview

This is a massive overhaul of the Garbage Collector code, which was outdated.

Due to its complexity, comments on its internal workings were added in the garbage_collector.py module

What do these changes do?

  • Previously the garbage collector was not able to remove a GUEST user which had no projects.
  • Fixes the issue of having guest users without projects in master
  • GC implementation has been refactored to keep access rights in mind
  • added extensive docstrings to all functions in the garbage_collector module which explain the logic behind its workings
  • adds tests to check the correct removal of GUEST users and their associated projects

Bonus changes:

  • changelog now links PRs
  • logs from aiohttp apps will now show during tests with options -vvvand -s
  • nvidia/cuda:10.0-base container used by the sidecar is now removed after usage
  • it is now possible to test in isolation with template databases, after each test is run, the DB will be dropped and recreated from the template. This avoids the overhead of applying all the migrations each time. For now this is only enabled in the test_garbage_collection.py module

Related issue number

How to test

Checklist

  • Did you change any service's API? Then make sure to bundle document and upgrade version (make openapi-specs, git commit ... and then make version-*)
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes
  • New module? Add your github username to .github/CODEOWNERS

@GitHK GitHK changed the title WIP: guests without projects are now removed WIP: guests without projects are now removed by the garbage collector Aug 20, 2020
@GitHK GitHK self-assigned this Aug 20, 2020
@GitHK GitHK added a:webserver issue related to the webserver service t:enhancement Improvement or request on an existing feature labels Aug 20, 2020
@codecov
Copy link

codecov bot commented Aug 20, 2020

Codecov Report

Merging #1724 into master will increase coverage by 2.1%.
The diff coverage is 80.8%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #1724     +/-   ##
========================================
+ Coverage    70.2%   72.4%   +2.1%     
========================================
  Files         305     306      +1     
  Lines       11778   11886    +108     
  Branches     1272    1285     +13     
========================================
+ Hits         8274    8608    +334     
+ Misses       3179    2928    -251     
- Partials      325     350     +25     
Flag Coverage Δ
#integrationtests 58.6% <76.7%> (+2.3%) ⬆️
#unittests 66.0% <52.2%> (-0.3%) ⬇️

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

Impacted Files Coverage Δ
...gres-database/src/simcore_postgres_database/cli.py 0.0% <0.0%> (ø)
...ce_webserver/resource_manager/garbage_collector.py 72.8% <77.7%> (+3.2%) ⬆️
...server/src/simcore_service_webserver/groups_api.py 89.3% <100.0%> (+0.3%) ⬆️
.../simcore_service_webserver/projects/projects_db.py 84.4% <100.0%> (+0.9%) ⬆️
...ore_service_webserver/resource_manager/__init__.py 100.0% <100.0%> (ø)
...src/simcore_service_webserver/socketio/handlers.py 82.6% <100.0%> (ø)
.../server/src/simcore_service_webserver/users_api.py 93.9% <100.0%> (+0.2%) ⬆️
...c/simcore_service_webserver/users_to_groups_api.py 100.0% <100.0%> (ø)
.../director/src/simcore_service_director/producer.py 60.1% <0.0%> (-0.5%) ⬇️
... and 10 more

@GitHK GitHK marked this pull request as ready for review August 20, 2020 13:03
@GitHK GitHK changed the title WIP: guests without projects are now removed by the garbage collector guests without projects are now removed by the garbage collector Aug 20, 2020
@GitHK GitHK requested review from sanderegg, pcrespov and mguidon and removed request for sanderegg August 20, 2020 13:03
@GitHK GitHK requested a review from sanderegg September 10, 2020 06:37
@GitHK GitHK requested review from pcrespov and sanderegg September 11, 2020 12:36
@GitHK GitHK added this to the Nephelai milestone Sep 11, 2020
@GitHK GitHK requested a review from sanderegg September 14, 2020 07:27
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

great! go in!
for the testing I do not see any change in speed between here and there
but maybe we can use these in // soon.

@GitHK GitHK merged commit df0de8e into ITISFoundation:master Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service t:enhancement Improvement or request on an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants