Skip to content

fix(fetch): avoid holding DB sessions across GitHub RPCs - #59

Merged
ewhauser merged 4 commits into
mainfrom
codex/fix-observation-pool-starvation
Aug 7, 2026
Merged

fix(fetch): avoid holding DB sessions across GitHub RPCs#59
ewhauser merged 4 commits into
mainfrom
codex/fix-observation-pool-starvation

Conversation

@ewhauser

@ewhauser ewhauser commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace per-entity session advisory locks with resource-free optimistic observation tokens
  • compare and advance committed entity observation versions inside the short entity write transaction
  • preserve River and branch-generation fencing while treating stale branch-page responses as superseded
  • add small-pool regression coverage for concurrent slow branch-reconciliation pages
  • update the sync-engine contract and regenerate sqlc bindings

Root cause

BeginObservation acquired a dedicated pgxpool connection and held a session-level advisory lock across the authoritative GitHub request. Branch reconciliation can run four pages with up to 25 targets each, so slow remote responses could reserve roughly 100 connections and starve River leadership, producers, completion, and budget lease renewal even when PostgreSQL itself was healthy.

The replacement token snapshots the entity's last committed observation version and immediately releases the query connection. After remote I/O, the entity transaction takes the existing transaction-scoped advisory lock, rejects the response if another entity write committed in the meantime, and advances the version atomically with cache, outbox, and follow-up writes.

Impact

GitHub RPC fanout is now independent of pgxpool capacity. Concurrent responses remain monotonic, including equal-upstream-version races, without retaining a transaction, advisory-lock session, or reserved connection during network I/O.

Validation

  • go test ./...
  • go vet ./...
  • git diff --check
  • added TestSlowBranchPagesDoNotStarveSmallDatabasePool
  • added optimistic supersession, rollback, cancellation, and one-connection pool tests

The PostgreSQL-backed tests require TEST_DATABASE_URL; the new integration regression was discovered locally but skipped because that variable was unset. It will run in the repository's database-enabled CI environment.

Fixes #58

@ewhauser
ewhauser marked this pull request as ready for review August 7, 2026 03:34
@ewhauser
ewhauser merged commit 3cc5a62 into main Aug 7, 2026
5 checks passed
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.

Do not hold database sessions or transactions across GitHub RPCs

1 participant