Skip to content

Add pilot hooks for job deletion - #1500

Merged
bgentry merged 1 commit into
masterfrom
bg/pilot-job-delete-hooks
Oct 13, 2026
Merged

bgentry merged 1 commit into
masterfrom
bg/pilot-job-delete-hooks

Conversation

@bgentry

@bgentry bgentry commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Client.JobDelete, Client.JobDeleteMany, their Tx variants, and the job cleaner call the driver executor directly, so unlike cancels, retries, and completions, deletes never go through the pilot.

This adds an optional PilotJobDeleter interface with JobDelete, JobDeleteMany, and JobDeleteBefore (the job cleaner's batch delete). Like JobCancel and JobRetry, each hook receives the executor and driver params and performs the delete itself, so a pilot can substitute its own delete statement or run additional writes in the same transaction, using dbutil.WithTxV to reuse the caller's transaction when there is one. The job cleaner now receives the client's pilot through its config.

The interface is temporarily kept separate from Pilot, following the PilotJobRescuer pattern, so existing pilot implementations still compile. It isn't meant to stay optional: once supported implementations have adopted it, it gets embedded into Pilot and the executor fallbacks are removed, as the doc comment says. The client and cleaner use the hooks when the pilot implements them and fall back to the executor otherwise. StandardPilot implements them as plain executor calls, so default behavior is unchanged.

@bgentry
bgentry force-pushed the bg/pilot-job-delete-hooks branch 2 times, most recently from f89d12b to a588258 Compare October 13, 2026 02:22
`Client.JobDelete`, `Client.JobDeleteMany`, their `Tx` variants, and the
job cleaner's `JobDeleteBefore` call the driver executor directly, so a
pilot can't take part in deletes the way it can in cancels, retries, and
completions.

Add an optional `PilotJobDeleter` interface with `JobDelete`,
`JobDeleteMany`, and `JobDeleteBefore`. Each hook receives the executor
and the driver params and performs the delete itself, like `JobCancel`
and `JobRetry`, so a pilot can substitute its own delete statement or
run additional writes in the same transaction. The job cleaner now
takes the client's pilot in its config.

The interface is temporarily kept separate from `Pilot`, following the
`PilotJobRescuer` pattern, so existing pilot implementations still
compile; it's meant to be embedded into `Pilot`, and the fallbacks
removed, once supported implementations have adopted it. The client and job cleaner use the hooks when the pilot
implements them and otherwise fall back to the executor.
`StandardPilot` implements them by calling the executor, so behavior is
unchanged by default.

Tests cover the client and cleaner routing deletes through the pilot,
the executor fallback for pilots without the new interface, and the
standard pilot's pass-through.
@bgentry
bgentry force-pushed the bg/pilot-job-delete-hooks branch from a588258 to 15f10b0 Compare October 13, 2026 02:25

@brandur brandur 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 trying to keep it compatible with the existing versions. The last two pilot changes were breaking, and it really reminded me as I was cutting releases that we need to stop doing that. If any pro user tries to upgrade after we release River but before we release River Pro, they'll get an error, and the full pro release still takes substantial time so that window is like an hour.

@bgentry
bgentry marked this pull request as ready for review October 13, 2026 02:29
@bgentry
bgentry merged commit 3e6c2d5 into master Oct 13, 2026
23 checks passed
@bgentry
bgentry deleted the bg/pilot-job-delete-hooks branch October 13, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants