Skip to content

Scheduler creates duplicate execution records (skipped + success) for single trigger #91

Description

@vybe

Bug Description

A single schedule trigger produces two execution records — one with status success and one with status skipped. This is visible in the agent's execution history and is confusing since there is only one schedule configured.

Screenshot

Root Cause

The scheduler has two independent duplicate-prevention mechanisms that both fire for the same condition:

  1. Distributed Redis lock (src/scheduler/service.py:524-527) — prevents cross-instance duplicates by returning silently if the lock is already held
  2. APScheduler max_instances=1 event (src/scheduler/service.py:118-123) — fires EVENT_JOB_MAX_INSTANCES when a job is still running at the next internal check

The APScheduler event handler (_on_job_max_instances at line 407) calls _record_skipped_agent_schedule() which creates a "skipped" execution record — even though the distributed lock already handled concurrency and the "running" execution is the same trigger, not a genuinely separate one.

Timeline

T0  Cron fires → _execute_schedule() starts
T1  Lock acquired → create_execution(status="running")
T2  Agent work begins (takes minutes)
T3  APScheduler internal check sees job still running
T4  EVENT_JOB_MAX_INSTANCES fires → create_skipped_execution()  ← spurious record
T5  Agent work completes → update status to "success"

Key Files

File Lines Role
src/scheduler/service.py 516-533 _execute_schedule() — acquires lock, runs task
src/scheduler/service.py 407-430 _on_job_max_instances() — creates spurious "skipped" record
src/scheduler/service.py 431-469 _record_skipped_agent_schedule() — writes skipped execution to DB
src/scheduler/service.py 558-564 _execute_schedule_with_lock() — creates the real execution record
src/scheduler/database.py 237-297 create_skipped_execution()

Proposed Fix

Either:

  1. Remove the EVENT_JOB_MAX_INSTANCES listener entirely — the distributed lock already handles concurrency, making the APScheduler event redundant
  2. Add dedup logic in _on_job_max_instances() — check if there is already a "running" execution for this schedule from the current trigger window before creating a "skipped" record
  3. Suppress the event by setting max_instances higher and relying solely on the distributed lock for concurrency control

Option 1 is the simplest and eliminates the race condition entirely. The "skipped" record was presumably added for observability, but it creates more confusion than value when it fires spuriously.

Impact

  • User confusion: Execution history shows duplicate entries for a single trigger
  • Incorrect metrics: Skipped count is inflated
  • No data loss or functional impact: The actual task executes correctly once

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcomplexity-lowComplexity: low (board points 1-3)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions