Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Commit

Permalink
fix: MySQL health checks need to happen over TCP (#1207)
Browse files Browse the repository at this point in the history
By using the correct health check (TCP, rather than UNIX socket) we can get
rid of the 10 second sleep.

See docker-library/mysql#930 for discussion of
health checks.

This commit also simplifies the wait-for-mysql code in the provisioning script.
  • Loading branch information
timmc-edx authored Oct 11, 2023
1 parent e0f04a7 commit d9ff540
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 21 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,6 @@ dev.dbcopyall8: ## Clean mysql80 container and copy data from old mysql 5.7 cont
docker volume rm devstack_mysql80_data
$(MAKE) dev.up.mysql57+mysql80
$(MAKE) dev.wait-for.mysql57+mysql80
sleep 10
docker compose exec -T mysql80 mysql -uroot mysql < provision-mysql80.sql
$(MAKE) $(_db_copy8_targets)
$(MAKE) stop
Expand Down
12 changes: 11 additions & 1 deletion check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,18 @@ run_check() {
mysql_run_check() {
container_name="$1"
mysql_probe="SELECT EXISTS(SELECT 1 FROM mysql.user WHERE user = 'root')"
# The use of `--protocol tcp` forces MySQL to connect over TCP rather than
# via a UNIX socket. This is needed because when MySQL starts for the first
# time in a new container, it starts a "temporary server" that runs for a
# few seconds and then shuts down before the "real" server starts up. The
# temporary server does not listen on the TCP port, but if the mysql
# command is not told which server to use, it will first try the UNIX
# socket and only after that will it try the default TCP port.
#
# By specifying that mysql should use TCP, we won't get an early false
# positive "ready" response while the temporary server is running.
run_check "${container_name}_query" "$container_name" \
"docker compose exec -T $(printf %q "$container_name") mysql -uroot -se $(printf %q "$mysql_probe")"
"docker compose exec -T $(printf %q "$container_name") mysql --protocol tcp -uroot -se $(printf %q "$mysql_probe")"
}

if should_check mysql57; then
Expand Down
23 changes: 4 additions & 19 deletions provision.sh
Original file line number Diff line number Diff line change
Expand Up @@ -128,25 +128,10 @@ if needs_mongo "$to_provision_ordered"; then
docker compose up -d mongo
fi

# Ensure the MySQL5 server is online and usable
echo "${GREEN}Waiting for MySQL 5.7.${NC}"
make dev.wait-for.mysql57

# Ensure the MySQL8 server is online and usable
echo "${GREEN}Waiting for MySQL 8.0.${NC}"
make dev.wait-for.mysql80

# In the event of a fresh MySQL container, wait a few seconds for the server to restart
# See https://github.com/docker-library/mysql/issues/245 for why this is necessary.
sleep 10

echo "${GREEN}Waiting for MySQL 5.7 to restart.${NC}"
make dev.wait-for.mysql57
echo -e "${GREEN}MySQL5 ready.${NC}"

echo "${GREEN}Waiting for MySQL 8.0 to restart.${NC}"
make dev.wait-for.mysql80
echo -e "${GREEN}MySQL8 ready.${NC}"
# Ensure the MySQL server is online and usable
echo -e "${GREEN}Waiting for MySQL.${NC}"
make dev.wait-for.mysql57+mysql80
echo -e "${GREEN}MySQL is ready.${NC}"

# Ensure that the MySQL databases and users are created for all IDAs.
# (A no-op for databases and users that already exist).
Expand Down

0 comments on commit d9ff540

Please sign in to comment.