Skip to content

director: renew cache in the background instead of clearing cache #1496

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

sanderegg
Copy link
Member

What do these changes do?

Until now the director was caching calls to the registry by clearing the cache every 15 minutes.
Now the background task first gets the new cache and replaces it after success thus there is no time where there is no cache. Unless some error (registry not available) happens.

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

@sanderegg sanderegg self-assigned this May 11, 2020
@codecov
Copy link

codecov bot commented May 11, 2020

Codecov Report

Merging #1496 into master will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1496      +/-   ##
==========================================
- Coverage   71.24%   71.16%   -0.08%     
==========================================
  Files         247      139     -108     
  Lines        9959     5189    -4770     
  Branches     1094      501     -593     
==========================================
- Hits         7095     3693    -3402     
+ Misses       2550     1382    -1168     
+ Partials      314      114     -200     
Flag Coverage Δ
#integrationtests ?
#unittests 73.91% <ø> (+10.61%) ⬆️
Impacted Files Coverage Δ
...simcore_service_webserver/computation_subscribe.py 33.33% <0.00%> (-63.77%) ⬇️
.../simcore_service_webserver/computation_handlers.py 40.00% <0.00%> (-48.00%) ⬇️
...r/src/simcore_service_webserver/computation_api.py 18.98% <0.00%> (-46.21%) ⬇️
...webserver/computation_comp_tasks_listening_task.py 30.15% <0.00%> (-41.27%) ⬇️
...simcore_service_webserver/director/director_api.py 25.00% <0.00%> (-26.67%) ⬇️
...erver/src/simcore_service_webserver/computation.py 63.63% <0.00%> (-22.73%) ⬇️
...rc/simcore_service_webserver/computation_models.py 55.55% <0.00%> (-22.23%) ⬇️
..._service_webserver/projects/projects_exceptions.py 54.54% <0.00%> (-18.19%) ⬇️
...r/src/simcore_service_webserver/director/config.py 86.66% <0.00%> (-13.34%) ⬇️
...simcore_service_webserver/projects/projects_api.py 74.33% <0.00%> (-7.08%) ⬇️
... and 106 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 0843954...31b0ba8. Read the comment docs.

@@ -46,7 +47,7 @@ async def test_registry_caching_task(loop, client, push_services):
number_of_computational_services=1, number_of_interactive_services=1
)
# the services shall be updated
await sleep(config.DIRECTOR_REGISTRY_CACHING_TTL)
await sleep(config.DIRECTOR_REGISTRY_CACHING_TTL * 1.1) # NOTE: this can take some time. Sleep increased by 10%.
Copy link
Member

Choose a reason for hiding this comment

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

MINOR: why not simply increasing the variable itself? Is it used somewhere else?

Copy link
Member Author

@sanderegg sanderegg May 12, 2020

Choose a reason for hiding this comment

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

yes it is used by the director registry caching task. the point is to wait longer than him. probably the very right way would be to find the task out and wait for it.

@sanderegg sanderegg merged commit 5d3c405 into ITISFoundation:master May 13, 2020
@sanderegg sanderegg deleted the bugfix/slow_update_of_registry branch May 13, 2020 08:53
@sanderegg sanderegg mentioned this pull request Jun 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants