Change TaskInstance and TaskReschedule PK from execution_date to run_id #17719

ashb · 2021-08-19T11:23:44Z

Since TaskReschedule had an existing FK to TaskInstance we had to move
change both of these at the same time.

This puts an explicit FK constraint between TaskInstance and DagRun,
meaning that we can remove a lot of "find TIs without DagRun" code in
the scheduler too, as that is no longer a possible situation.

Since there is now an explicit foreign key between TaskInstance and
DagRun, we can remove a lot of the "cleanup" code in the scheduler that
was dealing with this.

This PR is, sadly, unavoidably large as it changes a fundamental part of the Airflow system, the primary key of the TaskInstance table :(

Closes #17046

Related #16302, #17030

^ 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.

ephraimbuddy

I'm wondering if

airflow/airflow/www/views.py

Lines 1485 to 1486 in 4da4c18

    
           ti = models.TaskInstance(task=task, execution_date=execution_date) 
        
           ti.refresh_from_db()

need a change too?

ashb · 2021-08-19T11:56:36Z

I'm wondering if

airflow/airflow/www/views.py

Lines 1485 to 1486 in 4da4c18

ti = models.TaskInstance(task=task, execution_date=execution_date)

ti.refresh_from_db()

need a change too?

Yes it should. It'll still work as it is but that relies on the compat shims

ashb · 2021-08-19T20:53:23Z

I'm not quite expecting all tests to pass on this yet, but I'd like a review on this, so I'm marking it ready while I work on fixing up the remaining tests

ashb · 2021-08-19T20:53:57Z

tests/core/test_core.py

@@ -314,64 +330,12 @@ def test_task_get_template(self):
            assert value == expected_value
            assert [str(m.message) for m in recorder] == [message]

-    def test_local_task_job(self):


These tests are duplicating what was already in test_local_task_job -- we don't need them again.

ashb · 2021-08-19T20:55:41Z

airflow/jobs/scheduler_job.py

@@ -189,80 +189,6 @@ def is_alive(self, grace_multiplier: Optional[float] = None) -> bool:
            and (timezone.utcnow() - self.latest_heartbeat).total_seconds() < scheduler_health_check_threshold
        )

-    @provide_session
-    def _change_state_for_tis_without_dagrun(


This is deleted, as the DB won't let us have TIs without Dagruns anymore :)

Huge, can clear the issue of tasks getting stuck!

Since tasks will no longer exist without dagrun, obviously the dag will fail or succeed and tis won't be stuck. I think we should remove these lines.:

airflow/airflow/jobs/scheduler_job.py

Lines 603 to 604 in 5d90bcb

self.log.info('Setting task instance %s state to %s as reported by executor', ti, state)

ti.set_state(state)

I think this can happen if (for example) there is a crash in the worker early on in the "boot" process, so it can't report it's own status in the DB.

airflow/jobs/scheduler_job.py

uranusjr

Only read about 1/5 (probably less) of the patch. Generally seems fine to me but some thoughts on the implementation.

airflow/cli/commands/task_command.py

airflow/jobs/backfill_job.py

airflow/models/baseoperator.py

airflow/models/dagrun.py

airflow/models/skipmixin.py

uranusjr

Nervously

boring-cyborg bot added area:CLI area:providers area:Scheduler Scheduler or dag parsing Issues area:webserver Webserver related Issues kind:documentation provider:google Google (including GCP) related issues labels Aug 19, 2021

ashb requested a review from uranusjr August 19, 2021 11:23

ashb added AIP-39 Timetables and removed provider:google Google (including GCP) related issues area:webserver Webserver related Issues area:CLI area:providers kind:documentation labels Aug 19, 2021

ashb added this to In progress in AIP-39 Pluggable schedule_interval via automation Aug 19, 2021

ashb moved this from In progress to Review in progress in AIP-39 Pluggable schedule_interval Aug 19, 2021

ashb force-pushed the delegate-exec-date-to-dagrun branch from 94e0cb2 to 1a1cd01 Compare August 19, 2021 11:26

ephraimbuddy reviewed Aug 19, 2021

View reviewed changes

ashb force-pushed the delegate-exec-date-to-dagrun branch from 1a1cd01 to 27c58db Compare August 19, 2021 20:52

ashb marked this pull request as ready for review August 19, 2021 20:53

ashb requested review from kaxil, ryanahamilton, turbaszek and XD-DENG as code owners August 19, 2021 20:53

ashb commented Aug 19, 2021

View reviewed changes

airflow/jobs/scheduler_job.py Outdated Show resolved Hide resolved

uranusjr reviewed Aug 20, 2021

View reviewed changes

ashb removed the request for review from ryanahamilton August 20, 2021 08:21

uranusjr and others added 15 commits September 6, 2021 20:44

Rewrite gantt view queries to use DagRun join

2bca8fd

Further simplify queries in gantt view

4685b9f

Make tests more robust against ti order in db

5eb82e6

Less queries on MSSQL!

7cadf5b

Fix unique constraint violation in tests

1496ce5

Fix constraint names on SQLite

1b2d2c6

Fix scheduler job test by requiring serialized DAG

eba4459

fixup! Fix constraint names on SQLite

8d955b1

Attempt to restore session sanity

0845f43

fixup! Fix (most of) task and tree view tests

c79e0b3

Fix sentry tests

3669b2e

More ti ordering fixes

bd9a645

Fix kube executor

b9cf613

Fix KubernetesPodOperator tests

62ef463

PR comments

8dd8622

ashb force-pushed the delegate-exec-date-to-dagrun branch from 178ce9e to 8dd8622 Compare September 6, 2021 19:45

Fix botched suggestion

d877e2d

uranusjr approved these changes Sep 7, 2021

View reviewed changes

AIP-39 Pluggable schedule_interval automation moved this from In progress to Reviewer approved Sep 7, 2021

ashb merged commit 944dcfb into apache:main Sep 7, 2021

AIP-39 Pluggable schedule_interval automation moved this from Reviewer approved to Done Sep 7, 2021

ashb deleted the delegate-exec-date-to-dagrun branch September 7, 2021 08:16

Taragolis mentioned this pull request Sep 12, 2021

NoResultFound raises in UI when access to Task Instance Details #18188

Closed

2 tasks

kaxil mentioned this pull request Oct 6, 2021

Deleting task instance from UI deletes Dag Run too #16896

Closed

ephraimbuddy mentioned this pull request Oct 23, 2021

Tasks scheduled with airflow tasks run fails on a query to the meta db #19172

Closed

2 tasks

ud803 mentioned this pull request Nov 9, 2021

Airflow 2.2.0 compatibility teamclairvoyant/airflow-maintenance-dags#117

Open

ephraimbuddy mentioned this pull request Nov 23, 2021

_change_state_for_tis_without_dagrun query triggers mysql full scan #17943

Closed

2 tasks

mhenc mentioned this pull request Apr 28, 2022

airflow_db_cleanup fails on TaskInstance and TaskReschedule tables in Composer 2.x GoogleCloudPlatform/python-docs-samples#7835

Closed

ashb mentioned this pull request Nov 9, 2022

Remove self.using_mysql attribute #27571

Merged

utkarsharma2 mentioned this pull request Jul 11, 2023

execution date is missing from Task Instance tooltip #32503

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change TaskInstance and TaskReschedule PK from execution_date to run_id #17719

Change TaskInstance and TaskReschedule PK from execution_date to run_id #17719

ashb commented Aug 19, 2021 •

edited

Loading

ephraimbuddy left a comment

ashb commented Aug 19, 2021

ashb commented Aug 19, 2021

ashb Aug 19, 2021

ashb Aug 19, 2021

ephraimbuddy Aug 20, 2021

ephraimbuddy Aug 20, 2021

ashb Aug 21, 2021

uranusjr left a comment

uranusjr left a comment

	ti = models.TaskInstance(task=task, execution_date=execution_date)
	ti.refresh_from_db()

	self.log.info('Setting task instance %s state to %s as reported by executor', ti, state)
	ti.set_state(state)

Change TaskInstance and TaskReschedule PK from execution_date to run_id #17719

Change TaskInstance and TaskReschedule PK from execution_date to run_id #17719

Conversation

ashb commented Aug 19, 2021 • edited Loading

ephraimbuddy left a comment

Choose a reason for hiding this comment

ashb commented Aug 19, 2021

ashb commented Aug 19, 2021

ashb Aug 19, 2021

Choose a reason for hiding this comment

ashb Aug 19, 2021

Choose a reason for hiding this comment

ephraimbuddy Aug 20, 2021

Choose a reason for hiding this comment

ephraimbuddy Aug 20, 2021

Choose a reason for hiding this comment

ashb Aug 21, 2021

Choose a reason for hiding this comment

uranusjr left a comment

Choose a reason for hiding this comment

uranusjr left a comment

Choose a reason for hiding this comment

ashb commented Aug 19, 2021 •

edited

Loading