Repository navigation
fix(cohorts): stop server-side cursor scan in dependency cache warm - #75972
trunk-io[bot] merged 3 commits into
Conversation
warm_team_cohort_dependency_cache() used .iterator(), which opens a named PostgreSQL server-side cursor and holds it open across per-cohort cache round-trips. If the connection is recycled in that window, psycopg raises InvalidCursorName, which escapes the synchronous transaction.on_commit callback fired by cohort creation/update and surfaces as a 500. Replaced with keyset pagination on id so no cursor is held open between batches. Generated-By: PostHog Code Task-Id: 0e296c94-4ea3-4fad-ac3c-4685d393defc
|
😎 Merged successfully - details. |
github-actions
Bot
added
feature/cohorts
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
There was a problem hiding this comment.
Pull request overview
This PR updates cohort dependency cache warming to avoid holding a PostgreSQL server-side cursor open across cache/ORM work, preventing intermittent InvalidCursorName errors when connections are recycled.
Changes:
- Replaced
QuerySet.iterator(chunk_size=...)scan with keyset pagination (id__gt last_id, ordered byid) inwarm_team_cohort_dependency_cache. - Expanded the function docstring to document the rationale for avoiding
.iterator()here.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
🤖 CI report
|
…r-side-cursor-scan-deac49
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Small, contained fix that swaps a server-side cursor iterator for keyset pagination with identical behavior; not risky territory, and a human reviewer already approved with no unresolved concerns.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 28L, 1F substantive — within ceiling |
| tier | ✓ | T1-agent / T1b-small (28L, 1F, single-area, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 04b6060 · reviewed head 04b6060 |
🦔 PostHog Review reviewed this pull requestNothing worth raising this time, so here's a calming picture instead:
|
Problem
cohort_changed'spost_savehandler firestransaction.on_commit(...)synchronously, and cohort creation isn't wrapped in an outeratomic()block, so the callback runs inline with the request.warm_team_cohort_dependency_cache, which scans the team's cohorts with.iterator()— opening a named PostgreSQL server-side cursor — then does cache round-trips and more ORM work betweenfetchmanybatches.InvalidCursorName: cursor "..." does not exist, and nothing catches it, so it escapesperform_createas a 500.Changes
.iterator(chunk_size=batch_size)inwarm_team_cohort_dependency_cachewith keyset pagination onid, so no server-side cursor is held open across the per-cohort cache writes.How did you test this code?
hogli testerrors trying to resolve hostdb), so I could not run the existing test suite (products/cohorts/backend/models/test/test_dependencies.py, which already coverswarm_team_cohort_dependency_cacheincluding batching behavior).ruff check/ruff formatandmypyon the changed file; both pass clean.test_warm_team_cohort_dependency_cache_batchingtests already exercise the batched-scan behavior end-to-end (result correctness acrossbatch_sizevalues), and this change preserves that contract while swapping the underlying query strategy — a cursor-recycling failure isn't reproducible in the Django test harness.Automatic notifications
Docs update
N/A — internal implementation detail, no documented behavior changes.
🤖 Agent context
Autonomy: Fully autonomous
I (Claude, via PostHog's coding agent) investigated an inbox report describing intermittent 500s on cohort save caused by
InvalidCursorNameerrors during dependency-cache warming. Traced the failure to.iterator()inwarm_team_cohort_dependency_cache(products/cohorts/backend/models/dependencies.py) holding a named server-side cursor open across cache calls in the synchronouson_commitpath. Fixed by switching to keyset pagination, which addresses the root cause without changing the function's public behavior or moving the warm off the request path (a larger, separate change the report flagged as a nice-to-have, not required to fix the reported symptom).Created with PostHog Desktop from this inbox report.