ashb commented on code in PR #76749:
URL: https://github.com/apache/airflow/pull/76749#discussion_r4518547638
##########
chart/values.schema.json:
##########
@@ -1747,6 +1747,14 @@
"items": {
"type": "string"
}
+ },
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period, in seconds,
for a kill triggered by this probe. Overrides the pod's
`terminationGracePeriodSeconds` for liveness kills only.",
+ "type": [
+ "integer",
+ "null"
+ ],
+ "default": null
Review Comment:
Nit: this shouldn't allow null. It should not be required, but if set it
shouldn't be null?
##########
chart/values.yaml:
##########
@@ -646,6 +646,12 @@ workers:
failureThreshold: 5
periodSeconds: 60
command: ~
+ # Grace period for a kill triggered by this probe. Unset, the pod's
+ # `terminationGracePeriodSeconds` applies, which workers often set to
hours
+ # so that a rolling update drains running tasks; a worker killed because
it
+ # stopped answering is not draining anything, so a shorter period here
+ # restarts it sooner.
Review Comment:
But when does this longer period actually fire? If the probe fails, nothing
is running? So it should exit Pretty Damn Quick anyway?
Am I missing something?
##########
chart/values.schema.json:
##########
@@ -1747,6 +1747,14 @@
"items": {
"type": "string"
}
+ },
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period, in seconds,
for a kill triggered by this probe. Overrides the pod's
`terminationGracePeriodSeconds` for liveness kills only.",
Review Comment:
This doesn't quite make sense to me. Why would we want the grace period for
a failing probe different from a pod stop signal? (Have I understood what this
config controls)
--
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]
