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.

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

Check to see if the job has been running more then 3 hours #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stuartshields
Copy link

@stuartshields stuartshields commented Mar 6, 2024

When a job is stuck (long running jobs) it makes it hard to delete the job when using plugins like WP Crontrol.

This PR is based on a solution that has been tested and is being used on an existing project that allows for us to delete these jobs via WP Control.

Props: @ivankristianto

// Check if the job is running more then 3 hours from the timestamp.
$is_delete_running = ( time() - $job->start ) > 10800;
if ( $is_delete_running ) {
$job->delete( [ 'delete_running' => $is_delete_running ] );

Choose a reason for hiding this comment

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

Rather than being this opinionated in the Cavalcade code itself, could we introduce a filter cavalcade/job-max-runtime which defaults to false, but lets a project opt in to this behavior when set?

Choose a reason for hiding this comment

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

The issue when I try to fix this on another project was, if the job is running status and stalled, it cannot be deleted from wp_unschedule_event triggered from cron delete from cli.

Although I agree on the max time out is up to project to decide.

@rmccue
Copy link
Member

rmccue commented Mar 6, 2024

I don't want to make specific assumptions in Cavalcade itself about the run time of jobs or similar, as it's designed to be generic infrastructure; a 3 hour job could be normal in the context of a particular site.

From what I see here, the issue is that Crontrol doesn't have any ability to "force" deletion, which kind of makes sense there. If a job is actually currently running, what does it mean to cancel it? Would we need to kill the process? (And what implications does that have for the partially-completed job?)

Ideally, if we could lift this functionality up, that seems like the best option; i.e. add into the Crontrol UI the distinction. This would allow per-project discretion, while also making it clear to the user what the implications are.

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.

4 participants