mongo icon indicating copy to clipboard operation
mongo copied to clipboard

forked temporary mongod not shutdown if initdb failing script

Open ollofx opened this issue 4 years ago • 3 comments

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

https://github.com/docker-library/mongo/blob/9db9e3d4704f5d963e424a3894fa740b8ce4ea70/4.4/docker-entrypoint.sh#L304

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

then https://github.com/docker-library/mongo/blob/9db9e3d4704f5d963e424a3894fa740b8ce4ea70/4.4/docker-entrypoint.sh#L357

	rm -f "$pidfile"
        # reset shutdown
	trap - EXIT

ollofx avatar Apr 09 '21 12:04 ollofx

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. :confused:

tianon avatar Apr 10 '21 00:04 tianon

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
```				

ollofx avatar Apr 10 '21 19:04 ollofx

I guess an approach like what I describe (and then demonstrate) in https://github.com/docker-library/postgres/issues/159#issuecomment-1042466970 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. :thinking:

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).

tianon avatar Jun 10 '22 23:06 tianon