Skip to content

Concurrent "Retry failed" presses re-enqueue the same executions twice #298

Description

@antosubash

RetryCoordinator.retry_failed (modules/background_tasks/background_tasks/retry_service.py:54-121)
selects the eligible rows, publishes each to the broker, then inserts the child rows that make them
ineligible. Nothing holds a lock across those three steps.

Two operators pressing "Retry failed" at the same time — or one operator double-clicking — each run
select(TaskExecution).where(*conditions) (retry_service.py:92-104) before either has flushed, so
both see the same batch and both call send_task on every row
(retry_service.py:107, :126-134). The guard that is supposed to prevent this is
bulk_retry_conditions' "parents that already have a child are excluded", and the child does not
exist until flush() on line 116. The result is every task in the sweep executed twice, with two
TaskExecution chains pointing at the same parent.

What would fix it: take the rows with .with_for_update(skip_locked=True) so a second sweep skips
what the first is holding, and insert the child rows before publishing to the broker rather than
after — an orphaned pending row that was never sent is a far cheaper failure than a task that ran
twice.

Smaller things noticed in the same area:

  • Events for the whole sweep are published in one loop after the batch flush
    (retry_service.py:118-120), and before the request commits — so subscribers see nothing while
    a 500-row sweep runs, then 500 events at once, for a transaction that may still roll back.
    Same missing hook as Request-scoped session exposes no post-commit hook, so module caches refresh against uncommitted state #268.
  • The API and the view disagree on the filter's name: GET /api/background-tasks/executions
    takes task_name (modules/background_tasks/background_tasks/endpoints/api_admin.py:37) while
    the page takes it as q
    (modules/background_tasks/background_tasks/endpoints/views.py:54). A URL copied from one does
    not filter the other.
  • The strip mixes units: succeeded_24h is a windowed count
    (views.py:75-79) sitting beside unwindowed status totals in the same row of tiles
    (views.py:94-98), so "Succeeded 24h" can read lower than the table's total for the same
    filter with nothing on screen explaining why.

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 working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions