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

Use Docker Compose v2, wait for healthchecks to pass #8041

Merged
merged 3 commits into from
Jul 10, 2023

Conversation

robertknight
Copy link
Member

Switch to Docker Compose v2 and change make services to use the --wait flag to wait for healthchecks to pass before exiting. Also add a healthcheck for Elasticsearch. The Makefile changes follow the pattern in the lms app.

Together these changes mean that h and its services can be started with make services && make dev, without encountering errors due to Postgres / Elasticsearch not being ready to connect.

The start_period setting is set to a generous value (1 min) as normal startup times for ES are quite long (15-20s on my 2020 Intel MacBook Pro).

See also hypothesis/lms#5556.

Example output from make services:

$ make services
[+] Running 3/3
 ✔ Container h-elasticsearch-1  Healthy                                           24.9s
 ✔ Container h-postgres-1       Healthy                                            2.4s
 ✔ Container h-rabbit-1         Healthy                                            1.9s

The startup time for Elasticsearch, as measured by whether curl http://localhost:9200 returns a non-200 response, seems rather long to me, and also somewhat variable (~17s seems more typical). It might be possible to optimize that, but it is out of scope for this PR.

@robertknight robertknight requested a review from seanh July 10, 2023 10:42
Copy link
Contributor

@seanh seanh left a comment

Choose a reason for hiding this comment

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

h doesn't use the cookiecutter (yet) so we can edit these files directly in h 👍

Can you also deleted the requirements/dockercompose.{in,txt} files?

Also these:

Comment on lines +16 to +19
healthcheck:
test: curl --fail --silent http://localhost:9200 >/dev/null
interval: 3s
start_period: 1m
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? Does h actually fail to start up if Elasticsearch is not running yet? I know make dev will actually crash if Postgres isn't up yet, but I'm not sure whether the same is true for Elasticsearch and RabbitMQ. If we can avoid having to wait for Elasticsearch and RabbitMQ then we can keep things simpler (no healthchecks) and save a few seconds on the startup time

Copy link
Member Author

Choose a reason for hiding this comment

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

Does h actually fail to start up if Elasticsearch is not running yet?

h tries to initialize the Elasticsearch index when it starts, so you get some confusing error messages if it isn't running. Also things will just seem broken if you try to access various pages served by h during the time when Elasticsearch is starting. I think it would be better to direct efforts into speeding up Elasticsearch startup time, as it seems to me that it really ought to be possible for it to start up quicker than it currently does.

@robertknight
Copy link
Member Author

Can you also deleted the requirements/dockercompose.{in,txt} files?

Done. I've updated / removed the various references to docker-compose and the dockercompose tox env.

@robertknight robertknight requested a review from seanh July 10, 2023 13:23
Switch to Docker Compose v2 and change `make services` to use the `--wait` flag
to wait for healthchecks to pass before exiting. Also add a healthcheck for
Elasticsearch. The Makefile changes follow the pattern in the lms app.

Together these changes mean that h and its services can be started with `make
services && make dev`, without encountering errors due to Postgres /
Elasticsearch not being ready to connect.

The `start_period` setting is set to a generous value (1 min) as normal startup
times for ES are quite long (15-20s on my 2020 Intel MacBook Pro).

See also hypothesis/lms#5556.
This was replaced by use of the globally installed Docker Compose v2 in
the previous commit.
@robertknight robertknight force-pushed the docker-compose-v2-es-healthcheck branch from 68e60f7 to a2dc2db Compare July 10, 2023 13:24
@robertknight robertknight merged commit 38da143 into main Jul 10, 2023
@robertknight robertknight deleted the docker-compose-v2-es-healthcheck branch July 10, 2023 15:01
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.

2 participants