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

forked temporary mongod not shutdown if initdb failing script #464

Open
ollofx opened this issue Apr 9, 2021 · 3 comments
Open

forked temporary mongod not shutdown if initdb failing script #464

ollofx opened this issue Apr 9, 2021 · 3 comments

Comments

@ollofx
Copy link

ollofx commented Apr 9, 2021

when any script in /docker-entrypoint-initdb.d/* fails the whole docker-entrypoint.sh fails but without shutting down the temporary forked mongod

"${mongodHackedArgs[@]}" --fork

	function shutdown {
	     "${mongodHackedArgs[@]}" --shutdown
	      rm -f "$pidfile"
	}
        # hook showtdown
        trap shutdown EXIT

then

"${mongodHackedArgs[@]}" --shutdown

	rm -f "$pidfile"
        # reset shutdown
	trap - EXIT
@ollofx ollofx changed the title initdb forked mongod not shutdown if failing script initdb forked mongod not shutdown if initdb failing script Apr 9, 2021
@ollofx ollofx changed the title initdb forked mongod not shutdown if initdb failing script forked temporary mongod not shutdown if initdb failing script Apr 9, 2021
@tianon
Copy link
Member

tianon commented Apr 10, 2021

I'm honestly a little confused by this -- if any of the initdb scripts fails, it would be useful to not have a database that can actually be used because then it's way too easy to mistake for one that did properly initialize, so if we were to add an explicit error trap, IMO it should purposefully make the database "unclean" such that it requires administrator action/attention. 😕

@ollofx
Copy link
Author

ollofx commented Apr 10, 2021

Hello Tianon,

Let's consider the /data/db is a volume, so it could be persisted if the volume is mounted on the host.

The issue is that the initdb scripts are loaded only if /data/db folder is empty, db not yet initiated.

	# check for a few known paths (to determine whether we've already initialized and should thus skip our initdb scripts)
	if [ -n "$shouldPerformInitdb" ]; then
		dbPath="$(_dbPath "$@")"
		for path in \
			"$dbPath/WiredTiger" \
			"$dbPath/journal" \
			"$dbPath/local.0" \
			"$dbPath/storage.bson" \
		; do
			if [ -e "$path" ]; then
				shouldPerformInitdb=
				break
			fi
		done
	fi

but the issue is that before initdb scripts are loaded, there is a local (127.0.0.1) mongod which is forked (started into another process)

"${mongodHackedArgs[@]}" --fork 

then this temporary (only meant to load initdb scripts) will create on /data/db the initial files of the db even before start executing initdb scripts, so /data/db is not anymore empty, and also create socket file /monogo.22017 and start a process mongod.

now if any of the initdb scripts fails, then the docker-entrypoint.sh fails without shutting down properly the temporary mongod, so it stays up locks the port 22017 and the /data/db directory

and indeed if the entrypoint of the container is docker-entrypoint.sh then you are saved and you wont notice it, so th zombie temporary mongod is killed, because when the docker container dies it kills all remaining processes.

now if you restart the container again, the /data/db is already filled with data that temporary mongod has initiated, so the initdb scripts do not start again, because the container thinks that the database had been initated properly already, but the issue is that the initdb scripts will never be executed again, and the error will never be noticed.

so the best approach is that if one of the initdb scripts fails, then the /data/db should not be initiated.
and that the container will be stuck with the error linked to initidb scripts startup.

to achieve this, we just need to ensure that the temporary mongod is shutdown properly if any errors in the initdb scripts, and remove the initial data created by this temporary local mongod.

line 304

		"${mongodHackedArgs[@]}" --fork

		function cleanup {
		 "${mongodHackedArgs[@]}" --shutdown
		  rm -f "$pidfile"
		  rm -rf "$dbPath/WiredTiger" "$dbPath/journal" "$dbPath/local.0" "$dbPath/storage.bson"
		  echo
		  echo 'MongoDB initdb process failed. cleanup data'
		  echo
		}

		trap cleanup EXIT

		mongo=( mongo --host 127.0.0.1 --port 27017 --quiet )

		# check to see that our "mongod" actually did start up (catches "--help", "--version", MongoDB 3.2 being silly, slow prealloc, etc)
		# https://jira.mongodb.org/browse/SERVER-16292
		tries=30
		while true; do
			if ! { [ -s "$pidfile" ] && ps "$(< "$pidfile")" &> /dev/null; }; then
				# bail ASAP if "mongod" isn't even running
				echo >&2

line 368

		"${mongodHackedArgs[@]}" --shutdown
		rm -f "$pidfile"

		trap - EXIT

		echo
		echo 'MongoDB initdb process complete; ready for start up.'
		echo

@tianon
Copy link
Member

tianon commented Jun 10, 2022

I guess an approach like what I describe (and then demonstrate) in docker-library/postgres#159 (comment) could also work -- the short version is that we initialize to a subdirectory of the provided datadir so that subsequent runs can fail immediately if said subdirectory still exists. 🤔

Then we get the benefit that containers with a restart policy stay failing and keep their (failed) state for inspection.

The biggest downside is the added script complexity (and thus added fragility + maintenance overhead, which is why that PostgreSQL one hasn't actually been implemented as a PR yet).

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

No branches or pull requests

2 participants