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

local task job: add overtime mechanism, to not kill task prematurely when auxiliary processes are running #39890

Merged
merged 1 commit into from
Jun 13, 2024
Merged

local task job: add overtime mechanism, to not kill task prematurely when auxiliary processes are running #39890

merged 1 commit into from
Jun 13, 2024

Conversation

mobuchowski
Copy link
Contributor

@mobuchowski mobuchowski commented May 28, 2024 •
edited
Loading

Currently, LocalTaskJobRunner kills worker processes after they have their state set to SUCCESS and are still running. This can happen in few ways - most common are either state is set externally, or there are relatively slow listeners or other processes (mini scheduler) running.

This PR introduces mechanism in which we give the task some additional to finish their job - by default 20 seconds, configurable - after which the process is terminated.

@boring-cyborg boring-cyborg bot added area:providers area:Scheduler including HA (high availability) scheduler provider:openlineage AIP-53 labels May 28, 2024
@mobuchowski mobuchowski force-pushed the listener-task-timeout branch from 460fadf to 32e46dd Compare May 30, 2024 12:57
@mobuchowski mobuchowski marked this pull request as ready for review May 30, 2024 13:01
@mobuchowski mobuchowski requested a review from potiuk May 30, 2024 13:01
@tatiana tatiana mentioned this pull request May 31, 2024
Closed
2 tasks
@mobuchowski mobuchowski force-pushed the listener-task-timeout branch 2 times, most recently from 430c4fa to e2acd51 Compare June 4, 2024 16:52
@mobuchowski mobuchowski changed the title local task job: add timeout, to not kill on_task_instance_success listener by heartbeat_callback local task job: add overtime mechanism, to not kill task prematurely when auxiliary processes are running Jun 4, 2024
@mobuchowski mobuchowski force-pushed the listener-task-timeout branch from e2acd51 to ec1b194 Compare June 5, 2024 21:25
@mobuchowski mobuchowski mentioned this pull request Jun 5, 2024
Merged
@mobuchowski mobuchowski force-pushed the listener-task-timeout branch 2 times, most recently from f0128ce to 3d4661d Compare June 10, 2024 14:40
@potiuk
Copy link
Member

potiuk commented Jun 11, 2024

Nice and simple. I think we should have another committer to take a look as well.

jscheffl
jscheffl approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Code-wise looks good.

@mobuchowski mobuchowski force-pushed the listener-task-timeout branch from 3d4661d to 7b0404b Compare June 12, 2024 14:53
@mobuchowski mobuchowski merged commit fa65a20 into main Jun 13, 2024
51 checks passed
@eladkal eladkal mentioned this pull request Jun 22, 2024
Closed
96 tasks
@eladkal eladkal deleted the listener-task-timeout branch June 29, 2024 17:32
@ephraimbuddy ephraimbuddy added this to the Airflow 2.10.0 milestone Jul 1, 2024
@ephraimbuddy ephraimbuddy added the type:improvement Changelog: Improvements label Jul 1, 2024
@ephraimbuddy ephraimbuddy added type:bug-fix Changelog: Bug Fixes and removed type:improvement Changelog: Improvements labels Jul 1, 2024
@potiuk potiuk mentioned this pull request Jul 2, 2024
Closed
20 tasks
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
@mobuchowski @romsharon98
local task job: add timeout, to not kill on_task_instance_success lis…
97ea16e 
…tener prematurely (apache#39890)

Signed-off-by: Maciej Obuchowski <[email protected]>
@kaxil kaxil mentioned this pull request Nov 30, 2024
Closed
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
48ca10c 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
@kaxil kaxil mentioned this pull request Dec 3, 2024
Merged
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
72ed26f 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
f0ce4de 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
da50ce0 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
7864d5c 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
1d0afab 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
4be85f7 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
752e4f3 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit to astronomer/airflow that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor
5b8fd67 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
kaxil added a commit that referenced this pull request Dec 3, 2024
@kaxil
AIP-72: Port task success overtime to the Supervisor (#44590)
d059d4a 
This PR ports the overtime feature on `LocalTaskJob` (added in #39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes #44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
@kaxil
AIP-72: Port task success overtime to the Supervisor (apache#44590)
3cc1994 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
@kaxil @got686-yandex
AIP-72: Port task success overtime to the Supervisor (apache#44590)
5b6f468 
This PR ports the overtime feature on `LocalTaskJob` (added in apache#39890) to the Supervisor.
It allows to terminate Task process to terminate when it exceeding the configured success overtime threshold which is useful when we add Listenener to the Task process.

closes apache#44356

Also added `TaskState` to update state and send end_date from task process to the supervisor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk approved these changes

@josh-fell josh-fell josh-fell left review comments

@kacpermuda kacpermuda kacpermuda left review comments

@jscheffl jscheffl jscheffl 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:providers area:Scheduler including HA (high availability) scheduler provider:openlineage AIP-53 type:bug-fix Changelog: Bug Fixes
Projects
None yet
Milestone
Airflow 2.10.0
Development

Successfully merging this pull request may close these issues.
6 participants
@mobuchowski @potiuk @josh-fell @jscheffl @kacpermuda @ephraimbuddy