fix(docker): always start the RQ scheduler - #4033
Conversation
Greptile SummaryThis PR starts the production RQ scheduler unconditionally and changes scan concurrency discovery to ignore scheduler entries more than 15 minutes overdue. Confidence Score: 4/5This PR should not merge until overdue delayed scans remain protected against later scheduler recovery. A temporary scheduler outage lasting beyond the fixed grace period makes a still-executable delayed scan invisible to both concurrency prevention and cancellation, allowing it to overlap a manual scan or run after a stop request. Files Needing Attention: backend/endpoints/sockets/scan.py Important Files Changed
Prompt To Fix All With AI### Issue 1
backend/endpoints/sockets/scan.py:153
**Overdue scans remain executable**
When the scheduler recovers after a delayed watcher scan is more than 15 minutes overdue, this filter hides the still-executable entry from both the concurrency guard and cancellation path, allowing it to overlap a manual scan or run after a stop request.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(docker): always start the RQ schedul..." | Re-trigger Greptile |
|
Confirmation from the reporter in Discord. Their configuration is: Three scheduler-dependent features enabled, and not one of them starts the scheduler. The condition on So the watcher runs, defers rescans through a scheduler that does not exist, and the entries accumulate until the scan guard refuses every manual scan. That is the reported symptom, reached through a configuration I had not specifically predicted, which I think makes the case for dropping the condition rather than extending it: any fixed list here is one more thing to keep in sync, and this one has already drifted three times. Two further consequences on that same instance, both silent:
That second one is probably worth noting for its own sake: the backfill masking the missing cron is a good part of why this went unnoticed for so long. Worth adding that the workaround is not "enable any scheduled task", which is the natural assumption and the one I gave them first. It has to be one of the four in the condition. On this instance, |
The scheduler only started when one of four ENABLE_SCHEDULED_* flags was true, but nothing else respects that condition. startup.py registers the netplay, upload-tmp and zip-cache cleanups unconditionally, three more flags gate periodic tasks that are absent from the list, and the filesystem watcher defers its rescans through the scheduler as well. Any of those leaves jobs sitting in the scheduler registry with no process to run them. On a default install the three cleanups never fire at all. A watcher running with none of the four listed tasks enabled leaves a delayed scan_platforms entry that nothing will execute, and the concurrent scan guard reads it as a scan already queued, so every manual scan is refused with "A scan is already in progress". Restarting does not help, because the entry is persisted state rather than a running process. Start the scheduler unconditionally, matching entrypoint.sh, which never gated it. A stuck entry then clears on its own, since the scheduler enqueues past-due jobs as soon as it comes up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0681c86 to
0ae753e
Compare
|
Good catch, and correct. I have dropped that part entirely rather than patch around it. It was defence in depth against a scheduler that stalls anyway, and it bought that at the cost of weakening a guard that works. The PR is now just the init script change. Nothing is needed in its place: once the scheduler always starts, a stuck entry resolves itself, because the scheduler enqueues past-due jobs at startup and the worker runs them. Separately, I have corrected the description. I had written that watcher entries accumulate. They do not, |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a Docker startup inconsistency by ensuring the RQ scheduler is always supervised and running in the legacy docker/init_scripts/init path, matching entrypoint.sh behavior. This prevents scheduler-backed features (watcher-delayed rescans and periodic cleanups) from silently never executing due to a drifted, incomplete flag-gated condition.
Changes:
- Start/supervise
rq_schedulerunconditionally in the main watchdog loop. - Remove the brittle multi-flag conditional that had fallen out of sync with actual scheduled-task usage.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The s6 service carried over the four-flag ENABLE_SCHEDULED_* gate from the init script, which #4033 has since removed on master. Since this branch deletes that script, the fix would be lost on rebase. Nothing else respects the condition: startup.py registers the netplay, upload-tmp and zip-cache cleanups unconditionally, three more flags gate periodic tasks absent from the list, and the watcher defers its rescans through the scheduler. Any of those leaves jobs in the scheduler registry with no process to run them, and a stuck delayed scan_platforms entry reads to the concurrent scan guard as a scan already queued, refusing every manual scan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Description
Explain the changes or enhancements you are proposing with this pull request.
The RQ scheduler only starts when one of four
ENABLE_SCHEDULED_*flags is true:Nothing else respects that condition, and the list has fallen behind in three separate ways:
startup.pyregisters three cleanups unconditionally.cleanup_netplay(every 30 min),cleanup_upload_tmp(hourly) andcleanup_zip_cache(daily) are declaredenabled=Truewith cron strings andinit()ed regardless of any flag. On a default install none of them ever run.ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBP,ENABLE_SCHEDULED_RETROACHIEVEMENTS_PROGRESS_SYNCandENABLE_SYNC_PUSH_PULL. Enabling any one of those alone silently does nothing.watcher.py:214,tasks_scheduler.enqueue_in(...)), butENABLE_RESCAN_ON_FILESYSTEM_CHANGEis not in the list either.entrypoint.shstarts the scheduler unconditionally and always has, so the two entry paths already disagree about this.The user-visible failure
Reported in Discord on 5.1.0: clicking Scan is refused immediately and permanently with
🛑 Scan already in progress, ignoring request, with no scan running.With the watcher on and none of the four listed tasks enabled, the watcher runs but the scheduler does not. A filesystem change leaves a delayed
scan_platformsentry in the scheduler registry that nothing will ever execute, and #3974's concurrent scan guard counts it as a queued scan, so every manual scan is refused.Only one entry is needed to cause this. The watcher deduplicates before scheduling (
get_pending_scan_jobs()inwatcher.py, which bails when a full rescan is already pending and skips per-platform slugs that already have one), so entries do not pile up. A single stuck one blocks indefinitely.That matches the report: restarting the container does not help, because it is persisted Redis state rather than a running process; clearing
redis-datadoes help; and the block returns once the next scan finishes, because that scan writes resources, the watcher fires, and a fresh entry is scheduled.Reproduced on a live instance by creating one watcher-style entry:
The reporter's configuration, confirmed after the fact, is
ENABLE_RESCAN_ON_FILESYSTEM_CHANGE,ENABLE_SCHEDULED_CONVERT_IMAGES_TO_WEBPandENABLE_SCHEDULED_RETROACHIEVEMENTS_PROGRESS_SYNCall true. Three scheduler-dependent features enabled, none of which starts the scheduler.The silent variant is visible on any default install. These are the unconditional cleanups on my own instance, all overdue, because no scheduler exists to run them:
Changes
Start the scheduler unconditionally, matching
entrypoint.sh. A hand-maintained boolean chain in bash is what drifted here, and since three periodic tasks are registered regardless of configuration, there is no case left for the condition to express.No cleanup step is needed for instances already stuck: rq-scheduler enqueues past-due jobs as soon as it starts, so the orphaned entry is picked up, run, and gone.
Testing
rqscheduleris absent from the process list with the default configuration.bash -n,shellcheck,shfmt 3.6.0clean.No new tests. The change is one line of the init script, which has no test harness in-repo, and the guard behaviour it feeds is already covered by
TestScanConcurrency.Note for #4019
The s6-overlay rewrite copies the same four-flag condition verbatim into
docker/s6-overlay/s6-rc.d/romm-rq-scheduler/runand callsdisable_service, so it currently reintroduces all of the above. Commented there. Whichever lands second should carry this across.Checklist
Please check all that apply.
AI assistance disclosure
Per
CONTRIBUTING.md: this change was written with AI assistance (Claude Code). The root cause was traced from the Discord report and reproduced on my own instance; the AI wrote the patch, and I reviewed the result.