Tag 13-3.1 doesn't survive container recreate because waldir is missing.

[lucernae]

When the container is recreated, it complains that the pg_wal directory is missing.
It turns out that the pg_wal directory inside the PGDATA directory is only a link to whatever directory specified in POSTGRES_INITDB_WALDIR.
I was under the impression that this env POSTGRES_INITDB_WALDIR is only for initializations.
It currently failed upon recreate because POSTGRES_INITDB_WALDIR is outside the default persistent volume.
When the container is recreated, the pg_wal content is lost, which is very dangerous.

Furthermore, if I specified POSTGRES_INITDB_WALDIR into a directory inside the persistent volume, the container is failed to initialize, because in setup-database.sh after directory creation, we deleted using rm -rf ${DATADIR} which caused the initdb waldir to be deleted again. So the postgres init, it failed to find the initdb waldir.

@NyakudyaA do you have any suggestion to fix this?
Is it ok to just move the rm -rf ${DATADIR} up to delete the content before the initdb waldir creation?
From what I understand by default you want the initdb waldir to be outside the default volume. However if initdb waldir is not persisted, when containers are recreated the symlink will not find the directory.

[NyakudyaA]

@lucernae how do I test this, let me test this and revert back

[NyakudyaA]

@lucernae is this the same issue as #317

If so can you please pull and get the latest image

[NyakudyaA]

When the container is recreated, it complains that the pg_wal directory is missing.
It turns out that the pg_wal directory inside the PGDATA directory is only a link to whatever directory specified in POSTGRES_INITDB_WALDIR.
I was under the impression that this env POSTGRES_INITDB_WALDIR is only for initializations.
It currently failed upon recreate because POSTGRES_INITDB_WALDIR is outside the default persistent volume.
When the container is recreated, the pg_wal content is lost, which is very dangerous.

Furthermore, if I specified POSTGRES_INITDB_WALDIR into a directory inside the persistent volume, the container is failed to initialize, because in setup-database.sh after directory creation, we deleted using rm -rf ${DATADIR} which caused the initdb waldir to be deleted again. So the postgres init, it failed to find the initdb waldir.

@NyakudyaA do you have any suggestion to fix this?
Is it ok to just move the rm -rf ${DATADIR} up to delete the content before the initdb waldir creation?
From what I understand by default you want the initdb waldir to be outside the default volume. However if initdb waldir is not persisted, when containers are recreated the symlink will not find the directory.

I could not replicate this issue @lucernae

[lucernae]

Ok. Let me test this again after repull.

[lucernae]

Still the same errors.

