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/09/05 13:42:55 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request, #26161: Fix backfill occassional deadlocking
ephraimbuddy opened a new pull request, #26161:
URL: https://github.com/apache/airflow/pull/26161
During backfilling, we set all task instances of the dagrun being run to scheduled,
this causes deadlocking of the dagrun whenever dagrun.update_state is called and
there's no running or schedulable task instances.
The fix was to remove the batch update of the task instances to scheduled state
and have the executor queue the tasks that the dependencies have been met.
--
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 diff in pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #26161:
URL: https://github.com/apache/airflow/pull/26161#discussion_r963482812
##########
airflow/jobs/backfill_job.py:
##########
@@ -351,18 +351,19 @@ def _task_instances_for_dag_run(self, dag_run, session=None):
dag_run.refresh_from_db()
make_transient(dag_run)
+ dag_run.dag = dag
Review Comment:
when we call `task_instance_scheduling_decision` it tries to do `self.get_dag()` and if the dag attribute is not set, it fails.
--
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] uranusjr commented on a diff in pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26161:
URL: https://github.com/apache/airflow/pull/26161#discussion_r963442648
##########
airflow/jobs/backfill_job.py:
##########
@@ -351,18 +351,19 @@ def _task_instances_for_dag_run(self, dag_run, session=None):
dag_run.refresh_from_db()
make_transient(dag_run)
+ dag_run.dag = dag
Review Comment:
`dag_run` does not generally have a `dag` attribute (and I don’t think there’s any code expecting it), why is this needed?
--
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 #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26161:
URL: https://github.com/apache/airflow/pull/26161#issuecomment-1237417144
Comment: I think it would be nice to get some description of state changes and what State.NONE really is.
I thin it would be really great to get a visual representation of all state changes for tasks in a few "state diagrams". Or maybe I am missing it and it is somewhere? Maybe we can have a nice set of state diagrams in mermaid since it is now nice integrated in GitHub ?
Happy to collaborate on that one.
--
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 #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26161:
URL: https://github.com/apache/airflow/pull/26161#issuecomment-1237433043
Ah Thanks @kaxil -> I remembered we had one - just could not find 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] ephraimbuddy commented on a diff in pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #26161:
URL: https://github.com/apache/airflow/pull/26161#discussion_r963487940
##########
airflow/jobs/backfill_job.py:
##########
@@ -351,18 +351,19 @@ def _task_instances_for_dag_run(self, dag_run, session=None):
dag_run.refresh_from_db()
make_transient(dag_run)
+ dag_run.dag = dag
Review Comment:
The test I added failed due to a lack of `dag` attribute but that could be because the `dagrun` on the test does not have the `dag` attribute. However, I decided that it's better to have the `dag` attribute set in the code instead of making the test pass by adding the attribute on the dr in the test
--
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] uranusjr merged pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
uranusjr merged PR #26161:
URL: https://github.com/apache/airflow/pull/26161
--
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 pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on PR #26161:
URL: https://github.com/apache/airflow/pull/26161#issuecomment-1237848665
> LGTM. So if I understand now - Scheduler will simply move tasks to "Scheduled" state and further and that should prevent the deadlocks as only scheduler(s) will be doing it ?
I have updated the code. Previously, it would be sent to the executor to be queued but now it follows scheduled -> queued etc.
Thanks for the question :)
--
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] uranusjr commented on a diff in pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #26161:
URL: https://github.com/apache/airflow/pull/26161#discussion_r963495819
##########
airflow/jobs/backfill_job.py:
##########
@@ -351,18 +351,19 @@ def _task_instances_for_dag_run(self, dag_run, session=None):
dag_run.refresh_from_db()
make_transient(dag_run)
+ dag_run.dag = dag
Review Comment:
Damn these old code are so hard to follow.
--
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] kaxil commented on pull request #26161: Fix backfill occassional deadlocking
Posted by GitBox <gi...@apache.org>.
kaxil commented on PR #26161:
URL: https://github.com/apache/airflow/pull/26161#issuecomment-1237431558
> Comment: I think it would be nice to get some description of state changes and what State.NONE really is.
>
> I thin it would be really great to get a visual representation of all state changes for tasks in a few "state diagrams". Or maybe I am missing it and it is somewhere? Maybe we can have a nice set of state diagrams in mermaid since it is now nice integrated in GitHub ?
>
> Happy to collaborate on that one.
We have one at https://airflow.apache.org/docs/apache-airflow/stable/concepts/tasks.html#task-instances but that might not be in mermaid ->
(https://airflow.apache.org/docs/apache-airflow/stable/_images/task_lifecycle_diagram.png)
--
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