Skip to content

Fix garbage collection #1480

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
Merged
merged 16 commits into from
May 6, 2020
Merged

Fix garbage collection #1480

merged 16 commits into from
May 6, 2020

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented May 4, 2020 •
edited
Loading

What do these changes do?

The following fixes aim to improve garbage collection in general inside the system.

  • after webserver crashes or is suddenly restarted, garbage is correctly recovered
  • when guest users finish their session, garbage collection removes the project, the user and all stored files

Related issue number

closes #1307
closes #1415

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

Andrei Neagu added 4 commits April 29, 2020 09:30
logging was failing, number formatting was not compatible with strings
9f23a30
Merge remote-tracking branch 'upstream/master' into fix-garbage-colle…
0568406 
…ction
Merge remote-tracking branch 'upstream/master' into fix-garbage-colle…
00e4b01 
…ction
fixes issue when webserver crashes
87d8911 
- if the webserver crashed before registering the ondisconnect event
from the websocket, the server will now properly cleanup
- added a client side "keep alive"
- this feature is till a work in prgoress
@codecov
Copy link

codecov bot commented May 4, 2020 •
edited
Loading

Codecov Report

Merging #1480 into master will increase coverage by 5.36%.
The diff coverage is 86.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1480      +/-   ##
==========================================
+ Coverage   71.47%   76.83%   +5.36%     
==========================================
  Files         244      155      -89     
  Lines        9707     6632    -3075     
  Branches     1060      752     -308     
==========================================
- Hits         6938     5096    -1842     
+ Misses       2464     1301    -1163     
+ Partials      305      235      -70
Flag Coverage Δ
#integrationtests 58.53% <50.00%> (-0.12%) ⬇️
#unittests 73.99% <86.00%> (+10.73%) ⬆️
Impacted Files Coverage Δ
...er/src/simcore_service_webserver/studies_access.py 88.29% <ø> (ø)
...src/simcore_service_webserver/socketio/handlers.py 84.52% <71.42%> (-2.98%) ⬇️
.../server/src/simcore_service_webserver/users_api.py 84.21% <84.21%> (ø)
...simcore_service_webserver/projects/projects_api.py 81.41% <100.00%> (ø)
...ce_webserver/resource_manager/garbage_collector.py 90.16% <100.00%> (+2.66%) ⬆️
.../simcore-sdk/src/simcore_sdk/node_data/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
...core-sdk/src/simcore_sdk/node_data/data_manager.py 0.00% <0.00%> (-95.24%) ⬇️
...ore-sdk/src/simcore_sdk/node_ports/_schema_item.py 87.50% <0.00%> (-12.50%) ⬇️
.../sidecar/src/simcore_service_sidecar/log_parser.py 79.31% <0.00%> (-12.07%) ⬇️
...mcore-sdk/src/simcore_sdk/node_ports/exceptions.py 62.12% <0.00%> (-10.61%) ⬇️
... and 97 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f14e0a...6df644c. Read the comment docs.

@GitHK GitHK self-assigned this May 4, 2020
@GitHK GitHK marked this pull request as draft May 4, 2020 13:29
@GitHK GitHK added the a:webserver issue related to the webserver service label May 4, 2020
@GitHK GitHK added this to the Zhong Zi milestone May 4, 2020
This was linked to issues May 4, 2020
when the webserver crashes, the garbage collection of services/project is not done #1307
Closed
garbage collection of anonymous storage space #1415
Closed
@odeimaiz odeimaiz self-requested a review May 4, 2020 14:07
Andrei Neagu added 4 commits May 5, 2020 17:02
fixed pytests to work with new changes
570bc57
code refactor
bf08f4d 
- decaoupled service and moved calls to user_api
- added warnings in case a user cannot be removed
- added warnings in case a project cannot be removed
moved test fixture
8c890b6
Merge remote-tracking branch 'upstream/master' into fix-garbage-colle…
4c2ed18 
…ction
@GitHK GitHK changed the title [WIP] Fix garbage collection Fix garbage collection May 5, 2020
@GitHK GitHK marked this pull request as ready for review May 5, 2020 16:46
@GitHK GitHK requested a review from pcrespov May 5, 2020 16:46
pcrespov
pcrespov reviewed May 6, 2020
View reviewed changes
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.

Some minor changes. I also miss a test. Can you add a variation of existing tests on garbage-collection? ONly if you think it's worth

odeimaiz
odeimaiz approved these changes May 6, 2020
View reviewed changes
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

WatchDog looking good 👍

pcrespov
pcrespov approved these changes May 6, 2020
View reviewed changes
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.

Go for it!

@GitHK GitHK merged commit 6065518 into ITISFoundation:master May 6, 2020
@sanderegg sanderegg mentioned this pull request May 26, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanderegg sanderegg sanderegg left review comments

@pcrespov pcrespov pcrespov approved these changes

@odeimaiz odeimaiz odeimaiz approved these changes

Assignees

@GitHK GitHK

Labels
a:webserver issue related to the webserver service
Projects
None yet
Milestone
Zhong Zi
4 participants
@GitHK @pcrespov @odeimaiz @sanderegg