Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AIRFLOW-1104 Update jobs.py so Airflow does not over schedule tasks #3568

Merged
merged 1 commit into from
Jul 31, 2018
Merged

AIRFLOW-1104 Update jobs.py so Airflow does not over schedule tasks #3568

merged 1 commit into from
Jul 31, 2018

Conversation

dan-sf
Copy link
Contributor

@dan-sf dan-sf commented Jul 2, 2018

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag

JIRA

Description

  • This fix updates jobs.py so it will count queued tasks against the concurrency limits. Currently the scheduler will over schedule highly concurrent dags producing the following message in the logs:

FIXME: Rescheduling due to concurrency limits reached at task runtime.

Tests

  • Added a unit test

@codecov-io
Copy link

codecov-io commented Jul 2, 2018

Codecov Report

Merging #3568 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3568      +/-   ##
==========================================
+ Coverage   77.51%   77.51%   +<.01%     
==========================================
  Files         205      205              
  Lines       15751    15751              
==========================================
+ Hits        12209    12210       +1     
+ Misses       3542     3541       -1
Impacted Files Coverage Δ
airflow/jobs.py 82.74% <100%> (ø) ⬆️
airflow/models.py 88.58% <0%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b35d36...b04c9b1. Read the comment docs.

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes sense to me. @bolkedebruin @mistercrunch any thoughts?

@mistercrunch
Copy link
Member

@saguziel ^^^

@saguziel
Copy link
Contributor

saguziel commented Jul 6, 2018

LGTM

@bolkedebruin
Copy link
Contributor

@saguziel this seems such a trivial fix, why wasn't it done earlier? Ie. honest question, there must have been a reason I assume?

@kaxil
Copy link
Member

kaxil commented Jul 21, 2018

@saguziel Was there a specific reason earlier to not do this like @bolkedebruin asks or if there was no reason we should merge this one.

@kaxil
Copy link
Member

kaxil commented Jul 31, 2018

@dan-sf Can you please resolve the conflicts?

@dan-sf
Copy link
Contributor Author

dan-sf commented Jul 31, 2018

@kaxil Conflicts have been updated

@kaxil
Copy link
Member

kaxil commented Jul 31, 2018

Can you squash your commits as well?

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
@dan-sf
Copy link
Contributor Author

dan-sf commented Jul 31, 2018

Sure, the changes have been rebased on master

@kaxil kaxil merged commit ed97204 into apache:master Jul 31, 2018
lxneng pushed a commit to lxneng/incubator-airflow that referenced this pull request Aug 10, 2018
…apache#3568)

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
ms32035 pushed a commit to ms32035/airflow that referenced this pull request Sep 17, 2018
…apache#3568)

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
kaxil pushed a commit that referenced this pull request Oct 3, 2018
…#3568)

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
ashb pushed a commit to ashb/airflow that referenced this pull request Oct 22, 2018
…apache#3568)

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
galak75 pushed a commit to VilledeMontreal/incubator-airflow that referenced this pull request Nov 23, 2018
…apache#3568)

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
…apache#3568)

This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Jan 23, 2019
This change will prevent tasks from getting scheduled and queued over
the concurrency limits set for the dag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants