fix(pgvector): reconnect once when the server drops the pooled connection - #2084
fix(pgvector): reconnect once when the server drops the pooled connection#2084messelink wants to merge 1 commit into
Conversation
|
Thanks for this contribution, and apologies for the slow turnaround.
If you'd rather not pick it back up, no problem at all — just say so and I'll close it out, and thanks either way for taking the time to send it. |
…tion _PgVectorClient holds one psycopg connection for the life of the process and reuses it. When Postgres restarts — a package upgrade, a failover, an operator's pg_ctl restart — that handle looks alive until the next statement runs on it, and that statement is the one that surfaces the drop. Every restart therefore costs each client one spurious "pgvector query failed: terminating connection due to administrator command" before the following call reconnects and succeeds. Retry that first statement once on a fresh connection, which is what a short-lived client would have done implicitly. The retry is gated on the failure being about the connection rather than the statement: - SQLSTATE class 57 (operator intervention): 57P01 admin shutdown, 57P02 crash shutdown, 57P03 cannot connect now. - A driver error raised on a handle psycopg has already marked closed or broken, which is how a mid-query TCP drop arrives — no SQLSTATE, because no server response came back to carry one. Consulted only when no SQLSTATE arrived, so a live connection that merely rejected the statement is never mistaken for a lost one. A statement-level failure (bad SQL, constraint violation, type error) leaves the connection usable and propagates unchanged; retrying it would run the same failing statement twice and report the second one. A second drop on the fresh connection is a real outage rather than a stale pooled handle, and surfaces as the BackendError callers already expect. Retrying is safe for the write path: upserts are by id and the callers already run an idempotency check, so a statement that never reached the server is replayed, not duplicated. Covers many=True (upsert_rows) as well as single statements.
7b728e7 to
fa2ed2c
Compare
|
Picked it back up. Rebased onto current The only conflict was in CI is green across all legs. Let me know if anything needs changing in review. |
Problem
_PgVectorClientholds one psycopg connection for the life of the process and reuses it. When Postgres restarts — a package upgrade, a failover, an operator'spg_ctl restart— that handle looks alive until the next statement runs on it, and that statement is the one that surfaces the drop:The call after it reconnects and succeeds, so every restart costs each connected client exactly one spurious error. On a shared palace with several long-lived MCP servers, one
systemctl restart postgresqlproduces one of these per client.Observed in a multi-machine setup where unattended-upgrades restarts Postgres for library updates: each restart surfaces the error once per peer, then self-heals.
Verified live:
pg_terminate_backend()against a client's own backend PID reproduces the error ondevelopand is transparently absorbed with this patch, with the client landing on a new backend PID.Fix
Retry that first statement once on a fresh connection — what a short-lived client would have done implicitly.
The retry is gated on the failure being about the connection rather than the statement:
57P01admin shutdown,57P02crash shutdown,57P03cannot connect now.A statement-level failure (bad SQL, constraint violation, type error) leaves the connection usable and propagates unchanged; retrying would run the same failing statement twice and report the second one. A second drop on the fresh connection is a real outage rather than a stale pooled handle, and surfaces as the
BackendErrorcallers already expect.Safety
Retrying is safe for the write path: upserts are by id and callers already run an idempotency check, so a statement that never reached the server is replayed, not duplicated.
many=True(upsert_rows) is covered.Tests
Five new cases in
tests/test_pgvector_backend.py:BackendError, exactly one retry, no retry stormexecutemanypath coveredRelated