Include states in run-postgresql

[matthyx]

We've noticed, with persistent storage, that if the script is interrupted between lines 16 and 20 users never get created even on following container restarts because "$PGDATA/postgresql.conf" gets created before create_users is run.
I have added states for each critical step in the startup in the folder "$PGDATA/status".
As a side question, is there a particular reason to call set_passwords every time? (this is why I have also added the if status for it)

[openshift-bot]

Can one of the admins verify this patch?

[centos-ci]

Can one of the admins verify this patch?

[rhscl-bot]

Can one of the admins verify this patch?

[praiskup]

Hi Matthias, thanks for the PR!

On Friday, January 13, 2017 3:01:44 AM CET Matthias Bertschy wrote: We've noticed, with persistent storage, that if the script is interrupted between lines 16 and 20 users never get created even on following container restarts because "$PGDATA/postgresql.conf" gets created before create_users is run.

But I'm afraid this doesn't help much. For example, if you kill the container in the middle of the 'initialize_database' method (which takes quite some time usually): * you can end up having completely broken database, so container restarts will fail anyway.. * You can end up having or not having "postgresql.conf" or "pg_hba.conf" re-created by initialize_database(). * In an extreme case you can have just half of the pg_hba.conf overwritten. Simply, the database initialisation is not atomic, and it is not really trivial to make it atomic, unless there's some toolset given by container's environment (OpenShift). Then, container could mark itself as "correctly initialized" and based on that OpenShift would would know that cleanup && re-init is OK before restart or not. This has been suggested before..

I have added states for each critical step in the startup in the folder "$PGDATA/status". As a side question, is there a particular reason to call set_passwords every time? (this is why I have also added the if status for it)

I don't think adding something new manually under "$PGDATA" dir is good idea, this could lead to issues sooner or later (e.g. initdb refuses to run when that directory is non-empty). It is probably not critical, but still it is directory where DBMS places data. I'm not sure what to suggest. One could suggest to always cleanup the data based on some heuristic (e.g. initialize_replica() does 'rm -rf' but I considered that mistake before that was added). One acceptable soultion would be to: 1. ensure $PGDATA is empty 2. try to initialize 3. mark (e.g. env var) container as initialized 4. run the database And if the container was "gently" terminated before 3., we could end up running some trap shell exit hook, but even this is pretty dangerous.

[matthyx]

@praiskup thanks for your detailed answer. You have a very good point with the interruption in the middle of the database initialization.
I suggest we then clean $PGDATA completely unless we have passed the first state. Somewhat brutal, but works in that case.
What do you think?

[praiskup]

:( Thanks for the update, but have a look at #82, that's something I'm totally against.
This would btw. clean perfectly valid database upon container update from older containers.
There are other typos ... but it imo doesn't matter.

[praiskup]

Note the part:

And if the container was "gently" terminated before 3., we could end up running some trap shell exit hook, but even this is pretty dangerous.

I would fix that as "if and only if".

[matthyx]

Ok, agreed. Normally it's up to OpenShift to clean and recycle the persistent volume.
I will try to find a higher level solution to this problem... and discard the $PGDATA deletion.

[centos-ci]

Can one of the admins verify this patch?

[matthyx]

I am closing that PR... will come back once PetSets are available.

[bparees]

@matthyx petsets (stateful sets) are available, the mongodb example uses them:

https://github.com/sclorg/mongodb-container/tree/master/examples/petset