Skip to content

Make Java SDK context signal-aware - #74690

Open
FrankYang0529 wants to merge 1 commit into
apache:mainfrom
FrankYang0529:java-sdk-signal-aware-context
Open

FrankYang0529 wants to merge 1 commit into
apache:mainfrom
FrankYang0529:java-sdk-signal-aware-context

Conversation

@FrankYang0529

@FrankYang0529 FrankYang0529 commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Why

  • When Airflow wants a running task to stop, for example after a user marks the task instance as failed, the supervisor sends the task's process SIGTERM. If the process is still running after [core] killed_task_cleanup_time seconds, the supervisor sends SIGKILL. The Go and TypeScript SDKs pass that SIGTERM to the task through its context, so the task can stop work it started, such as a job in an external system.

How

  • While a task runs, the SDK replaces the JVM's SIGINT and SIGTERM handlers through sun.misc.Signal. It puts the old handlers back after the supervisor has acknowledged the task's result.
  • Context.isCancelled() and Context.awaitCancellation(Duration) tell a task that Airflow asked it to stop.

closes: #72680

Was generative AI tooling used to co-author this PR?
  • Yes - Claude Code

Signed-off-by: PoAn Yang <payang@apache.org>

@fat-catTW fat-catTW left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR!
lgtm

@jason810496 jason810496 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR, LGTM overall.

/** Registration of the executing task; resolves wired data inputs. */
internal var taskDef: TaskDef? = null

internal var cancellation = Cancellation()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about constructing the Cancellation() once instead of constructing in several places?
Here're more context that I ask agent to double check:


Only the instance in runTaskAndReport (Server.kt) is connected to the signal trap. The others are defaults: this field, Context.from (line 174), TaskRunner.runTask (Task.kt line 101, never used since both wrappers pass it), and runTask(bundle, request, client) (Task.kt line 170, only TaskTest relies on it).

If a future caller forgets to pass the server's instance, it gets an unconnected one with no error. isCancelled() then marks the wrong instance as watched, and SIGTERM goes to the JVM.

Could we create it once in runTaskAndReport and make the parameter required in Context.from and both runTask functions? Tests can pass Cancellation() explicitly. This field's initializer also allocates an instance that Context.from replaces right away.

* handles the signal as it would without the SDK. By default the JVM runs its
* shutdown hooks and exits.
*
* A context made with [copy] does not see the stop request.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it better to block the copy method at all or the share the same Cancellation instance for the Context?


Since is a public data class, copy() is easy to reach for, and the copy gets its own Cancellation that the signal trap never sees. If a task checks only through the copy, the real instance is never marked as watched, so SIGTERM goes to the JVM. Is documenting it enough, or should the copy share the original instance?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Java SDK context signal-aware

3 participants