Skip to content

Gracefully fail claimed executions even if the supervisor process was pruned #559

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

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

Conversation

jeremy
Copy link
Member

@jeremy jeremy commented May 7, 2025

e.g. due to sleep/wake in a dev app

Fixes #430

@jeremy jeremy requested a review from rosa May 7, 2025 16:41
Copy link
Member Author

@jeremy jeremy left a comment

Choose a reason for hiding this comment

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

Not happy with this fix since 1. it may be a work-around masking that supervisor process pruning should be resilient to sleep/resume interference and 2. the test case is synthetic at best, but perhaps it's a useful starting point.

def wait_for_failed_executions(count, timeout: 1.second)
wait_for(timeout: timeout) { SolidQueue::FailedExecution.count == count }
end
end
Copy link
Member Author

Choose a reason for hiding this comment

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

This test isn't strictly necessary. I was using it to guide debugging and understanding.


supervisor = SolidQueue::Supervisor.allocate

supervisor.send(:handle_claimed_jobs_by, terminated_fork, status)
Copy link
Member Author

Choose a reason for hiding this comment

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

This test is terribly synthetic, I'm afraid. Just explicitly constructs the situation that's encountered in the wild (nil Supervisor#process) rather than realistically (actually pruning the process).

@jeremy jeremy force-pushed the supervisor-sleep-wake-in-dev branch from 85a32c2 to 055cb3a Compare May 7, 2025 16:47
@jeremy jeremy force-pushed the supervisor-sleep-wake-in-dev branch from 055cb3a to 849dcfd Compare May 7, 2025 16:48
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.

undefined method superviseesfor nil
1 participant