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

✨ postgres readiness check includes migration completed #2600

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 22, 2021

What do these changes do?

We noticed in the CI that under certain conditions, a service fails to start because while connecting to the database the migration (performed by the migration service) is still not complete (Notice that the swarm does not guarantee the order of deploy of services).

We could ensure that migration service always start first but this does not guarantee that the migration is completed. Therefore, this PR creates a check function to ensure the caller service that the db migration has successfully completed.

Hightlights of this PR:

  • simcore_postgres_database
    • ✨ defines utils_migration.get_current_head to get head revision of current db (adds alembic dependency to base.txt)
    • ♻️ creates utils_cli.py and moves there all utilities used in cli.py
    • ♻️ creates utils_aiopg.py
    • ♻️ creates retry_policies.py for common tenacity-based retry policies
    • ♻️ cleanup errors.py to structure pg errors
  • servicelib: steps towards fully depreciating aiopg utils
    • ♻️ moving most of the aiopg utils to simcore_postgres_database package
  • ✨ All pg startup events now include a check to ensure that the pg migration is complete. Affects:
    • api-server
    • catalog
    • director-v2
    • storage
    • webserver
    • simcore-sdk
    • sidecar

Related issue/s

services start communicating with pg before or while migration is taking place, leading to startup failures

How to test

  • all db tests involved

@pcrespov pcrespov force-pushed the maintenance/postgres-ready-check branch from 03849c6 to 61c1490 Compare October 22, 2021 02:04
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #2600 (addf0f3) into master (b014be7) will decrease coverage by 0.0%.
The diff coverage is 45.2%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #2600     +/-   ##
========================================
- Coverage    77.1%   77.1%   -0.1%     
========================================
  Files         621     625      +4     
  Lines       24043   24094     +51     
  Branches     2367    2362      -5     
========================================
+ Hits        18561   18581     +20     
- Misses       4850    4880     +30     
- Partials      632     633      +1     
Flag Coverage Δ
integrationtests 65.9% <78.6%> (+0.1%) ⬆️
unittests 71.6% <39.5%> (-0.2%) ⬇️

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%> (ø)
...abase/src/simcore_postgres_database/utils_aiopg.py 0.0% <0.0%> (ø)
...atabase/src/simcore_postgres_database/utils_cli.py 0.0% <0.0%> (ø)
...s/service-library/src/servicelib/retry_policies.py 0.0% <0.0%> (ø)
...s/catalog/src/simcore_service_catalog/db/events.py 39.2% <28.5%> (-5.6%) ⬇️
...server/src/simcore_service_api_server/db/events.py 86.2% <73.3%> (-13.8%) ⬇️
...c/simcore_service_director_v2/modules/db/events.py 87.5% <75.0%> (-12.5%) ⬇️
...sdk/src/simcore_sdk/node_ports_common/dbmanager.py 85.7% <78.5%> (-0.3%) ⬇️
services/sidecar/src/simcore_service_sidecar/db.py 88.2% <80.9%> (-3.8%) ⬇️
...ces/web/server/src/simcore_service_webserver/db.py 79.6% <80.9%> (+18.8%) ⬆️
... and 9 more

@pcrespov pcrespov changed the title WIP: ✨ postgres ready includes check on migration WIP: ✨ postgres readiness check includes migration completed Oct 22, 2021
@pcrespov pcrespov self-assigned this Oct 22, 2021
@pcrespov pcrespov added this to the Anti-PER milestone Oct 22, 2021
@pcrespov pcrespov added a:catalog catalog service a:database associated to postgres service and postgres-database package a:director-v2 issue related with the director-v2 service a:webserver issue related to the webserver service labels Oct 22, 2021
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.

looking promising!

services/catalog/src/simcore_service_catalog/db/events.py Outdated Show resolved Hide resolved
@pcrespov pcrespov force-pushed the maintenance/postgres-ready-check branch from ea85a28 to 4f2ef67 Compare October 26, 2021 06:49
@pcrespov pcrespov changed the title WIP: ✨ postgres readiness check includes migration completed ✨ postgres readiness check includes migration completed Oct 26, 2021
@pcrespov pcrespov requested review from GitHK and sanderegg October 26, 2021 06:58
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.

Looks way better now. Thanks!

@pcrespov pcrespov force-pushed the maintenance/postgres-ready-check branch from 6f8825a to 06e46cb Compare October 26, 2021 09:09
@pcrespov pcrespov marked this pull request as ready for review October 26, 2021 09:19
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, let's hope this brings stability

packages/service-library/src/servicelib/retry_policies.py Outdated Show resolved Hide resolved
@pcrespov pcrespov force-pushed the maintenance/postgres-ready-check branch from 0bd2bc0 to addf0f3 Compare October 26, 2021 16:38
@pcrespov pcrespov merged commit 5ef4cd9 into ITISFoundation:master Oct 27, 2021
@pcrespov pcrespov deleted the maintenance/postgres-ready-check branch October 27, 2021 11:39
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 a:director-v2 issue related with the director-v2 service a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants