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.

Sign up for GitHub

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

Jump to bottom

set executor.job_id to BackfillJob.id for backfills #27020

Merged
merged 1 commit into from
Oct 23, 2022
Merged

set executor.job_id to BackfillJob.id for backfills #27020

merged 1 commit into from
Oct 23, 2022

Conversation

RNHTTR
Copy link
Contributor

@RNHTTR RNHTTR commented Oct 12, 2022

BackfillJob sets executor.job_id = "backfill", which results in a bug in the Celery executor's stalled_task_timeout feature. The query to select stalled tasks is as follows:

session.query(TaskInstance).filter(
    TaskInstance.filter_for_tis(keys),
    TaskInstance.state == State.QUEUED,
    TaskInstance.queued_by_job_id == self.job_id,

TaskInstance.queued_by_job_id is an int, but for backfill jobs, it is comparing against a string (i.e. "backfill"). This prevents stalled tasks from backfill jobs to remain stalled and not be retried.

Setting executor.job_id = self.id (where self.id is BaseJob's id field should resolve this issue.

@RNHTTR RNHTTR requested review from kaxil, ashb and XD-DENG as code owners October 12, 2022 20:27
@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Oct 12, 2022
o-nikolas
o-nikolas approved these changes Oct 13, 2022
View reviewed changes
Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

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

Nice one 👍

@potiuk potiuk added this to the Airflow 2.4.3 milestone Oct 23, 2022
@potiuk
Copy link
Member

potiuk commented Oct 23, 2022

I think that one is good for 2.4.3 - marked it so.

@potiuk potiuk merged commit 57bb54d into apache:main Oct 23, 2022
ephraimbuddy pushed a commit that referenced this pull request Nov 9, 2022
@RNHTTR @ephraimbuddy
set executor.job_id to BackfillJob.id for backfills (#27020)
d4e5186 
Co-authored-by: Ryan Hatter <[email protected]>
(cherry picked from commit 57bb54d)
@ephraimbuddy ephraimbuddy added the type:bug-fix Changelog: Bug Fixes label Nov 9, 2022
@ephraimbuddy ephraimbuddy mentioned this pull request Nov 11, 2022
Closed
18 tasks
@snjypl snjypl mentioned this pull request Dec 15, 2022
Closed
2 tasks
@RNHTTR RNHTTR deleted the bugfix/backfill-job-id-stalled-tasks branch October 24, 2023 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@o-nikolas o-nikolas o-nikolas approved these changes

@ephraimbuddy ephraimbuddy ephraimbuddy approved these changes

@uranusjr uranusjr uranusjr approved these changes

@kaxil kaxil Awaiting requested review from kaxil

@ashb ashb Awaiting requested review from ashb ashb is a code owner

@XD-DENG XD-DENG Awaiting requested review from XD-DENG XD-DENG is a code owner

Assignees
No one assigned
Labels
area:Scheduler including HA (high availability) scheduler type:bug-fix Changelog: Bug Fixes
Projects
None yet
Milestone
Airflow 2.4.3
Development

Successfully merging this pull request may close these issues.
5 participants
@RNHTTR @potiuk @uranusjr @ephraimbuddy @o-nikolas