Feedback-ID: ia2694551:Fastmail
Date: Tue, 12 Mar 2024 17:50:48 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Jelte Fennema-Nio <postgres@jeltef.nl>
Cc: Denis Laxalde <denis.laxalde@dalibo.com>,
	vignesh C <vignesh21@gmail.com>, Thomas Munro <thomas.munro@gmail.com>,
	Greg Stark <stark@mit.edu>, Tom Lane <tgl@sss.pgh.pa.us>,
	"Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
 Jelte Fennema <Jelte.Fennema@microsoft.com>,
	Daniel Gustafsson <daniel@yesql.se>, Peter Eisentraut <peter@eisentraut.org>,
	Andres Freund <andres@anarazel.de>, Justin Pryzby <pryzby@telsasoft.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: [EXTERNAL] Re: Add non-blocking version of PQcancel
Message-ID: <202403121650.mpyd5m4y4udb@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAGECzQRbCq7y7TDBFao21ngnUw5VsBgdFKQ0QJXFCeNwSyf-+Q@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202403121650.mpyd5m4y4udb%40alvherre.pgsql>
Precedence: bulk

On 2024-Mar-12, Jelte Fennema-Nio wrote:

> On Tue, 12 Mar 2024 at 10:19, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > Here's a last one for the cfbot.
> 
> Thanks for committing the first 3 patches btw. Attached a tiny change
> to 0001, which adds "(backing struct for PGcancelConn)" to the comment
> on pg_cancel_conn.

Thanks, I included it.  I hope there were no other changes, because I
didn't verify :-) but if there were, please let me know to incorporate
them.

I made a number of other small changes, mostly to the documentation,
nothing fundamental.  (Someday we should stop using <listentry> to
document the libpq functions and use refentry's instead ... it'd be
useful to have manpages for these functions.)

One thing I don't like very much is release_conn_addrinfo(), which is
called conditionally in two places but unconditionally in other places.
Maybe it'd make more sense to put this conditionality inside the
function itself, possibly with a "bool force" flag to suppress that in
the cases where it is not desired.

In pqConnectDBComplete, we cast the PGconn * to PGcancelConn * in order
to call PQcancelPoll, which is a bit abusive, but I don't know how to do
better.  Maybe we just accept this ... but if PQcancelStart is the only
way to have pqConnectDBStart called from a cancel connection, maybe it'd
be saner to duplicate pqConnectDBStart for cancel conns.

Thanks!

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/