Skip to content

docs: explain why pg-sprite requires session affinity - #258

Merged
Kiran01bm merged 2 commits into
mainfrom
kiran01bm/docs-session-affinity
Oct 30, 2026
Merged

Kiran01bm merged 2 commits into
mainfrom
kiran01bm/docs-session-affinity

Conversation

@Kiran01bm

Copy link
Copy Markdown
Collaborator

Adds docs/session-affinity.md, one page that explains why pg-sprite needs one PostgreSQL session per connection and how to connect it.

Why

Every command that connects builds its pool through dbconn.NewPool, which always proves session affinity and refuses transaction-mode poolers, with no opt-out. The reasons are spread across LK-1, LK-2, CO-9, code comments and the Supabase guide, so an operator who hits ErrNoSessionAffinity has no single page saying why the requirement exists or what endpoint to use.

What

  • New docs/session-affinity.md:
    • whether affinity is mandatory and for which commands;
    • the session state it protects (timeouts, the table lock, a catalog-first search_path);
    • what breaks behind transaction- and statement-mode poolers;
    • the proof, step by step;
    • which endpoints pass, how to connect and size a pooler, what the proof does not cover, and where it is enforced.
  • Index row in docs/README.md.

Before / after

An operator points pg-sprite migrate at their platform's default transaction-mode pooler URL and gets ErrNoSessionAffinity. Nothing about the behaviour changes; only where they can read about it.

Before                                        After
──────                                        ─────
migrate --alter ... (transaction pooler)      migrate --alter ... (transaction pooler)
        │                                             │
        ▼                                             ▼
error: the connection does not keep one       error: the connection does not keep one
PostgreSQL session; connect through a         PostgreSQL session; connect through a
session-mode endpoint ...                     session-mode endpoint ...
        │                                             │
        ▼                                             ▼
operator searches docs: the reason is         operator opens docs/session-affinity.md:
split across invariants LK-1/LK-2/CO-9,       why the timeouts and the table lock need
code comments and supabase.md                 one session, what the proof checked, which
                                              endpoints pass, how to size the pooler

🤖 Generated with Amp (Claude Opus 4.6)

Every pool proves session affinity and refuses transaction- and
statement-mode poolers, but no doc explained why or how to connect.
@Kiran01bm
Kiran01bm marked this pull request as ready for review October 25, 2026 00:56
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-26T19:25:01.913161Z
We couldn’t complete this review. Comment @codex review to try again.
ca69420 Draft marked ready
ℹ️ 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" or "@codex security review".

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

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@Kiran01bm
Kiran01bm marked this pull request as draft October 26, 2026 19:24
@Kiran01bm
Kiran01bm marked this pull request as ready for review October 26, 2026 19:24
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@JashLal

JashLal commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

🤖 Reviewed by Jash's AI agent at ca6942072374cef4aa2d6f7f43d78f0e733da07f.

Verdict: useful documentation, with one safety-contract clarification needed. This changes only documentation; the explanation of session state and the recommendation to use a direct or session-mode endpoint are helpful.

1. [P2] Qualify the claim that transaction pooling is always detected (docs/session-affinity.md:113–116). This says the second connection necessarily takes the first connection's backend and that at least one reading fails. But lines 158–161 explicitly allow a transaction pooler with spare backends to answer every reading healthily. That caveat also conflicts with the unconditional refusal in the opening and endpoint table. An operator could interpret a successful startup as certification that a transaction endpoint is safe, despite the documented advisory-lock and timeout hazards.

Action item:

  1. Describe the probe as an attempt to expose backend reassignment, qualify the refusal/table claims, and state beside the algorithm that operators must configure a direct or session-mode endpoint even when the probe succeeds. Keep the existing false-negative caveat prominent.

Verification: read the description, existing top-level comments and full two-file diff; no runtime code or tests changed or were deleted. GitHub reports six successful checks and six skipped checks, including skipped build and PostgreSQL integration jobs. The automated Codex review reports a usage-limit failure. No local build/tests or live-pooler reproduction were run; exact-head implementation and inline review comments could not be retrieved because those API requests failed, so implementation claims beyond the document's own consistency remain unverified. No internal-only material found in the inspected title, body or diff.

Generated by Codex.

@JashLal

JashLal commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

🤖 Product review at ca6942072374cef4aa2d6f7f43d78f0e733da07f.

OSS adoption: The single entry point and separation of the application's connection path from pg-sprite's endpoint make setup clearer. One optional troubleshooting improvement: qualify the endpoint table's exact error names. Lines 69–73 already explain that startup parameters can be rejected or discarded and produce ErrSessionSettingsDiscarded before the affinity probe, whereas the transaction-mode rows promise ErrNoSessionAffinity. Listing both paths would help operators recognize the same endpoint problem from either symptom.

SchemaBot integration: This documentation-only change introduces no API change. Preserve the distinction between discarded settings and failed affinity detection for callers consuming typed errors, and describe a successful probe as evidence rather than endpoint certification. I did not inspect the consuming adapter or reproduce its error handling in this review.

Generated by Codex.

@JashLal JashLal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Approving on Jash's behalf under the authorized review workflow at ca69420. The P2 documentation finding remains: qualify deterministic refusal claims to match the documented false-negative limitation, and require a configured direct/session-mode endpoint even when the probe succeeds. This approval does not mean the finding is resolved. Source/API verification limits and the absence of local tests are disclosed in the review. Generated by Codex.

A transaction pooler with spare backends can pass the proof, so the page
no longer promises it is always refused and names both refusal errors.
It also follows the statement-mode refusal: SELECT 1 before the BEGIN,
then ErrNoSessionAffinity.
@Kiran01bm

Copy link
Copy Markdown
Collaborator Author

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude Opus 4.6) — pull/258, follow-up commit

Both findings are fixed in the follow-up commit. The page now treats the affinity proof as evidence, not certification, and names both errors an unsupported pooler can produce.

# Finding Status Explanation
C1-F1 The transaction-mode description says the proof always catches the pooler, contradicting the false-negative caveat fixed The opening, the algorithm and the endpoint table no longer promise a refusal. Beside the algorithm, the page now says the proof tries to expose the rebind, that a pooler with spare backends can pass it, and that a passing proof does not make a transaction-mode endpoint safe, so pg-sprite must be pointed at a direct or session-mode endpoint whether or not it passes. Transaction and statement mode are listed as "Not supported".
C2-F1 The endpoint table should list both symptoms: ErrSessionSettingsDiscarded before the probe, ErrNoSessionAffinity from it fixed The transaction- and statement-mode rows now name both errors, and the table notes that both carry the same remedy. While there, the page was brought up to date with main's statement-mode refusal: the probe's SELECT 1 before the BEGIN is in the diagram, and statement mode is refused with ErrNoSessionAffinity.
C2 Keep discarded settings and failed affinity distinct for typed-error callers no action They stay two distinct errors on the page, each described where it is raised.

Source: block/pg-sprite#258, review comments 6425433570 and 6425441119 and review 5657871043 at head ca694207; fixes in the follow-up commit.

@Kiran01bm
Kiran01bm enabled auto-merge (squash) October 30, 2026 04:50
@Kiran01bm
Kiran01bm merged commit d5e40c2 into main Oct 30, 2026
12 checks passed
@Kiran01bm
Kiran01bm deleted the kiran01bm/docs-session-affinity branch October 30, 2026 04:53
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