bramhanandlingala commented on code in PR #69608:
URL: https://github.com/apache/airflow/pull/69608#discussion_r3557080630
##########
chart/values.yaml:
##########
@@ -2859,8 +2859,10 @@ pgbouncer:
containerLifecycleHooks:
preStop:
exec:
- # Allow existing queries clients to complete within 120 seconds
- command: ["/bin/sh", "-c", "killall -INT pgbouncer && sleep 120"]
+ # Sleep first so the pod is marked Terminating and removed from
+ # Service endpoints before pgbouncer stops accepting connections,
+ # then gracefully drain existing queries.
+ command: ["/bin/sh", "-c", "sleep 10 && killall -INT pgbouncer &&
sleep 20"]
Review Comment:
Thanks for the review — updated the comment wording per your suggestion.
On && sleep 20, I'd like to keep it. killall -INT pgbouncer returns as soon
as the signal is sent, not once pgbouncer finishes draining — so without a
delay after it, Kubernetes immediately sends SIGTERM next. Per pgbouncer's
docs, SIGTERM arriving while a SIGINT-triggered shutdown is still in progress
escalates to an immediate/hard shutdown, killing connections that hadn't
finished draining yet — close to the exact bug this PR fixes.
The sleep 20 is what gives pgbouncer time to finish gracefully before that
second signal can reach it.
I'd rather keep it as-is, and I'm happy to add a comment explaining this.
Let me know if there's something
pgbouncer/Kubernetes-specific I'm missing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
