split VortaScheduler into state and scheduling components (#2360, goal 2) - #2558
Merged
m3nu merged 1 commit intoSep 14, 2026
Merged
Conversation
Contributor
Author
…lding the lock across job submission
m3nu
force-pushed
the
refactor/scheduler-state-scheduling
branch
from
September 14, 2026 17:06
26dff3d to
6fad94a
Compare
m3nu
approved these changes
Sep 14, 2026
m3nu
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the split and the lock change against current master.
- Re-exports cover every external import site (
application.py,schedule_page.py,jobs_table_model.py, tests);packages.findpicks up the new subpackage, so nothing to do for packaging. - The lock change is right.
create_backupis only ever entered from the GUI thread (timer callbacks and the catch-up path), solocknever gave it cross-thread protection — it only enabled the self-deadlock in ntninja's trace on #2367 (prepare()→ pre-commandprocessEvents()→repo_changed→set_timer_for_profile→lock.acquire). Returning the catch-up profile out ofarm_profileand guarding re-entry with_submittingcloses that, and every catch-up path still re-arms viabackup_finished_eventorpause→mark_paused. - The three new tests pin exactly those paths.
I rebased the branch onto master after squashing #2538 (only your commit is left, 6fad94af); CI is green on it. The property shims for _net_up / lock / pauses can go in the Execution PR once the tests are moved to the new seams.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
VortaSchedulerbecomes a package. This PR takes two of the three components goal 2 asks for: State (pauses, recorded outcomes, resume detection) and Scheduling (next run times, timers, network gating). Execution stays on the facade and moves in the next PR.src/vorta/scheduler.pywas 743 lines. It is nowsrc/vorta/scheduler/scheduler/__init__.pyVortaScheduler: both signals,tr(), the Qt slots,create_backup/notify/post_backup_tasks, delegation, and re-exportsscheduler/scheduling.pySchedulerTimers:timers,lock,arm_deadline_timerand its constants,arm_profile,reload_all_timers,next_job,next_job_for_profile,pending_jobs,mark_paused,remove_job, network gating, the 15 minute reschedule timerscheduler/state.pySchedulerState:pause/unpause/clear_pause/paused/restore_pauses,record_skip, and resume detectionEvery method moves as it is, apart from the lock ownership change described below. Nothing touches the schema.
The facade stays the
QObject. It keeps both signals, sinceschedule_page.pyandjobs_page.pyconnect on the scheduler object, and it keepstr()so the translation scope is stillVortaScheduler. The Qt slots stay on it as well and delegate one line inward, because QDBus wants aQObjectreceiver for the logind connection.The public surface is unchanged.
timers,pauses,lock,wake_timer,net_statusand_net_upare still reachable as properties, which keeps the test diff down to the seam. Otherwise it would be thirty renamed attribute accesses.What I left out:
create_backup,notify,post_backup_tasks, the atomic submit, therunning/completed/interruptedtransitions and theevent_loglink.Status.RUNNINGyet, so a sweep finds nothing and its test has to fabricate a row no production path creates. I would rather it landed with the lifecycle transitions that give it something to find.arm_profile's body and the twoEventLogModellast-run queries move as they are.Related Issue
#2360, goal 2. Cut from #2538, so that one has to merge first. I will rebase after that.
Likely also fixes #2533, see below.
Motivation and Context
The issue asks for the 743 line
VortaSchedulerto be split into Scheduling, Execution and State. This takes the first two.It also carries the one behaviour change in the PR, which is lock ownership. Today
set_timer_for_profileholdsself.lockacross its whole body. At the catch-up branch it callslock.release(), thencreate_backup(...), then reacquires in afinally, andcreate_backuptakes the same non-reentrant lock again.After this PR:
SchedulerTimers.arm_profilereturns the profile that has a missed run to catch up on, instead of submitting it inline. The facade submits once the lock is free.release()/acquire()pair is gone. It was only there to survive that inline call.create_backupdrops the lock and keeps a_submittingset of profile ids, returning early on re-entry.That guard does real work.
BorgCreateJob.prepare()reaches the keyring atborg_job.py:159, which can spin a nested event loop through KWallet's dialog, and nothing marks the profile busy untiladd_jobruns afterprepare()returns. Under the old lock, a 15 minuteqt_timertick landing in that window froze the GUI thread. Drop the lock and leave it at that, and you get a secondBorgCreateJobqueued for the same repo. The set turns both cases into an early return.I hit that freeze while working on this split, and going back through the tracker afterwards it looks like #2533 is the same bug.
lockis now taken only inarm_profile. It still serialises timer arming, and it no longer excludes submission. Narrowing it further belongs with the atomic submit in the Execution PR.How Has This Been Tested?
Unit tests,but Unit tests do not cover the deadlock, so I checked it by hand on Linux. An interval schedule with a last run far enough back to trigger catch-up starts the backup and re-arms instead of freezing. A forced failure pauses the profile, the Schedule page shows "Paused until X", and the Jobs page shows the paused row with the skip record. The pause survives a restart from
SchedulerPauseModel.Types of changes
Checklist: