Skip to content

bugfix for #1853: garbage collector deletes guest users #1928

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

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Nov 2, 2020

What do these changes do?

Access to /study/{study_id} link produces
image

The reason is that the Garbage collector removes GUEST users when they have no resources associated. This caused that temporary GUEST users created in studies access were deleted even before their projects were associated.

This fix overcomes this problem by

  • first creating the user as ANONYMOUS instead of GUEST
  • then, once the access_study is processed, a fictitious socket is created for the socket and it is turn into GUEST

This will prevent the GC to destroy the user before the access_study handle is completed and some extra time TTL to reconnect.

  • Adapted pytest tests/unit/with_dbs/fast/test_access_to_studies.py
  • test to demo that an eager GC does not delete user
  • cleaned

Related issue number

Fixes #1853

How to test

cd services/web
make install-dev
pytest --pdb -s --log-cli-level=DEBUG tests/unit/with_dbs/fast/test_access_to_studies.py

and can also see GC log.

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

@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #1928 into master will increase coverage by 3.6%.
The diff coverage is 85.7%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #1928     +/-   ##
========================================
+ Coverage    69.0%   72.6%   +3.6%     
========================================
  Files         350     351      +1     
  Lines       13314   13342     +28     
  Branches     1391    1392      +1     
========================================
+ Hits         9191    9694    +503     
+ Misses       3756    3267    -489     
- Partials      367     381     +14     
Flag Coverage Δ
integrationtests 61.3% <55.4%> (-0.2%) ⬇️
unittests 66.0% <85.7%> (+3.7%) ⬆️

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

Impacted Files Coverage Δ
...es/service-library/src/servicelib/logging_utils.py 0.0% <0.0%> (ø)
...erver/src/simcore_service_webserver/application.py 91.0% <50.0%> (+38.6%) ⬆️
...es/web/server/src/simcore_service_webserver/cli.py 81.5% <50.0%> (+16.1%) ⬆️
...es/web/server/src/simcore_service_webserver/log.py 76.9% <76.9%> (ø)
...er/src/simcore_service_webserver/studies_access.py 88.3% <92.3%> (+64.3%) ⬆️
...b/server/src/simcore_service_webserver/__init__.py 100.0% <100.0%> (ø)
.../web/server/src/simcore_service_webserver/_meta.py 100.0% <100.0%> (ø)
...eb/server/src/simcore_service_webserver/catalog.py 76.0% <100.0%> (+24.0%) ⬆️
...b/server/src/simcore_service_webserver/products.py 91.3% <100.0%> (+5.1%) ⬆️
...mcore_service_webserver/resource_manager/config.py 100.0% <100.0%> (ø)
... and 40 more

@pcrespov pcrespov self-assigned this Nov 2, 2020
@pcrespov pcrespov force-pushed the is1853/garbage-collector-fast-delete branch from b25934d to cf36964 Compare November 2, 2020 22:30
@pcrespov pcrespov changed the title WIP: Is1853/garbage collector fast delete Fixes #1853: garbage collector fast delete Nov 3, 2020
@pcrespov pcrespov added the bug buggy, it does not work as expected label Nov 3, 2020
@pcrespov pcrespov requested review from sanderegg and GitHK and removed request for sanderegg November 3, 2020 18:46
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

Please try to understand if there are any corner cases from what I've commented below.

@pcrespov pcrespov changed the title Fixes #1853: garbage collector fast delete bugfix for #1853: garbage collector deletes guest users Nov 4, 2020
@pcrespov pcrespov force-pushed the is1853/garbage-collector-fast-delete branch from 6575199 to 855dea4 Compare November 4, 2020 14:39
@pcrespov pcrespov requested a review from GitHK November 4, 2020 14:42
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

This should address the issue, due to the fact that the users are being used immediately after creation.

@pcrespov pcrespov force-pushed the is1853/garbage-collector-fast-delete branch from ca467ce to 9478980 Compare November 4, 2020 18:17
@pcrespov pcrespov force-pushed the is1853/garbage-collector-fast-delete branch from 13dae60 to d9b0297 Compare November 4, 2020 21:28
@pcrespov pcrespov merged commit 2faf8ab into ITISFoundation:master Nov 4, 2020
@pcrespov pcrespov deleted the is1853/garbage-collector-fast-delete branch November 4, 2020 22:11
pcrespov added a commit that referenced this pull request Nov 10, 2020
- It reimplements the fix using locks to protect construction and initialization of the GUEST user from an eager GC! 
- This PR replaces PR#1928 (bugfix for #1853: garbage collector deletes guest users bug). This implementation avoid setting fictitious resources or anonymous users as in PR #1928. Those proved to be very error prone.
- EXTRA: resources are removed from registry when GC actually executes the clean. This way, the share-lock mechanism will prevent a user from opening a resource that is being removed
@sanderegg sanderegg mentioned this pull request Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug buggy, it does not work as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An eager GC deletes guest users while getting created
3 participants