On Fri, 2 Feb 2024 at 16:06, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Now, looking at this list, I think it's surprising that the nonblocking
> request for a cancellation is called PQcancelPoll. PQcancelSend() is at
> odds with the asynchronous query API, which uses the verb "send" for the
> asynchronous variants. This would suggest that PQcancelPoll should
> actually be called PQcancelSend or maybe PQcancelStart (mimicking
> PQconnectStart). I'm not sure what's a good alternative name for the
> blocking one, which you have called PQcancelSend.
I agree that Send is an unfortunate suffix. I'd love to use PQcancel
for this, but obviously that one is already taken. Some other options
that I can think of are (from favorite to less favorite):
- PQcancelBlocking
- PQcancelAndWait
- PQcancelGo
- PQcancelNow
Finally, another option would be to renome PQcancelConn to
PQgetCancelConn and then rename PQcancelSend to PQcancelConn.
Regarding PQcancelPoll, I think it's a good name for the polling
function, but I agree it's a bit confusing to use it to also start
sending the connection. Even the code of PQcancelPoll basically admits
that this is confusing behaviour:
/*
* Before we can call PQconnectPoll we first need to start the connection
* using pqConnectDBStart. Non-cancel connections already do this whenever
* the connection is initialized. But cancel connections wait until the
* caller starts polling, because there might be a large delay between
* creating a cancel connection and actually wanting to use it.
*/
if (conn->status == CONNECTION_STARTING)
{
if (!pqConnectDBStart(&cancelConn->conn))
{
cancelConn->conn.status = CONNECTION_STARTED;
return PGRES_POLLING_WRITING;
}
}
The only reasonable thing I can think of to make that situation better
is to move that part of the function outside of PQcancelPoll and
create a dedicated PQcancelStart function for it. It introduces an
extra function, but it does seem more in line with how we do the
regular connection establishment. Basically you would have code like
this then, which looks quite nice honestly:
cancelConn = PQcancelConn(conn);
if (!PQcancelStart(cancelConn))
pg_fatal("bad cancel connection: %s", PQcancelErrorMessage(cancelConn));
while (true)
{
// polling using PQcancelPoll here
}
