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

BugFix: Tasks with depends_on_past or task_concurrency are stuck #12663

Merged
merged 1 commit into from
Nov 27, 2020

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Nov 27, 2020

closes #12659 (Fixes regression in 1.10.13)


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
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.

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Nov 27, 2020
@kaxil kaxil added this to the Airflow 1.10.14 milestone Nov 27, 2020
@kaxil
Copy link
Member Author

kaxil commented Nov 27, 2020

cc @nathadfield

@github-actions
Copy link

The PR needs to run all tests because it modifies core of Airflow! Please rebase it to latest master or ask committer to re-run it!

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Nov 27, 2020
@kaxil kaxil merged commit 2a7944d into apache:v1-10-stable Nov 27, 2020
@kaxil kaxil deleted the fix-bug-11013 branch November 27, 2020 19:42
assert ti_to_schedule == [
(dag.dag_id, dag_task1.task_id, DEFAULT_DATE, TRY_NUMBER),
(dag.dag_id, dag_task2.task_id, DEFAULT_DATE, TRY_NUMBER),
]
Copy link
Member

Choose a reason for hiding this comment

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

I'm observing CI error after this is merged into 1-10-stable.

I assume it's because the order of the two elements in ti_to_schedule is not guaranteed.

Possibly can instead assert like this:

        mock_list = Mock()
        scheduler._process_task_instances(dag, task_instances_list=mock_list)

        mock_list.append.assert_called_with(
            (dag.dag_id, dag_task1.task_id, DEFAULT_DATE, TRY_NUMBER)
        )

(https://github.com/astronomer/airflow/blob/67807ee492482f57442239e271747a5acc69e15b/tests/jobs/test_scheduler_job.py#L1604-L1609)

Copy link
Member

Choose a reason for hiding this comment

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

I will prepare a PR shortly.

Copy link
Member Author

Choose a reason for hiding this comment

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

argghh Python 2.7 & 3.5 I guess because of the ordering

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the order is not guaranteed so assert two lists would fail.

I have the change ready, and will raise the PR now.

@XD-DENG XD-DENG mentioned this pull request Nov 27, 2020
dewaldabrie pushed a commit to CoverGenius/airflow that referenced this pull request Dec 11, 2020
AntonyRileyAtVerto pushed a commit to vertoanalytics/incubator-airflow that referenced this pull request Feb 2, 2021
- BugFix: Tasks with ``depends_on_past`` or ``task_concurrency`` are stuck (apache#12663)
- Fix issue with empty Resources in executor_config (apache#12633)
- Fix: Deprecated config ``force_log_out_after`` was not used (apache#12661)
- Fix empty asctime field in JSON formatted logs (apache#10515)
- [AIRFLOW-2809] Fix security issue regarding Flask SECRET_KEY (apache#3651)
- [AIRFLOW-2884] Fix Flask SECRET_KEY security issue in www_rbac (apache#3729)
- [AIRFLOW-2886] Generate random Flask SECRET_KEY in default config (apache#3738)
- Add missing comma in setup.py (apache#12790)
- Bugfix: Unable to import Airflow plugins on Python 3.8 (apache#12859)
- Fix setup.py missing comma in ``setup_requires`` (apache#12880)
- Don't emit first_task_scheduling_delay metric for only-once dags (apache#12835)

- Update setup.py to get non-conflicting set of dependencies (apache#12636)
- Rename ``[scheduler] max_threads`` to ``[scheduler] parsing_processes`` (apache#12605)
- Add metric for scheduling delay between first run task & expected start time (apache#9544)
- Add new-style 2.0 command names for Airflow 1.10.x (apache#12725)
- Add Kubernetes cleanup-pods CLI command for Helm Chart (apache#11802)
- Don't let webserver run with dangerous config (apache#12747)
- Replace pkg_resources with importlib.metadata to avoid VersionConflict errors (apache#12694)

- Clarified information about supported Databases
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Scheduler including HA (high availability) scheduler full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tasks in DAGs with depends_on_past or task_concurrency are not being scheduled
3 participants