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

Wait for Faros DB and Hasura/Metabase/n8n databases to exist before attempting to bring up other services #94

Merged
merged 3 commits into from Apr 4, 2022

Conversation

ypc-faros
Copy link
Contributor

@ypc-faros ypc-faros commented Apr 4, 2022

Description

This change blocks Docker compose from attempting to bring up Hasura/Metabase/n8n until faros-init has created all the databases (Faros DB, Hasura/Metabase/n8n cfg databases). At the moment, we see a lot of failures/retries because these services attempt to connect to the database before it is available.

The issue has been reported here: https://faroscommunity.slack.com/archives/C0335HMN2NQ/p1648851029917679

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Have you checked to there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run tests with your changes locally?

Comment on lines 77 to 78
healthcheck:
test: ["CMD", "psql", "-d", "postgres://${FAROS_DB_USER?}:${FAROS_DB_PASSWORD?}@${FAROS_DB_HOST?}:${FAROS_DB_PORT?}/${FAROS_DB_NAME?}", "-c", "SELECT 1"]
Copy link
Contributor

Choose a reason for hiding this comment

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

we have a wait-for script to do this in from the init container. is this not sufficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

if you want to expand the check, then I think it should also be done from the container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check we have today in the init container is to wait for the postgres service to be ready before attempting to create the databases.

The change here is different. What I want to accomplish is for Hasura, Metabase and n8n to be started only after the databases have been created to avoid failures and restarts.

Copy link
Contributor

@tovbinm tovbinm Apr 4, 2022

Choose a reason for hiding this comment

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

do you need to tweak any additional settings, e.g.

The options that can appear before CMD are:
    --interval=DURATION (default: 30s)
    --timeout=DURATION (default: 30s)
    --start-period=DURATION (default: 0s)
    --retries=N (default: 3)

https://docs.docker.com/engine/reference/builder/#healthcheck

Copy link
Contributor

Choose a reason for hiding this comment

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

@tovbinm @ypc-faros could this change (starting only after the databases have been created) also fix #45 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you need to tweak any additional settings, e.g.

The options that can appear before CMD are:
    --interval=DURATION (default: 30s)
    --timeout=DURATION (default: 30s)
    --start-period=DURATION (default: 0s)
    --retries=N (default: 3)

https://docs.docker.com/engine/reference/builder/#healthcheck

I used:

      interval: 10s
      timeout: 5s
      start_period: 1m
      retries: 5

Also checked that it doesn't wait the full start_period to begin checking. If the container becomes healthy according to the test before the start_period the dependent containers are started.

I also changed the test to check for the n8n database instead of the faros database, since the n8n database is the last created in db-init.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

I would put something like this:

interval: 15s
timeout: 5s
start_period: 30s
retries: 5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

@ypc-faros ypc-faros changed the title Wait for Faros DB to exist before attempting to bring up other services Wait for Faros DB and Hasura/Metabase/n8n databases to exist before attempting to bring up other services Apr 4, 2022
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.

n8n initialization error - "/home/node/.n8n/config". It does not seem to be valid JSON."
3 participants