Skip to content

fix(scheduler): break updated_at feedback loop in sync (#420) - #424

Closed
dolho wants to merge 1 commit into
mainfrom
fix/420-scheduler-sync-loop
Closed

fix(scheduler): break updated_at feedback loop in sync (#420)#424
dolho wants to merge 1 commit into
mainfrom
fix/420-scheduler-sync-loop

Conversation

@dolho

@dolho dolho commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #420.

The scheduler re-registered every enabled schedule once per sync tick (60s), logging ~2 lines per schedule per minute. With 13 schedules in the field that was ~780 INFO lines/hour → ~120 MB/day of log noise, wasted CPU, and a latent hazard: any real scheduler bug gets buried under the flood.

Root cause

Infinite feedback loop between update_schedule_run_times and _sync_agent_schedules:

  1. Sync tick (every 60s) compares DB `updated_at` against in-memory `_schedule_snapshot` to detect config changes.
  2. When `_add_job` runs (either on startup or on a detected change) it calls `db.update_schedule_run_times(schedule.id, next_run_at=...)` at `service.py:374` to persist the computed next-run.
  3. `update_schedule_run_times` in `database.py:193` unconditionally bumped `updated_at = now` as part of a generic UPDATE-builder pattern.
  4. Next sync tick sees `DB.updated_at` ≠ `snapshot.updated_at` → thinks config changed → calls `_add_job` again → bumps `updated_at` again → loop forever.

Fix

`update_schedule_run_times` is now strictly bookkeeping: it updates `last_run_at` / `next_run_at` as requested and never touches `updated_at`. Legitimate config edits (cron, message, timezone, enabled) go through other methods (`update_schedule`, `set_schedule_enabled`) that correctly bump `updated_at` — sync's change detection still catches those.

Secondary: if called with both args `None`, the method now no-ops instead of emitting an invalid `UPDATE SET , WHERE`.

Applied to:

  • `src/scheduler/database.py` (the actual running copy in trinity-scheduler)
  • `src/backend/db/schedules.py` (dead-code backend copy — same bug, fixed for consistency)

Test plan

  • Unit (`tests/unit/test_scheduler_sync_loop.py`): 4 tests — regression guard on `updated_at`, `next_run_at` persistence, `last_run_at` persistence, no-arg no-op. All pass.
  • Live: restarted `trinity-scheduler` with the fix, observed 3 sync cycles (~3 min).
    • Pre-fix: 58 "Added schedule job" lines in 30 min (1 schedule).
    • Post-fix: 1 startup Add, silent for 3 sync cycles.
    • `updated_at` frozen at `2026-04-20T10:33:30` through all 3 ticks.
    • `next_run_at` correctly updated to `2026-04-20T10:39:00` — bookkeeping still works.

Out of scope

  • The scheduler Docker image is baked — production deploy still needs a rebuild to pick up the fix. `docker compose build trinity-scheduler && docker compose up -d trinity-scheduler`.
  • The snapshot-initialization race on startup (snapshot stores old `updated_at`, DB has new post-`_add_job`) is resolved by this fix since the DB value no longer diverges.

🤖 Generated with Claude Code

`update_schedule_run_times` unconditionally bumped `updated_at` to now on
every call, but _sync_agent_schedules uses `updated_at` as its change-
detection signal. Flow: _add_job → update_schedule_run_times (bumps
updated_at) → next sync tick reads mismatched updated_at → thinks config
changed → calls _add_job again → loop forever at the sync interval.

With 13 schedules this produced ~545 re-adds/schedule in 8h, ~120 MB/day of
log noise that grows linearly with fleet size and risks masking real
scheduler bugs.

Fix: `update_schedule_run_times` now updates only last_run_at/next_run_at
as requested, never updated_at. Bookkeeping is not a config change. The
sync loop's change detection still catches legitimate edits (cron, message,
timezone, enabled) because those go through a different write path that
correctly bumps updated_at.

Also fixed the dead-code backend copy in db/schedules.py for consistency.

Live verify: pre-fix 58 "Added schedule job" lines in 30 min with 1
schedule; post-fix 1 startup Add, then silent across 3 sync cycles.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dolho
dolho requested a review from vybe April 20, 2026 11:07
@dolho
dolho force-pushed the fix/420-scheduler-sync-loop branch from a98d651 to f190c92 Compare April 20, 2026 11:07
@dolho

dolho commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #425 (merged in f80b0f1). Same root cause, same fix approach — update_schedule_run_times no longer bumps updated_at. Closing to avoid duplicate work.

@dolho dolho closed this Apr 21, 2026
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.

bug: scheduler re-registers every schedule from DB once per minute

1 participant