Re: psql not responding to SIGINT upon db reconnection

From: "Tristan Partin" <tristan(at)neon(dot)tech>
To: "Robert Haas" <robertmhaas(at)gmail(dot)com>
Cc: "Jelte Fennema-Nio" <postgres(at)jeltef(dot)nl>, "Heikki Linnakangas" <hlinnaka(at)iki(dot)fi>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, <pgsql-hackers(at)postgresql(dot)org>, "Shlok Kyal" <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
Subject: Re: psql not responding to SIGINT upon db reconnection
Date: 2024-03-22 20:58:42
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri Mar 22, 2024 at 12:17 PM CDT, Robert Haas wrote:
> On Fri, Mar 22, 2024 at 1:05 PM Tristan Partin <tristan(at)neon(dot)tech> wrote:
> > Sorry for taking a while to get back to y'all. I have taken your
> > feedback into consideration for v9. This is my first time writing
> > Postgres docs, so I'm ready for another round of editing :).
> Yeah, that looks like it needs some wordsmithing yet. I can take a
> crack at that at some point, but here are a few notes:
> - "takes care of" and "working through the state machine" seem quite
> vague to me.
> - the meanings of forRead and forWrite don't seem to be documented.
> - "retuns" is a typo.
> > Robert, could you point out some places where comments would be useful
> > in 0002? I did rename the function and moved it as suggested, thanks! In
> > turn, I also realized I forgot a prototype, so also added it.
> Well, for starters, I'd give the function a header comment.
> Telling me that a 1 second timeout is a 1 second timeout is less
> useful than telling me why we've picked a 1 second timeout. Presumably
> the answer is "so we can respond to cancel interrupts in a timely
> fashion", but I'd make that explicit.
> It might be worth a comment saying that PQsocket() shouldn't be
> hoisted out of the loop because it could be a multi-host connection
> string and the socket might change under us. Unless that's not true,
> in which case we should hoist the PQsocket() call out of the loop.
> I think it would also be worth a comment saying that we don't need to
> report errors here, as the caller will do that; we just need to wait
> until the connection is known to have either succeeded or failed, or
> until the user presses cancel.

This is good feedback, thanks. I have added comments where you
suggested. I reworded the PQsocketPoll docs to hopefully meet your
expectations. I am using the term "connection sequence" which is from
the PQconnectStartParams docs, so hopefully people will be able to make
that connection. I wrote documentation for "forRead" and "forWrite" as

I had a question about parameter naming. Right now I have a mix of
camel-case and snake-case in the function signature since that is what
I inherited. Should I change that to be consistent? If so, which case
would you like?

Thanks for your continued reviews.

Tristan Partin
Neon (

Attachment Content-Type Size
v10-0001-Expose-PQsocketPoll-for-frontends.patch text/x-patch 6.4 KB
v10-0002-Allow-SIGINT-to-cancel-psql-database-reconnectio.patch text/x-patch 3.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2024-03-22 21:31:11 Re: Large block sizes support in Linux
Previous Message Nathan Bossart 2024-03-22 20:51:34 Re: SET ROLE documentation improvement