You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/25 23:40:27 UTC

[GitHub] [airflow] jedcunningham commented on a change in pull request #22378: Celery worker liveness probe

jedcunningham commented on a change in pull request #22378:
URL: https://github.com/apache/airflow/pull/22378#discussion_r835666300



##########
File path: chart/templates/workers/worker-deployment.yaml
##########
@@ -213,6 +213,20 @@ spec:
             - name: KRB5CCNAME
               value:  {{ include "kerberos_ccache_path" . | quote }}
           {{- end }}
+          livenessProbe:
+            initialDelaySeconds: {{ .Values.workers.livenessProbe.initialDelaySeconds }}
+            timeoutSeconds: {{ .Values.workers.livenessProbe.timeoutSeconds }}
+            failureThreshold: {{ .Values.workers.livenessProbe.failureThreshold }}
+            periodSeconds: {{ .Values.workers.livenessProbe.periodSeconds }}
+            exec:
+              command:
+                {{- if .Values.workers.livenessProbe.command }}
+                {{ toYaml .Values.workers.livenessProbe.command | nindent 16 }}
+                {{- else}}
+                - sh
+                - -c
+                - exec /entrypoint python -m celery --app airflow.executors.celery_executor.app inspect ping -d celery@${HOSTNAME}

Review comment:
       ```suggestion
                   - CONNECTION_CHECK_MAX_COUNT=0 exec /entrypoint python -m celery --app airflow.executors.celery_executor.app inspect ping -d celery@${HOSTNAME}
   ```
   
   Let's do this as well to keep the probe quick.

##########
File path: chart/values.yaml
##########
@@ -475,6 +475,14 @@ workers:
     # of local-path provisioner.
     fixPermissions: false
 
+  # If the celery stops heartbeating for 5 minutes (5*60s) kill the
+  # celery and let Kubernetes restart it

Review comment:
       ```suggestion
     # If the worker stops responding for 5 minutes (5*60s) kill the
     # worker and let Kubernetes restart it
   ```
   
   nit: it's not really a heartbeat like the scheduler

##########
File path: chart/templates/workers/worker-deployment.yaml
##########
@@ -213,6 +213,20 @@ spec:
             - name: KRB5CCNAME
               value:  {{ include "kerberos_ccache_path" . | quote }}
           {{- end }}
+          livenessProbe:
+            initialDelaySeconds: {{ .Values.workers.livenessProbe.initialDelaySeconds }}
+            timeoutSeconds: {{ .Values.workers.livenessProbe.timeoutSeconds }}
+            failureThreshold: {{ .Values.workers.livenessProbe.failureThreshold }}
+            periodSeconds: {{ .Values.workers.livenessProbe.periodSeconds }}
+            exec:
+              command:
+                {{- if .Values.workers.livenessProbe.command }}
+                {{ toYaml .Values.workers.livenessProbe.command | nindent 16 }}
+                {{- else}}
+                - sh
+                - -c
+                - exec /entrypoint python -m celery --app airflow.executors.celery_executor.app inspect ping -d celery@${HOSTNAME}

Review comment:
       ```suggestion
                   - exec /entrypoint celery --app airflow.executors.celery_executor.app inspect ping -d celery@${HOSTNAME}
   ```
   
   Let's do it this way as that's how the celery docs do it.




-- 
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