bitnami / containers

Bitnami container images
https://bitnami.com
Other
3.39k stars 4.86k forks source link

[bitnami/postgresql] Make SIGINT default stopsignal #40519

Open dgeissl opened 1 year ago

dgeissl commented 1 year ago

Name and Version

bitnami/postgresql*

What is the problem this feature will solve?

The default stop signal for containers seems to be SIGTERM which will lead to a greaceful shutdown, waiting for any connections until they are closed before terminating the postgresql process. This can be seen in the Log during shutdown:

[1] LOG:  received smart shutdown request

The downside is that your typical runtime does not wait forever (docker stop 10s, k8s 30s) and will just kill your process if it did not finish within that timeframe:

[134] FATAL:  terminating connection due to unexpected postmaster exit

this will likely cause data corruptions and has been discussed in the Docker "Official Image".

What is the feature you are proposing to solve the problem?

The discussion https://github.com/docker-library/postgres/issues/714 resulted in changing the default termination signal to SIGINT (see https://github.com/docker-library/postgres/pull/763).

I think it would also be beneficial to make this the default on bitnami images. Changing the behavior by the docker-compose.yaml led to the following log output that looked way better than before:

[1] LOG:  received fast shutdown request
[1] LOG:  aborting any active transactions
[1] LOG:  background worker "logical replication launcher" (PID 86) exited with exit code 1
[81] LOG:  shutting down

What alternatives have you considered?

We can change the signal in our own derived images. on the docker cli with docker run ---stop-signal=SIGINT of for docker compose with:

version: '3.6'
services:
  postgres:
    stop_signal: SIGINT
    image: bitnami/postgresql

In kubernetes there seems to be a ongoing discussion https://github.com/kubernetes/kubernetes/issues/30051 to support that but it's currently only possible if you build your own image (at least that's what I found).

Having said that I'll fix this in my derived images, but one should consider making that the default.

javsalgar commented 1 year ago

Hi!

Thank you so much for the feature request! I will forward it to the engineering team.

github-actions[bot] commented 1 year ago

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

dgeissl commented 1 year ago

@javsalgar is this smth your team want's to work on or should I ignore the stale notification next time as it won't be tackeled anyways?

javsalgar commented 1 year ago

Hi,

I'm afraid the task is still in the backlog. I added the on-hold label so the stale bot does not close it.