pgbouncer prestop command is not graceful enough

[andrew-stein-sp]

Under which category would you file this issue?

Helm chart

Apache Airflow version

2.11.0(airflow version is not applicable to this issue)

What happened and how to reproduce it?

To reproduce:

• perform a rolling restart of the pgbouncer deployment(with multiple replicas) in an airflow cluster and kubernetes cluster under heavy load
• observe scheduler, dag-processor, and worker pods
  • some pods will see restarts with a SQL connection error like "No route to host" or "Connection refused"

What you think should happen instead?

The prestop command used for pgbouncer in the helm chart(shown below) is problematic:

containerLifecycleHooks:
    preStop:
      exec:
        # Allow existing queries clients to complete within 120 seconds
        command: ["/bin/sh", "-c", "killall -INT pgbouncer && sleep 120"]

Although killall -INT pgbouncer does in fact trigger a safe shutdown as expected, it exits as soon as in-flight queries drain. Once this occurs, the liveness probe terminates the pod regardless of whether the sleep 120 has finished or not. In the event of an idle pgbouncer pod or one with no long-running queries, this happens in ~1 second or less. The container therefore stops accepting connections before the EndpointSlice/kube-proxy updates triggered by the pod entering Terminating have propagated, so new connections routed to the still-registered (but dead) pod fail with connection errors.

I've fixed this by putting a short sleep command in front of killall -INT pgbouncer like so:

containerLifecycleHooks:
    preStop:
      exec:
        # Allow existing queries clients to complete within 120 seconds
        command: ["/bin/sh", "-c", "sleep 10 && killall -INT pgbouncer && sleep 20"]

This allows the pod to transition to a terminating state while the pgbouncer container continues to run, which grants kubernetes more time to remove the target from the service endpoint and update the loadbalancer and kube-proxy rules. Now, if a query gets sent to the terminating pod before its been removed from the service endpoint fully, it is less likely to experience a connection failure because pgbouncer is still running and will still accept the query.

➜  ~ kubectl get endpointslice -o json -n airflow airflow-<redacted>-pgbouncer-n57hm | grep '"terminating": true' -B2
--
                "ready": true,
                "serving": true,
                "terminating": true

Also, I made the post sleep command 20 becuase there is no TerminationGracePeriod defined for the pgbouncer deployment in the chart, so it ends up defaulting to 30s. Basically, the sleep 120 doesn't really do what it says and is therefore misleading to users of the chart.

Operating System

Amazon Linux 2023(fedora)

Deployment

Official Apache Airflow Helm Chart

Apache Airflow Provider(s)

No response

Versions of Apache Airflow Providers

No response

Official Helm Chart version

1.22.0 (latest released)

Kubernetes Version

1.35

Helm Chart configuration

No response

Docker Image customizations

No response

Anything else?

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[Miretpl]

Thanks for creating this issue!

Also, I made the post sleep command 20 becuase there is no TerminationGracePeriod defined for the pgbouncer deployment in the chart, so it ends up defaulting to 30s. Basically, the sleep 120 doesn't really do what it says and is therefore misleading to users of the chart.

IMO, sleep after is pretty much useless; if it is shorter (total command exec time) than the grace period, it doesn't have any effect; if it is longer, it will make the grace period longer by 2 sec, so basically it could be deleted (unless I'm missing something).