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-7113] fix gantt render error #7913

Merged
merged 1 commit into from
Mar 28, 2020

Conversation

cong-zhu
Copy link
Contributor

@cong-zhu cong-zhu commented Mar 27, 2020


Issue link: AIRFLOW-7113

Make sure to mark the boxes below before creating PR: [x]


In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Mar 27, 2020
@kaxil
Copy link
Member

kaxil commented Mar 27, 2020

@cong-zhu Thanks for raising this PR. I know someone reported this error too. However, I couldn't reproduce the error. Do you have an example DAG or steps I can do to reproduce this error?

@kaxil kaxil added this to the Airflow 1.10.10 milestone Mar 27, 2020
@KevinYang21
Copy link
Member

@kaxil I think if we have tasks marked as failed by the scheduler upon receiving executor event then we'll end up having such failing tasks w/o a start date and crash the gantt chart.

@cong-zhu correct me if I'm wrong.

@KevinYang21
Copy link
Member

The CI error looks irrelevant tho, maybe try rebase and rerun?

@cong-zhu cong-zhu force-pushed the congzhu-fix-gantt-rendering branch from 98edf41 to 4bad6e1 Compare March 28, 2020 03:18
@cong-zhu
Copy link
Contributor Author

@KevinYang21, yes it's correct.

@kaxil. Tasks fail at early stage, e.g. during parsing, won't contain the start date. When handling failures, scheduler will store those failed tasks in task_fail table. Which crashes the gantt chart. To reproduce the error, you need to have a row w/o start_date in task_fail table. Entries w/o start_date in task_instance table will be fine as the query filters them out. I reverted the change on it.

@kaxil
Copy link
Member

kaxil commented Mar 28, 2020

@KevinYang21, yes it's correct.

@kaxil. Tasks fail at early stage, e.g. during parsing, won't contain the start date. When handling failures, scheduler will store those failed tasks in task_fail table. Which crashes the gantt chart. To reproduce the error, you need to have a row w/o start_date in task_fail table. Entries w/o start_date in task_instance table will be fine as the query filters them out. I reverted the change on it.

Shouldn't we change the Scheduler code to store the start_date and if there is any error store start_date = end_date during that time before we store in task_fail table?

@cong-zhu
Copy link
Contributor Author

Shouldn't we change the Scheduler code to store the start_date and if there is any error store start_date = end_date during that time before we store in task_fail table?

I think it still makes sense to store the empty start_date as it indicates the task failed before running. We just need to appropriately represent this from the UI.

@cong-zhu cong-zhu force-pushed the congzhu-fix-gantt-rendering branch from 4bad6e1 to a218284 Compare March 28, 2020 06:18
@cong-zhu cong-zhu force-pushed the congzhu-fix-gantt-rendering branch from a218284 to be2044d Compare March 28, 2020 07:09
@kaxil kaxil merged commit 1eafde2 into apache:master Mar 28, 2020
kaxil pushed a commit that referenced this pull request Mar 28, 2020
kaxil pushed a commit to astronomer/airflow that referenced this pull request Mar 28, 2020
Co-authored-by: Cong Zhu <[email protected]>
(cherry picked from commit d97711a)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Mar 30, 2020
kaxil pushed a commit that referenced this pull request Mar 30, 2020
@kaxil kaxil added kind:bug This is a clearly a bug and removed kind:bug This is a clearly a bug labels Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants