[AIRFLOW-3607] collected trigger rule dep check per dag run #4751
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Please reference your jira ticket number in the PR title. It is required for the jira git integration to work. |
amichai07
force-pushed
the
decrease_scheduler_dependencies_queries
branch
from
24f5221
to
519b43a
Compare
Codecov Report
@@ Coverage Diff @@
## master #4751 +/- ##
==========================================
+ Coverage 84.81% 85.08% +0.27%
==========================================
Files 679 723 +44
Lines 38493 39558 +1065
==========================================
+ Hits 32646 33658 +1012
- Misses 5847 5900 +53
Continue to review full report at Codecov.
|
VladiPeled
approved these changes
Feb 21, 2019
amichai07
changed the title
ashb
reviewed
Feb 22, 2019
ashb
reviewed
Feb 22, 2019
ashb
reviewed
Feb 22, 2019
ashb
reviewed
Feb 22, 2019
amichai07
force-pushed
the
decrease_scheduler_dependencies_queries
branch
3 times, most recently
from
e9c50ca
to
bb5c825
Compare
ashb
reviewed
Apr 30, 2019
ashb
reviewed
Apr 30, 2019
| # see if the task name is in the task upstream for our task | ||
| upstream_tasks = [finished_task for finished_task in dep_context.finished_tasks | ||
| if finished_task.task_id in ti.task.upstream_task_ids] | ||
| if upstream_tasks: |
There was a problem hiding this comment.
I was wondering before this change we had only SQL query, but now it's both python + sql.
🤔
There was a problem hiding this comment.
yes, because we need to share sql results for more than one purpose
There was a problem hiding this comment.
I left the sql query for backwards compatibility but I suppose it will be best that in most places the python will be used.
amichai07
force-pushed
the
decrease_scheduler_dependencies_queries
branch
3 times, most recently
from
5244994
to
e03c63a
Compare
amichai07
force-pushed
the
decrease_scheduler_dependencies_queries
branch
2 times, most recently
from
9f63c64
to
ed922f2
Compare
|
@amichai07 you have failure in the test: |
amichai07
force-pushed
the
decrease_scheduler_dependencies_queries
branch
2 times, most recently
from
da89779
to
3788262
Compare
amichai07
force-pushed
the
decrease_scheduler_dependencies_queries
branch
from
7562be3
to
b9a88f8
Compare
feluelle
reviewed
Nov 5, 2019
| op5 = DummyOperator(task_id='E', trigger_rule=TriggerRule.ONE_FAILED) | ||
|
|
||
| op1.set_downstream([op2, op3]) # op1 >> op2, op3 | ||
| op4.set_upstream([op3, op2]) # op3 >> op4 |
There was a problem hiding this comment.
Suggested change
| op4.set_upstream([op3, op2]) # op3 >> op4 | |
| op4.set_upstream([op3, op2]) # (op3, op2) >> op4 |
There was a problem hiding this comment.
I think it is also fine to use the >> operator. You just need to disable the related pylint rule.
There was a problem hiding this comment.
Thanks I will fix it
ashb
requested changes
Nov 5, 2019
6 tasks
galuszkak
pushed a commit
to FlyrInc/apache-airflow
that referenced
this pull request
Mar 5, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run
kaxil
added a commit
to kaxil/airflow
that referenced
this pull request
Mar 31, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run
kaxil
added a commit
that referenced
this pull request
Mar 31, 2020
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run
yuqian90
pushed a commit
to yuqian90/airflow
that referenced
this pull request
Sep 21, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
turbaszek
pushed a commit
to PolideaInternal/airflow
that referenced
this pull request
Oct 1, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run
turbaszek
pushed a commit
to PolideaInternal/airflow
that referenced
this pull request
Oct 1, 2020
[AIRFLOW-3607] Only query DB once per DAG run for TriggerRuleDep (apache#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run [AIRFLOW-3607] fix scheduler bug related to concurrency and depends on past (apache#7402) commit 50efda5 introduced a bug that prevents scheduler from scheduling tasks with the following properties: * has depends on past set to True * has custom concurrency limit [AIRFLOW-3607] Optimize dep checking when depends on past set and concurrency limit (apache#7503)
8 tasks
yuqian90
pushed a commit
to yuqian90/airflow
that referenced
this pull request
Oct 26, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
kaxil
pushed a commit
that referenced
this pull request
Oct 26, 2020
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
kaxil
pushed a commit
that referenced
this pull request
Nov 12, 2020
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
kaxil
pushed a commit
that referenced
this pull request
Nov 12, 2020
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
potiuk
pushed a commit
that referenced
this pull request
Nov 16, 2020
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
kaxil
pushed a commit
that referenced
this pull request
Nov 18, 2020
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
kaxil
pushed a commit
to astronomer/airflow
that referenced
this pull request
Nov 24, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5) (cherry picked from commit cb750c1)
kaxil
pushed a commit
to astronomer/airflow
that referenced
this pull request
Nov 27, 2020
…che#4751) This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5) (cherry picked from commit cb750c1)
cfei18
pushed a commit
to cfei18/incubator-airflow
that referenced
this pull request
Mar 5, 2021
This decreases scheduler delay between tasks by about 20% for larger DAGs, sometimes more for larger or more complex DAGs. The delay between tasks can be a major issue, especially when we have dags with many subdags, figures out that the scheduling process spends plenty of time in dependency checking, we took the trigger rule dependency which calls the db for each task instance, we made it call the db just once for each dag_run (cherry picked from commit 50efda5)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira
Description
figures out that the scheduling process spends plenty of time in dependency checking, we took the
trigger rule dependency which calls the db for each task instance, we made it call the db just once for
each dag_run.
Tests
Commits
Documentation
no need for new docs
Code Quality
flake8Results
The tests was made on a heavily multitasks dag (35 tasks).
The tasks don't do any db queries
On local environment
before changes:
after collecting dep check queries:
Stress test - running the dag for every 10 sec for an hour:
before changes:
after:
On production environment
before changes:
after:
Stress test - running with 150 other dags:
before changes:
after:
Edit
we recently did a stress test to check this change again with version 1.10.4
we did the test on a staging like production environment with one dag with 49 tasks that starts once a minute :
This is a graph of the time delay between two tasks in the dag by time:
This graph shows the db read iops difference which can explain the change (the times are different because this is in utc)
Conclusion
The query that we changed indeed had a dramatic impact on the performance of the scheduler. Reusing db results decreased the delay notably and gave the system chance to recover from stress.