First case (if we didn't set POSTGRES_INITDB_WALDIR)

This is a simplified version of how I usually deploy on prod.
I mount a persistent volume into /opt/kartoza/postgis/data and set PGDATA=/opt/kartoza/postgis/data.
Is it mandatory for me to also set POSTGRES_INITDB_WALDIR to a different persistent volume? Because I thought this env is for custom initialization (if you already have a waldir to import). CMIIW
The docker-compose recipe:

# docker-compose build
version: '3.4'
volumes:
  dbbackups:
  postgis-data:

services:

  db:
    image: kartoza/postgis:13-3.1
    volumes:
      - postgis-data:/opt/kartoza/postgis/data
      - dbbackups:/backups
    environment:
      # If you need to create multiple database you can add coma separated databases eg gis,data
      - POSTGRES_DB=gis
      - POSTGRES_USER=docker
      - POSTGRES_PASS=docker
      - ALLOW_IP_RANGE=0.0.0.0/0
      - PGDATA=/opt/kartoza/postgis/data
    ports:
      - 25432:5432
    restart: on-failure
    healthcheck:
      test: "PGPASSWORD=${POSTGRES_PASS} psql -h localhost -U docker -d gis -l"

Run it with docker-compose up -d.
First run will be successful. If you shell inside the container, you will see that /opt/kartoza/postgis/data/pg_wal is a symlink.
Delete the container using docker-compose down (this will delete only the container but not the volumes).
Run it again using docker-compose up -d. I expect this to run, but it failed because /var/lib/postgresql/13/pg_waldir was reset to empty. That means POSTGRES_INITDB_WALDIR is actually used by postgres not just in initialization phase, but throughout the lifecycle. If this is indeed true then we should change the env to just become POSTGRES_WALDIR then set the default to be within the default persisted volume.

Second case (assuming that setting POSTGRES_INITDB_WALDIR is mandatory not just in initialization step)

If the waldir was meant to be persisted (not just from initialization phase), that means by default it has to stay within the default persisted volume. I emulate it by setting the environment variable (it needs to be implicitly defined by the entrypoint if user don't provide this. However I set this explicitly at the moment).

# docker-compose build
version: '3.4'
volumes:
  dbbackups:
  postgis-data:

services:

  db:
    image: kartoza/postgis:13-3.1
    volumes:
      - postgis-data:/opt/kartoza/postgis/data
      - dbbackups:/backups
    environment:
      # If you need to create multiple database you can add coma separated databases eg gis,data
      - POSTGRES_DB=gis
      - POSTGRES_USER=docker
      - POSTGRES_PASS=docker
      - ALLOW_IP_RANGE=0.0.0.0/0
      - PGDATA=/opt/kartoza/postgis/data
      - POSTGRES_INITDB_WALDIR=/opt/kartoza/postgis/data/pg_wal
    ports:
      - 25432:5432
    restart: on-failure
    healthcheck:
      test: "PGPASSWORD=${POSTGRES_PASS} psql -h localhost -U docker -d gis -l"

This recipe doesn't run the containers because of:

Furthermore, if I specified POSTGRES_INITDB_WALDIR into a directory inside the persistent volume, the container is failed to initialize, because in setup-database.sh after directory creation, we deleted using rm -rf ${DATADIR} which caused the initdb waldir to be deleted again. So the postgres init command failed to find the initdb waldir.

I think, this second case here is definitely a bug. We should delete the DATADIR first then create the directory for datadir and waldir.
Meanwhile, the first case is just about convention. I don't know if POSTGRES_INITDB_WALDIR were supposed to be used as initialization only, or forever (as a way to specify the pg_wal location). For this one, I need your opinion.

[NyakudyaA]

Still the same errors.

First case (if we didn't set POSTGRES_INITDB_WALDIR)

This is a simplified version of how I usually deploy on prod.
I mount a persistent volume into /opt/kartoza/postgis/data and set PGDATA=/opt/kartoza/postgis/data.
Is it mandatory for me to also set POSTGRES_INITDB_WALDIR to a different persistent volume? Because I thought this env is for custom initialization (if you already have a waldir to import). CMIIW
The docker-compose recipe:

# docker-compose build
version: '3.4'
volumes:
  dbbackups:
  postgis-data:

services:

  db:
    image: kartoza/postgis:13-3.1
    volumes:
      - postgis-data:/opt/kartoza/postgis/data
      - dbbackups:/backups
    environment:
      # If you need to create multiple database you can add coma separated databases eg gis,data
      - POSTGRES_DB=gis
      - POSTGRES_USER=docker
      - POSTGRES_PASS=docker
      - ALLOW_IP_RANGE=0.0.0.0/0
      - PGDATA=/opt/kartoza/postgis/data
    ports:
      - 25432:5432
    restart: on-failure
    healthcheck:
      test: "PGPASSWORD=${POSTGRES_PASS} psql -h localhost -U docker -d gis -l"

Run it with docker-compose up -d.
First run will be successful. If you shell inside the container, you will see that /opt/kartoza/postgis/data/pg_wal is a symlink.
Delete the container using docker-compose down (this will delete only the container but not the volumes).
Run it again using docker-compose up -d. I expect this to run, but it failed because /var/lib/postgresql/13/pg_waldir was reset to empty. That means POSTGRES_INITDB_WALDIR is actually used by postgres not just in initialization phase, but throughout the lifecycle. If this is indeed true then we should change the env to just become POSTGRES_WALDIR then set the default to be within the default persisted volume.

If you are setting up a custom data directory we also need to add the POSTGRES_INITDB_WALDIR and it's mount point i.e

-v pgwal-volume:/opt/postgres/pg_wal
-e POSTGRES_INITDB_WALDIR=/opt/postgres/pg_wal

Second case (assuming that setting POSTGRES_INITDB_WALDIR is mandatory not just in initialization step)

If the waldir was meant to be persisted (not just from initialization phase), that means by default it has to stay within the default persisted volume. I emulate it by setting the environment variable (it needs to be implicitly defined by the entrypoint if user don't provide this. However I set this explicitly at the moment).

# docker-compose build
version: '3.4'
volumes:
  dbbackups:
  postgis-data:

services:

  db:
    image: kartoza/postgis:13-3.1
    volumes:
      - postgis-data:/opt/kartoza/postgis/data
      - dbbackups:/backups
    environment:
      # If you need to create multiple database you can add coma separated databases eg gis,data
      - POSTGRES_DB=gis
      - POSTGRES_USER=docker
      - POSTGRES_PASS=docker
      - ALLOW_IP_RANGE=0.0.0.0/0
      - PGDATA=/opt/kartoza/postgis/data
      - POSTGRES_INITDB_WALDIR=/opt/kartoza/postgis/data/pg_wal
    ports:
      - 25432:5432
    restart: on-failure
    healthcheck:
      test: "PGPASSWORD=${POSTGRES_PASS} psql -h localhost -U docker -d gis -l"

This recipe doesn't run the containers because of:

Furthermore, if I specified POSTGRES_INITDB_WALDIR into a directory inside the persistent volume, the container is failed to initialize, because in setup-database.sh after directory creation, we deleted using rm -rf ${DATADIR} which caused the initdb waldir to be deleted again. So the postgres init command failed to find the initdb waldir.

The POSTGRES_INITDB_WALDIR should be a volume on its own.

I think, this second case here is definitely a bug. We should delete the DATADIR first then create the directory for datadir and waldir.
Meanwhile, the first case is just about convention. I don't know if POSTGRES_INITDB_WALDIR were supposed to be used as initialization only, or forever (as a way to specify the pg_wal location). For this one, I need your opinion.

If we are to assume that a user does set the DATADIR and not set the POSTGRES_INITDB_WALDIR then we can need to edit the logic from https://github.com/kartoza/docker-postgis/blob/develop/scripts/setup-database.sh#L9 to stop the initialisation.

The main reason for having a separate POSTGRES_INITDB_WALDIR is that this can grow big depending on the replication setup and having someone independently set this as a mount point will be beneficial and also enables to keep the data directory size to be minimal size

[NyakudyaA]

@lucernae Can we not just move around the lines to be.

if [[ -z "$(ls -A ${DATADIR} 2> /dev/null)" || "${RECREATE_DATADIR}" == 'TRUE' ]]; then
    # Only attempt reinitializations if ${RECREATE_DATADIR} is true
    # No Replicate From settings. Assume that this is a master database.
    # Initialise db
    echo "Initializing Postgres Database at ${DATADIR}"
    create_dir ${DATADIR}
    rm -rf ${DATADIR}/*
    if [ "${POSTGRES_INITDB_WALDIR}" == '/var/lib/postgresql/${POSTGRES_MAJOR_VERSION}/pg_waldir' ]; then
      POSTGRES_INITDB_WALDIR=${DATADIR}/pg_wal
    fi
    create_dir ${POSTGRES_INITDB_WALDIR}
    chown -R postgres:postgres ${DATADIR} ${POSTGRES_INITDB_WALDIR}
    command="$INITDB -U postgres --pwfile=<(echo "$POSTGRES_PASS") -E ${DEFAULT_ENCODING} --lc-collate=${DEFAULT_COLLATION} --lc-ctype=${DEFAULT_CTYPE} --wal-segsize=${WAL_SEGSIZE} --auth=${PASSWORD_AUTHENTICATION} -D ${DATADIR} --waldir=${POSTGRES_INITDB_WALDIR} ${INITDB_EXTRA_ARGS}"
    echo "Initializing with command: $command"
    su - postgres -c "$command"
fi;

[lucernae]

If you are setting up a custom data directory we also need to add the POSTGRES_INITDB_WALDIR and it's mount point

Yeah, this has to be guarded by the entrypoint scripts. Either by implicitly defining default value to be within the primary volume, or making sure that if it comes from two different volumes, then you prevent mount it nested (not every volume driver supports nested mount). Currently nothing prevents me from accidentally running the container with incorrect settings.

The POSTGRES_INITDB_WALDIR should be a volume on its own.

Ok, sorry, I know my docker-compose recipe will be misleading :p, that's why I said I put it just to emulate the default behaviour. I understand that it has to be from a separate volume if you use the environment variable (the first case). In the second case, I mean:

1. Run the recipe to replicate the behaviour of the buggy entrypoint script
2. Just ignore the key POSTGRES_INITDB_WALDIR in the docker-compose recipe, it was not meant to be there (the case should use default value), consider it don't exists in the recipe. But I have to put it there so that I can demonstrate that the entrypoint was not behaving like a standard postgres image (where the waldir is inside the data dir). IMO, this is definitely a bug in the entrypoint script, unrelated with how POSTGRES_INITDB_WALDIR parameter should behave.

@lucernae Can we not just move around the lines to be.

I also propose to just move around the deletion command in my first post. So I think we are agreeing on that.

What I need is a confirmation from you of which behaviour is expected/intended to work:
A. POSTGRES_INITDB_WALDIR is an initialization only parameter (used only once at first), then both the parameters and the volume is not used anymore.
Or
B. POSTGRES_INITDB_WALDIR should not change forever, and the mount point should not also change. The volume is then a persistent volume where it was allowed to change at runtime. This means this is not an "initialization" only parameters. The entrypoint script has to be actively checking and guarding the value to be consistent forever.

If it's A, then the content of the POSTGRES_INITDB_WALDIR should be copied into the ${DATA_DIR}/pg_wal, before the DB initializes. Previously I thought postgres did this, but it isn't. I'm quite surprised that it only make a symlink. It passed the test, because the test doesn't check containers recreation 😆 .
If it's B, then we should rather call it POSTGRES_WALDIR. The script then has to make sure that the environment variable and the mount point is consistent forever because postgres create symlink in the data dir and not using the waldir directly. If it's not consistent with the symlink then it has to warn the user because you can't have the waldir suddenly missing, or running using an incorrect directory.

I got the impression from this discussion that what you actually want is B, but the environment name is misleading and the script also lacks the check. If this is true, we should fix that.

[NyakudyaA]

If you are setting up a custom data directory we also need to add the POSTGRES_INITDB_WALDIR and it's mount point

Yeah, this has to be guarded by the entrypoint scripts. Either by implicitly defining default value to be within the primary volume, or making sure that if it comes from two different volumes, then you prevent mount it nested (not every volume driver supports nested mount). Currently, nothing prevents me from accidentally running the container with incorrect settings.

If you set the env variable POSTGRES_INITDB_WALDIR to point to a subdirectory of the data directory makes no sense because the env variable is there to ensure that we split the data directory contents. So that we can have POSTGRES_INITDB_WALDIR - for storing write-ahead logs and the actual data in postgres. I think a solution would be to check if POSTGRES_INITDB_WALDIR is a subfolder of the DATADIR and then if it is we run the initialisation without the --waldir option.

The POSTGRES_INITDB_WALDIR should be a volume on its own.

Ok, sorry, I know my docker-compose recipe will be misleading :p, that's why I said I put it just to emulate the default behaviour. I understand that it has to be from a separate volume if you use the environment variable (the first case). In the second case, I mean:

1. Run the recipe to replicate the behaviour of the buggy entrypoint script

2. Just ignore the key `POSTGRES_INITDB_WALDIR` in the docker-compose recipe, it was not meant to be there (the case should use default value), consider it don't exists in the recipe. But I have to put it there so that I can demonstrate that the entrypoint was not behaving like a standard postgres image (where the waldir is inside the data dir). IMO, this is definitely a bug in the entrypoint script, unrelated with how `POSTGRES_INITDB_WALDIR` parameter should behave.

@lucernae Can we not just move around the lines to be.

I also propose to just move around the deletion command in my first post. So I think we are agreeing on that.
The delete command ensures that the DATADIR is empty before the initialisation. So we need to be careful where we put it.

What I need is a confirmation from you of which behaviour is expected/intended to work:
A. POSTGRES_INITDB_WALDIR is an initialization only parameter (used only once at first), then both the parameters and the volume is not used anymore.
Or
B. POSTGRES_INITDB_WALDIR should not change forever, and the mount point should not also change. The volume is then a persistent volume where it was allowed to change at runtime. This means this is not an "initialization" only parameters. The entrypoint script has to be actively checking and guarding the value to be consistent forever.

B should be the default behaviour. I guess it's the same behaviour as the DATADIR where a user doesn't change it at every startup or restart.

If it's A, then the content of the POSTGRES_INITDB_WALDIR should be copied into the ${DATA_DIR}/pg_wal, before the DB initializes. Previously I thought postgres did this, but it isn't. I'm quite surprised that it only make a symlink. It passed the test, because the test doesn't check containers recreation laughing .

It only makes a symlink when the DATADIR is specified to a new location but the POSTGRES_INITDB_WALDIR is not specified. It makes a symlink because the default location of the https://github.com/kartoza/docker-postgis/blob/develop/scripts/env-data.sh#L6 is set there

If it's B, then we should rather call it POSTGRES_WALDIR. The script then has to make sure that the environment variable and the mount point is consistent forever because postgres create symlink in the data dir and not using the waldir directly. If it's not consistent with the symlink then it has to warn the user because you can't have the waldir suddenly missing, or running using an incorrect directory.

It has to be B. The symlink is only created on rare occasions where you are mounting the DATADIR into an empty volume and then you do not specify the POSTGRES_INITDB_WALDIR env. In this case, it will create a default data directory using the env variable https://github.com/kartoza/docker-postgis/blob/develop/scripts/env-data.sh#L6. That env variable has to stay that way though because for legacy if a user mounts an existing data directory it checks if the pg_wal directory exists in the folder and does not startup as per #317

I got the impression from this discussion that what you actually want is B, but the environment name is misleading and the script also lacks the check. If this is true, we should fix that.

I think the env variable should stay that way because it corresponds to the official Postgres documentation.

I have tried the following solution on my machine and it works on my side.

if [[ -z "$(ls -A ${DATADIR} 2> /dev/null)" || "${RECREATE_DATADIR}" == 'TRUE' ]]; then
    # Only attempt reinitializations if ${RECREATE_DATADIR} is true
    # No Replicate From settings. Assume that this is a master database.
    # Initialise db
    echo "Initializing Postgres Database at ${DATADIR}"
	if [ "${DATADIR}" == ${POSTGRES_INITDB_WALDIR%/*} ]; then
	  INIT_WAL_DIR=
    else
	  create_dir ${POSTGRES_INITDB_WALDIR}
      chown -R postgres:postgres ${POSTGRES_INITDB_WALDIR}
	  INIT_WAL_DIR="--waldir=${POSTGRES_INITDB_WALDIR}"
      
    fi
    create_dir ${DATADIR}
    rm -rf ${DATADIR}/*
    chown -R postgres:postgres ${DATADIR}
    command="$INITDB -U postgres --pwfile=<(echo "$POSTGRES_PASS") -E ${DEFAULT_ENCODING} --lc-collate=${DEFAULT_COLLATION} --lc-ctype=${DEFAULT_CTYPE} --wal-segsize=${WAL_SEGSIZE} --auth=${PASSWORD_AUTHENTICATION} -D ${DATADIR} ${INIT_WAL_DIR} ${INITDB_EXTRA_ARGS}"

    echo "Initializing with command: $command"
    su - postgres -c "$command"
fi;

[DmitryKvant]

Any news? How to make it work?

Tag pr-309-13-3.1 have same error

[DmitryKvant]

delete environment variable DATADIR and it will keep data in default directory - data-volume:/var/lib/postgresql/13

[lucernae]

Hi, sorry. Been busy with other things. Continuing with the discussion:

If you set the env variable POSTGRES_INITDB_WALDIR to point to a subdirectory of the data directory makes no sense because the env variable is there to ensure that we split the data directory contents.

Yes, I agree it doesn't make sense. I set the env to simulate the entrypoint script behaviour and point out where the real problem is. This was not meant to focus on the environment variable. Let's just drop the the discussion of the above quote, because it brings more confusion than clarity.

It has to be B. The symlink is only created on rare occasions where you are mounting the DATADIR into an empty volume and then you do not specify the POSTGRES_INITDB_WALDIR env. In this case, it will create a default data directory using the env variable https://github.com/kartoza/docker-postgis/blob/develop/scripts/env-data.sh#L6. That env variable has to stay that way though because for legacy if a user mounts an existing data directory it checks if the pg_wal directory exists in the folder and does not startup as per #317

Ok, since it's definitely behaviour B. What we should do is make a guard in entrypoint script so that by default the pg_wal directory is under the DATADIR. Wherever it is. It's very broken to let it stay on unpersisted volume if user only use one volume (the default behaviour).

I think the env variable should stay that way because it corresponds to the official Postgres documentation.

We will use the postgres convention then.

Your proposed entrypoint script fix relies on matching the $POSTGRES_INITDB_WALDIR. I would prefer to just leave it to empty if the user doesn't specify it, so that the DATADIR creation doesn't have to use the --waldir flag. I will elaborate it on the PR.

Proposal for the Expected Behaviour

For first time initialization:

1. First time initialization correctly uses $DATADIR/pg_wal for the persistent waldir location, inside the specified persistent volume path $DATADIR
2. First time initialization correctly cleanup $DATADIR directory and it's content, before initialization.
3. If it is first time initialization and user specify $POSTGRES_INITDB_WALDIR, script checks that the waldir is not under $DATADIR, because it's have to be cleaned up. Else, fails the initialization with warning.

For subsequent run/restart/recreate:

1. $POSTGRES_INITDB_WALDIR does not affect subsequent run/recreate because postgres is using symlink inside $DATADIR that maps pg_wal symlink to the actual pg_wal directory mounted using persistent volume. (There's no way to introspect the path from inside the container, and we don't want to use docker client API).
2. Give warning (but not exiting), if $POSTGRES_INITDB_WALDIR doesn't match with what the pg_wal symlink points to. User has to resolve this by themselves, because we will not be able to decide which one is correct.
3. Regardless of the $POSTGRES_INITDB_WALDIR value and pg_wal symlink target (if they are equal), we can't check if the volume is properly mounted in the target location. So if it's empty, better to just exit with warning (CMIIW @NyakudyaA , pg_wal will always be non empty, right? at least they will have the last live checkpoint).

I will make a PR for this.