From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: Andres Freund <andres@anarazel.de>, Jacob Champion <pchampion@vmware.com>,
        "Jelte.Fennema@microsoft.com" <Jelte.Fennema@microsoft.com>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Add non-blocking version of PQcancel
In-reply-to: 
 <CA+Tgmoav-v8HVuA-kLJM3Kf6XHtVfOPajQtyAkb5ejs7C_Aw2A@mail.gmail.com>
References: 
 <AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com>
 <20220113004411.3tdgiqdmmo4musb5@alap3.anarazel.de>
 <AM5PR83MB0178CE0CEB690F1C041355B7F7539@AM5PR83MB0178.EURPRD83.prod.outlook.com>
 <b6e02c2ef43a4c8dcbca03dd677ae8331c892175.camel@vmware.com>
 <3150365.1648158113@sss.pgh.pa.us>
 <20220324224909.4tnubqv2owmfoyoc@alap3.anarazel.de>
 <CA+Tgmoav-v8HVuA-kLJM3Kf6XHtVfOPajQtyAkb5ejs7C_Aw2A@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
	message dated "Fri, 25 Mar 2022 14:34:30 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3453534.1648234019.1@sss.pgh.pa.us>
Date: Fri, 25 Mar 2022 14:46:59 -0400
Message-ID: <3453535.1648234019@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3453535.1648234019%40sss.pgh.pa.us>
Precedence: bulk

Robert Haas <robertmhaas@gmail.com> writes:
> That said, I don't think that this particular patch is going in the
> right direction. I think Jacob's comment upthread is right on point:
> "This seems like a big change compared to PQcancel(); one that's not
> really hinted at elsewhere. Having the async version of an API open up
> a completely different code path with new features is pretty
> surprising to me." It seems to me that we want to end up with similar
> code paths for PQcancel() and the non-blocking version of cancel. We
> could get there in two ways. One way would be to implement the
> non-blocking functionality in a manner that matches exactly what
> PQcancel() does now. I imagine that the existing code from PQcancel()
> would move, with some amount of change, into a new set of non-blocking
> APIs. Perhaps PQcancel() would then be rewritten to use those new APIs
> instead of hand-rolling the same logic. The other possible approach
> would be to first change the blocking version of PQcancel() to use the
> regular connection code instead of its own idiosyncratic logic, and
> then as a second step, extend it with non-blocking interfaces that use
> the regular non-blocking connection code. With either of these
> approaches, we end up with the functionality working similarly in the
> blocking and non-blocking code paths.

I think you misunderstand where the real pain point is.  The reason
that PQcancel's functionality is so limited has little to do with
blocking vs non-blocking, and everything to do with the fact that
it's designed to be safe to call from a SIGINT handler.  That makes
it quite impractical to invoke OpenSSL, and probably our GSS code
as well.  If we want support for all connection-time options then
we have to make a new function that does not promise signal safety.

I'm prepared to yield on the question of whether we should provide
a non-blocking version, though I still say that (a) an easier-to-call,
one-step blocking alternative would be good too, and (b) it should
not be designed around the assumption that there's a completely
independent state object being used to perform the cancel.  Even in
the non-blocking case, callers should only deal with the original
PGconn.

> Leaving the question of approach aside, I think it's fairly clear that
> this patch cannot be seriously considered for v15.

Yeah, I don't think it's anywhere near fully baked yet.  On the other
hand, we do have a couple of weeks left.

			regards, tom lane