PostHog / PostHog/posthog

cdp: rerun over a historical window mishandles already-replayed invocations

Open
#85,285 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
39.9k
Forks
3.4k
Avg merge
6h 51m
Merged PRs (30d)
232

Description

Two findings from review on #85156, deliberately deferred there to keep that PR out of core rerun logic. Both stem from the same property: replay lifecycle rows land in the ClickHouse partition of their own scheduled_at, outside a historical rerun window, while the paginator reads several values from the window-bounded collapse.

1. Stale skips exhaust the cap

Skipped invocations count against max_count (remainingBudget = cap - queued - skipped), and a new rerun job restarts at the top of the window with the same DESC ordering. On a workflow with more matching failures than the cap: run 1 queues the newest cap-worth and stops; run 2 walks those same rows, finds them stale (already succeeded), charges them all against the cap, and finishes with queued=0. Every later run repeats this, so the leftover below the cap boundary is unreachable.

Sketch of a fix: charge only queued invocations against the cap; make isDone compare the budget to the page's queued count rather than its processed count.

2. Rehydration reuses the in-window snapshot

The cross-partition stale check added in #85156 validates only status. An invocation that a prior replay advanced and then failed again keeps latest status failed, so it correctly reruns - but invocation_globals, attempts, parent_run_id, and first_scheduled_at all come from the window-bounded argMax, i.e. the pre-replay state. For hog flows the stored currentAction is the resume point, so resuming from the old snapshot re-runs actions that already completed - re-sending emails that already went out. The max_attempts guard also reads the stale (lower) attempts value.

Sketch of a fix: extend the stale-status query into a latest-state fetch (status, is_deleted, attempts, invocation_globals, parent_run_id, first_scheduled_at via argMax without a partition bound) and rehydrate from that instead of the in-window row.

Both have review-comment threads with full analysis on #85156. The integration suite in nodejs/src/cdp/rerun/rerun-paginator.service.test.ts only uses windows that include now, which is why neither shows up there - regression tests for both should seed a succeeded/failed-again row outside the window.

🤖 Generated with Claude Code

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the rerun paginator and nodejs/src/cdp/rerun/rerun-paginator.service.test.ts, then read the linked review analysis on #85156. Add regression coverage using succeeded and failed-again rows outside the rerun window; done means stale rows no longer consume the cap and rehydration uses the latest state, including attempts and invocation globals.

Written by the indexing model from the issue text.

Assessment

Tech stack
clickhouse, node.js, python
Domain
backend, databases, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.