jbergstroem / mariadb-alpine

A tiny and fast MariaDB container
MIT License
72 stars 19 forks source link

run.sh: optimize startup and shutdown of temporary mysql daemon for imports #57

Closed jones1008 closed 2 years ago

jones1008 commented 3 years ago

I had the problem that sleep 1 wasn't enough at some point and eval "${MYSQL_CMD}" < /tmp/init was executed before th mysql daemon was started, so I searched how I can wait for the mysql daemon to be up. It turns out it is quite easy.

This also has the positive side effect to decrease the startup time a little bit when importing scripts.

I wasn't able to execute your tests, because I am on Windows and I wasn't able to execute your tests on WSL2, but maybe you can validate that they are running with this change?

jones1008 commented 3 years ago

There seems to be some problems in the automatic testing, but I think they are unrelated to my change:

#7 0.555 ERROR: unsatisfiable constraints:
#7 0.570   mariadb-10.4.17-r1:
#7 0.570     breaks: world[mariadb=10.4.15-r0]
jones1008 commented 3 years ago

added a second commit that uses mysqladmin shutdown to shutdown the temporary mysql daemon because the kill command that was used before doesn't wait for mysql to be properly shut down and therefore the script just continues...

This leads to the following error

[ERROR] mysqld: Can't lock aria control file '/var/lib/mysql/aria_log_control' for exclusive use, error: 11. Will retry for 30 seconds

This happens because the mysql daemon is started at the end of run.sh before the temporary mysql daemon is fully shutdown with kill.

jbergstroem commented 3 years ago

Thanks for filing a PR! I will take a look.

There seems to be some problems in the automatic testing, but I think they are unrelated to my change:

#7 0.555 ERROR: unsatisfiable constraints:
#7 0.570   mariadb-10.4.17-r1:
#7 0.570     breaks: world[mariadb=10.4.15-r0]

I will tag the new version shortly.

edit: if you could rebase against latest master, that'd be great!

jbergstroem commented 3 years ago

@jones1008 did you perhaps add mysqladmin to TO_KEEP in sh/clean.sh?

jones1008 commented 3 years ago

@jbergstroem I just rebased against latest master.

Do you want me to add /usr/bin/mysqladmin to TO_KEEP in sh/clean.sh or was it a question if I did?

jbergstroem commented 3 years ago

Do you want me to add /usr/bin/mysqladmin to TO_KEEP in sh/clean.sh or was it a question if I did?

My point is that this binary isn't currently in the container, so we'd have to consider adding it (increase in size vs benefit):

$ docker run -it --entrypoint mysqladmin jbergstroem/mariadb-alpine:10.4.17
docker: Error response from daemon: OCI runtime create failed: container_linux.go:370: starting container process caused: exec: "mysqladmin": executable file not found in $PATH: unknown.
jones1008 commented 3 years ago

I just replaced the mysqladmin dependency with posix functionality. It isn't as elegant as before since I am relying on the output of mysqld but we are saving about 3.5MB by not having to add the mysqladmin binary to the container.

jones1008 commented 3 years ago

just added the commit for the fix for issue #62 to this PR since I somehow was unable to create another PR with just the changes for this fix in a new branch...

jbergstroem commented 2 years ago

Very sorry for the quiet here. I will cherry pick a few things shortly.

jbergstroem commented 2 years ago

I actually do something similar in tests: https://github.com/jbergstroem/mariadb-alpine/blob/main/test/test_helper.bash#L12-L18

..will get it in the coming week as I'm preparing for a significant 10.6.10 release.

jbergstroem commented 2 years ago

Tests pass locally; I'm going to ignore the stall since its related to a known issue (#116).