Skip to content

fix(cron): release scheduler lock before running jobs - #3764

Closed
Sertug17 wants to merge 1 commit into
NousResearch:mainfrom
Sertug17:fix/cron-scheduler-blocking
Closed

Sertug17 wants to merge 1 commit into
NousResearch:mainfrom
Sertug17:fix/cron-scheduler-blocking

Conversation

@Sertug17

Copy link
Copy Markdown

What does this PR do?

Long-running cron jobs were blocking the entire scheduler tick loop because run_job() executed while the tick lock was still held. One slow job could delay all unrelated due jobs and block the next 60-second gateway tick — effectively causing scheduler starvation.

This PR scopes the file lock to due-job selection only, then releases it before executing jobs. Each job now runs outside the global tick lock.

Related Issue

Fixes #3752

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • cron/scheduler.py: Moved run_job() loop outside the tick lock scope. Lock is released immediately after get_due_jobs() completes.

How to Test

  1. Create two cron jobs — one slow (long LLM task), one fast
  2. Schedule both for the same tick
  3. Verify fast job is not blocked by slow job
  4. Verify next 60-second tick starts on time

Checklist

  • I've read the Contributing Guide
  • Commit follows Conventional Commits
  • No duplicate PRs
  • Tested on Ubuntu 24.04 / WSL2

Long-running cron jobs were blocking the entire scheduler tick loop
because run_job() executed while the tick lock was still held.

Scope the lock to due-job selection only, then release it before
executing jobs. This prevents slow jobs from delaying unrelated
due jobs and blocking the next 60-second gateway tick.

Fixes NousResearch#3752
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for identifying the lock starvation issue @Sertug17! After reviewing, we think the current approach is safe enough — advance_next_run() (added recently on main) already prevents double-execution by advancing next_run_at before run_job() starts, and the file lock is non-blocking (LOCK_NB) so concurrent ticks skip gracefully rather than queue up.

Moving the job loop outside the lock would introduce a race window between get_due_jobs() and advance_next_run() that would need careful handling. The current design trades some tick delay for simplicity and correctness, which we're comfortable with for now.

Appreciate you thinking about scheduler robustness though — this is a real concern for heavy cron users.

@teknium1 teknium1 closed this Mar 30, 2026
@Sertug17

Copy link
Copy Markdown
Author

Thanks for the detailed explanation @teknium1!

Makes sense — if advance_next_run() already guards against double-execution before run_job() starts, the race window concern is valid. Good to know the LOCK_NB design is intentional.

I'll close this PR. Appreciate you taking the time to review!

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.

Long-running cron jobs block the scheduler tick loop

2 participants