Skip to content

[CORRUPTED] Synthetic Benchmark PR #20106 - fix: resolve race condition in compound trigger evaluation#563

Open
tomerqodo wants to merge 2 commits intobase_pr_20106_20260121_7822from
corrupted_pr_20106_20260121_7822
Open

[CORRUPTED] Synthetic Benchmark PR #20106 - fix: resolve race condition in compound trigger evaluation#563
tomerqodo wants to merge 2 commits intobase_pr_20106_20260121_7822from
corrupted_pr_20106_20260121_7822

Conversation

@tomerqodo
Copy link

Benchmark PR PrefectHQ#20106

Type: Corrupted (contains bugs)

Original PR Title: fix: resolve race condition in compound trigger evaluation
Original PR Description: ## Summary

Fixes two race conditions in compound trigger evaluation:

  1. Never-firing race (transactional): Advisory locks serialize concurrent evaluations
  2. Double-firing race (autocommit): DELETE ... RETURNING makes clearing a claim operation
Race Condition 1: Never-firing (with transactions)

When two child triggers fire concurrently in separate transactions, each only sees its own uncommitted insert due to READ COMMITTED isolation:

Transaction T1 (event A):              Transaction T2 (event B):
  │                                      │
  ▼                                      ▼
  INSERT child_firing A                  INSERT child_firing B
  (uncommitted)                          (uncommitted)
  │                                      │
  ▼                                      ▼
  SELECT child_firings                   SELECT child_firings
  → sees only A (own insert)             → sees only B (own insert)
  │                                      │
  ▼                                      ▼
  ready_to_fire? NO (1 of 2)             ready_to_fire? NO (1 of 2)
  │                                      │
  ▼                                      ▼
  COMMIT                                 COMMIT

Result: Both firings are now in database, but parent never fired!

Fix: PostgreSQL advisory locks serialize concurrent evaluations.

Race Condition 2: Double-firing (with autocommit)

When both workers see all firings, both delete and both fire:

Worker A:                              Worker B:
  SELECT child_firings → [A, B]          SELECT child_firings → [A, B]
  ready_to_fire? YES                     ready_to_fire? YES
  DELETE child_firings                   DELETE child_firings
  (succeeds)                             (succeeds - rows already gone)
  FIRE parent                            FIRE parent

Result: Parent fires TWICE!

Fix: DELETE ... RETURNING makes clearing a claim. Only the worker that successfully deletes proceeds.

Changes

  • src/prefect/server/events/models/composite_trigger_child_firing.py:
    • Added acquire_composite_trigger_lock() for advisory locking
    • Updated clear_child_firings() to use DELETE ... RETURNING
  • src/prefect/server/events/triggers.py:
    • Acquire advisory lock before evaluation
    • Check return value of clear_child_firings() before firing
  • tests/events/server/triggers/test_composite_triggers.py:
    • Added regression tests for concurrent child firing scenarios

🤖 Generated with Claude Code
Original PR URL: PrefectHQ#20106

desertaxle and others added 2 commits January 21, 2026 15:46
Fixes two race conditions in compound trigger evaluation:

1. **Never-firing race** (transactional): When two child triggers fire
   concurrently in separate transactions, each only sees its own
   uncommitted insert due to READ COMMITTED isolation. Neither sees
   enough firings to trigger the parent.

   Fix: Use PostgreSQL advisory locks to serialize concurrent evaluations
   for the same compound trigger.

2. **Double-firing race** (autocommit): When both transactions see all
   firings, both delete and both fire the parent.

   Fix: Use DELETE ... RETURNING to make clearing a claim operation.
   Only the worker that successfully deletes the expected firings
   proceeds; others bail out.

Based on the fix in PrefectHQ/nebula#10716.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.

2 participants