You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "antruigon (via GitHub)" <gi...@apache.org> on 2026/08/06 12:07:01 UTC
[PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
antruigon opened a new pull request, #71237:
URL: https://github.com/apache/airflow/pull/71237
## What
Adds `pgbouncer.terminationGracePeriodSeconds` to the Helm chart, rendered on the PgBouncer Deployment's pod spec. Defaults to **120** to match the drain window of the chart's default preStop hook.
## Why
The chart ships a default PgBouncer preStop hook that drains client connections for up to 120 seconds:
```yaml
containerLifecycleHooks:
preStop:
exec:
# Allow existing queries clients to complete within 120 seconds
command: ["/bin/sh", "-c", "killall -INT pgbouncer && sleep 120"]
```
…but the Deployment never sets `terminationGracePeriodSeconds`, so the Kubernetes default of **30s** SIGKILLs the pod mid-drain. On any node drain or eviction (cluster upgrades, autoscaler scale-down, AMI rolls) in-flight client connections are cut instead of drained — workers see `psycopg2.OperationalError: server closed the connection unexpectedly`, and tasks that were mid-query fail. We hit this in production during a routine EKS node-group AMI roll: one evicted PgBouncer replica cut ~880 in-flight connections and failed 21 zero-retry DAG runs.
Every other long-running component in the chart already exposes this knob (`scheduler`, `workers.celery`, `triggerer`, `dagProcessor`, `statsd`, `redis`, `otelCollector`); PgBouncer was the only one missing it, and it is arguably the component that needs it most since it holds everyone's DB connections.
## Default choice
`120`, matching the shipped preStop. Note this is an upper bound, not a fixed wait: PgBouncer receives SIGINT immediately, stops accepting new connections, and exits as soon as the last client connection is released — so pods still terminate quickly under low load. Users who prefer the old behaviour can set it back to `30`.
## Changes
- `chart/values.yaml` — new `pgbouncer.terminationGracePeriodSeconds: 120` next to the preStop hook it pairs with
- `chart/values.schema.json` — schema entry (`integer`, default 120, Kubernetes docs section)
- `chart/templates/pgbouncer/pgbouncer-deployment.yaml` — render the field on the pod spec
- `chart/tests/helm_tests/other/test_pgbouncer.py` — parametrized test for default + override (mirrors `test_statsd.py`)
All 86 tests in `test_pgbouncer.py` + `test_pdb_pgbouncer.py` pass locally.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on PR #71237:
URL: https://github.com/apache/airflow/pull/71237#issuecomment-5285812984
Could you please match the PR description with the project template and follow https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #71237:
URL: https://github.com/apache/airflow/pull/71237#issuecomment-5204425284
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our [Contributors' Guide](https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [prek-hooks]( https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst#prerequisites-for-prek-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/airflow-core/docs/howto/custom-operator.rst) Consider adding an example Dag that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/dev/breeze/doc/README.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "antruigon (via GitHub)" <gi...@apache.org>.
antruigon commented on code in PR #71237:
URL: https://github.com/apache/airflow/pull/71237#discussion_r3803274835
##########
chart/newsfragments/71237.significant.rst:
##########
@@ -0,0 +1,5 @@
+PgBouncer Deployment now sets ``terminationGracePeriodSeconds`` (default ``120``)
Review Comment:
Removed, thanks
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "Miretpl (via GitHub)" <gi...@apache.org>.
Miretpl commented on code in PR #71237:
URL: https://github.com/apache/airflow/pull/71237#discussion_r3778672063
##########
chart/newsfragments/71237.significant.rst:
##########
@@ -0,0 +1,5 @@
+PgBouncer Deployment now sets ``terminationGracePeriodSeconds`` (default ``120``)
Review Comment:
I think that this change does not need the newsfragment. Could you delete it?
##########
chart/values.schema.json:
##########
@@ -7801,6 +7801,12 @@
}
]
},
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes. The default matches the default preStop hook, which needs up to 120 seconds to drain client connections.",
Review Comment:
```suggestion
"description": "Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes.",
```
It is more justification for why we have default on 120 and it is rather not needed on the parameter documentation level.
##########
chart/values.schema.json:
##########
@@ -7801,6 +7801,12 @@
}
]
},
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes. The default matches the default preStop hook, which needs up to 120 seconds to drain client connections.",
+ "type": "integer",
+ "default": 120,
Review Comment:
```suggestion
"default": 120,
"minimum": 0
```
for better validation.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #71237:
URL: https://github.com/apache/airflow/pull/71237#issuecomment-5332173720
### Backport successfully created: chart/v1-2x-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>chart/v1-2x-test</td>
<td><a href="https://github.com/apache/airflow/pull/71787"><img src="https://img.shields.io/badge/PR-71787-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "antruigon (via GitHub)" <gi...@apache.org>.
antruigon commented on PR #71237:
URL: https://github.com/apache/airflow/pull/71237#issuecomment-5327095532
Done, sorry for the miss. The description now follows the project template and includes the Gen-AI disclosure. The change was AI-assisted and reviewed by me. I validated the rendered chart and ran the helm tests locally (test_pgbouncer.py, test_pdb_pgbouncer.py, test_chart_quality.py) before pushing
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "antruigon (via GitHub)" <gi...@apache.org>.
antruigon commented on code in PR #71237:
URL: https://github.com/apache/airflow/pull/71237#discussion_r3803277393
##########
chart/values.schema.json:
##########
@@ -7801,6 +7801,12 @@
}
]
},
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes. The default matches the default preStop hook, which needs up to 120 seconds to drain client connections.",
Review Comment:
Applied. The rationale for the 120 default now lives as a comment in `values.yaml`
##########
chart/values.schema.json:
##########
@@ -7801,6 +7801,12 @@
}
]
},
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period for PgBouncer to finish after SIGTERM is sent from Kubernetes. The default matches the default preStop hook, which needs up to 120 seconds to drain client connections.",
+ "type": "integer",
+ "default": 120,
Review Comment:
Applied, thanks
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #71237:
URL: https://github.com/apache/airflow/pull/71237#issuecomment-5332159366
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add terminationGracePeriodSeconds support for PgBouncer in Helm chart [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #71237:
URL: https://github.com/apache/airflow/pull/71237
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org