Repository navigation
stats: the snapshot counts its fetch series in full after any migration - #636
Conversation
Only a migration deletes receipts, and its schema_migrations row commits with the deletion. The delete counter alone could miss one for the first second or so after the commit: statistics reach other sessions late (Codex on #627). The counter stays as a guard for any other deletion. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9458271475
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rows: [now], | ||
| } = await db.query( | ||
| `select coalesce((select max(receipt_id) from api_receipt), 0)::text as through, | ||
| (select coalesce(max(id), 0) from schema_migrations)::text as migrated, |
There was a problem hiding this comment.
When a migration commits after this initial SELECT but before the later receipt queries, PostgreSQL's default READ COMMITTED isolation lets those queries see the deletion even though now.migrated still contains the old ledger value. The refresh can therefore choose the incremental path, preserve the deleted receipt's earlier day, and store the old migration marker; this leaves the published snapshot stale until the post-deploy or next hourly refresh. Re-read the ledger before returning and retry with a full count if it changed, or perform the refresh against one consistent transaction snapshot. Migrations run separately in the migrate Lambda during deployment, so this overlap is possible.
AGENTS.md reference: AGENTS.md:L107-L110
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right. Under read committed, a migration that commits after the ledger read was visible to the recount but not to the guard. Fixed in #638.
The refresh now runs every read in one repeatable read read only transaction, and the snapshot's instant is the published as_of. A migration that lands during the refresh is invisible to it, ledger row and deletion together. The payload is the record exactly as of as_of, and the next refresh sees the ledger move and counts in full. The write never replaces a newer stored snapshot with an older one.
The new test commits a receipt-deleting migration from a second connection right after the ledger read. It requires a payload equal to the record before the migration, an as_of before the commit, and a full count on the next refresh. The test fails under read committed.
Under read committed a migration could commit between the refresh's ledger read and its recount, so the payload mixed two instants under an as_of later than both (Codex on #636). The refresh now reads in one repeatable read, read only transaction whose instant is the as_of; a migration during it is seen whole by the next refresh. The write never replaces a newer snapshot with an older one. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 participant
Codex flagged a gap on #627. The hourly stats snapshot recounted its fetch series in full only when
api_receipt's delete counter (pg_stat_user_tables.n_tup_del) had moved. Statistics reach other sessions a second or more after the commit, so a refresh just after a deletion could keep the deleted receipt in an earlier day until the next hour.schema_migrationsrow commits in the same transaction, so a refresh that can see the deletion also sees the ledger move.services/jobs/test/public-stats.test.mjssets the stored counter to the current value, as a refresh in that window would see it, and requires a full count. The test fails without the guard.No migration, contract or JSON API change. No acceptance family: no tool reads the snapshot.
🤖 Generated with Claude Code