Add reconcile loop scheduler #69
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: yolo/arr#69
Reference in New Issue
Block a user
Delete Branch "issue/21-reconcile-loop"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Issue
Closes #21
Test plan
just ci(153 tests passed)Reviewed
a292f2f. Four anchored findings.Nothing outside the tests calls
register(), so the loop the daemon actually starts has no actions in it and every tick is a no-op. That reads as deliberate for a scheduler-only issue — flagging it only so it isn't mistaken for wired-up reconciliation.CI is still pending on this SHA; I didn't run anything from the branch.
@@ -119,0 +128,4 @@.await;let _ = shutdown_tx.send(true);reconcile_task.await?;A panic inside any action aborts the whole spawned task, taking all four lanes with it. Nothing observes that:
axum::servekeeps running, no log is emitted, and theJoinErroronly surfaces here — after shutdown. The daemon then serves HTTP for hours with reconciliation dead and looks healthy.Either catch per-action panics in
run_tick(AssertUnwindSafe(...).catch_unwind(), log, count as a failure), or select overreconcile_taskalongside serve so the process exits when the loop dies.@@ -0,0 +184,4 @@if *shutdown.borrow() {return;}self.run_tick(tick).await;No timeout around the action call. A reconciler blocked on Transmission or Prowlarr with no read timeout stalls its lane indefinitely, and because
maindoesreconcile_task.awaitwith no bound, SIGTERM never completes — systemd ends up killing the process on its stop timeout.Wrap in
tokio::time::timeout(something under the lane interval) and count an expiry as a failure.@@ -0,0 +204,4 @@action = %outcome.action,"reconcile action");report.actions_taken += 1;gaps_foundandactions_takenare incremented in the same loop over the sameVec, so they are equal by construction and the log line reports one number twice. Either dropgaps_found, or letActionreport gaps it found but chose not to act on (blocked, cooling off, no eligible release) so the two can diverge.@@ -0,0 +225,4 @@gaps_found = report.gaps_found,actions_taken = report.actions_taken,failures = report.failures,"reconcile tick"This fires on every tick regardless of outcome, so an idle instance writes ~2880 all-zero lines a day from the reconcile lane alone, plus the other three. Log at info only when
gaps_found + failures > 0, and drop the quiet case to debug.Reviewed
d8455df. All four earlier findings are addressed — the loop is supervised, the duplicate metric is gone, actions are bounded, and idle ticks dropped to debug. One new finding on the timeout constant.@@ -0,0 +20,4 @@const METADATA_START_DELAY: Duration = Duration::from_secs(5);const REAPER_START_DELAY: Duration = Duration::from_secs(10);const RSS_START_DELAY: Duration = Duration::from_secs(20);const ACTION_TIMEOUT: Duration = Duration::from_secs(25);One 25 s timeout covers every lane. It fits reconcile (30 s interval), but the metadata lane runs daily and a full TMDB refresh over the library is minutes of work, so that lane will time out on every tick and never complete once there is real data behind it — logged as a failure with no partial progress.
Make the timeout per-lane, e.g. derived from the lane interval (
interval - slack, or an explicit value perTick), rather than one constant.Reviewed
76e398d. No findings. Per-lane timeouts address the last one, and racingrun_tickagainst shutdown is a genuine improvement over waiting out an in-flight action.