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/01/25 20:20:47 UTC

[GitHub] [airflow] dstandish opened a new pull request #21108: Fix liveness probe speedup for scheduler and triggerer

dstandish opened a new pull request #21108:
URL: https://github.com/apache/airflow/pull/21108


   PR https://github.com/apache/airflow/pull/20833 tried to speed up the liveness probe by setting variable CONNECTION_CHECK_MAX_COUNT=0 which disables a connectivity check in `/entrypoint` (which turns out to be slow).
   
   Unfortunately the approach taken doesn't work; we have to use `sh -c exec` instead.
   


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



[GitHub] [airflow] dstandish commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792239394



##########
File path: chart/templates/triggerer/triggerer-deployment.yaml
##########
@@ -165,26 +165,26 @@ spec:
             periodSeconds: {{ .Values.triggerer.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "
+                  import os
+                  os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
+                  os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
 
-                from airflow.jobs.triggerer_job import TriggererJob
-                from airflow.utils.db import create_session
-                from airflow.utils.net import get_hostname
-                import sys
+                  from airflow.jobs.scheduler_job import SchedulerJob

Review comment:
       should be fixed now




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



[GitHub] [airflow] ephraimbuddy commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792653987



##########
File path: chart/templates/scheduler/scheduler-deployment.yaml
##########
@@ -162,26 +162,26 @@ spec:
             periodSeconds: {{ .Values.scheduler.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "

Review comment:
       Curious where `CONNECTION_CHECK_MAX_COUNT=0` is used




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



[GitHub] [airflow] potiuk commented on pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#issuecomment-1022518929


   If pre-commit approves. I am fine too :shrug: :)


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



[GitHub] [airflow] dstandish commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792126072



##########
File path: chart/templates/triggerer/triggerer-deployment.yaml
##########
@@ -165,26 +165,26 @@ spec:
             periodSeconds: {{ .Values.triggerer.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "
+                  import os
+                  os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
+                  os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
 
-                from airflow.jobs.triggerer_job import TriggererJob
-                from airflow.utils.db import create_session
-                from airflow.utils.net import get_hostname
-                import sys
+                  from airflow.jobs.scheduler_job import SchedulerJob

Review comment:
       Oy, I guess that's why we have reviews




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



[GitHub] [airflow] github-actions[bot] commented on pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#issuecomment-1022519076


   The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.


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



[GitHub] [airflow] dstandish commented on pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
dstandish commented on pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#issuecomment-1022493432


   fixed the bad copy paste.  also i notice that my IDE formatted with indent for the list not sure if we care about that... but there's no pre-commit for it so 🤷 
   


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



[GitHub] [airflow] potiuk commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792927710



##########
File path: chart/templates/scheduler/scheduler-deployment.yaml
##########
@@ -162,26 +162,26 @@ spec:
             periodSeconds: {{ .Values.scheduler.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "

Review comment:
       Yep: https://airflow.apache.org/docs/docker-stack/entrypoint.html#waits-for-airflow-db-connection




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



[GitHub] [airflow] jedcunningham commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
jedcunningham commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792112070



##########
File path: chart/templates/triggerer/triggerer-deployment.yaml
##########
@@ -165,26 +165,26 @@ spec:
             periodSeconds: {{ .Values.triggerer.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "
+                  import os
+                  os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
+                  os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
 
-                from airflow.jobs.triggerer_job import TriggererJob
-                from airflow.utils.db import create_session
-                from airflow.utils.net import get_hostname
-                import sys
+                  from airflow.jobs.scheduler_job import SchedulerJob

Review comment:
       Oops, bad copy/paste here!




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



[GitHub] [airflow] ephraimbuddy commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792653987



##########
File path: chart/templates/scheduler/scheduler-deployment.yaml
##########
@@ -162,26 +162,26 @@ spec:
             periodSeconds: {{ .Values.scheduler.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "

Review comment:
       Curious where `CONNECTION_CHECK_MAX_COUNT=0` is used. Nevermind, got 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



[GitHub] [airflow] jedcunningham merged pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
jedcunningham merged pull request #21108:
URL: https://github.com/apache/airflow/pull/21108


   


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



[GitHub] [airflow] dstandish commented on a change in pull request #21108: Fix liveness probe speedup for scheduler and triggerer

Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #21108:
URL: https://github.com/apache/airflow/pull/21108#discussion_r792744145



##########
File path: chart/templates/scheduler/scheduler-deployment.yaml
##########
@@ -162,26 +162,26 @@ spec:
             periodSeconds: {{ .Values.scheduler.livenessProbe.periodSeconds }}
             exec:
               command:
-              - CONNECTION_CHECK_MAX_COUNT=0
-              - /entrypoint
-              - python
-              - -Wignore
-              - -c
-              - |
-                import os
-                os.environ['AIRFLOW__CORE__LOGGING_LEVEL'] = 'ERROR'
-                os.environ['AIRFLOW__LOGGING__LOGGING_LEVEL'] = 'ERROR'
+                - sh
+                - -c
+                - exec
+                - |
+                  CONNECTION_CHECK_MAX_COUNT=0 /entrypoint python -Wignore -c "

Review comment:
       Yeah, in entrypoint




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