Re: Add non-blocking version of PQcancel

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Jacob Champion <pchampion(at)vmware(dot)com>, "Jelte(dot)Fennema(at)microsoft(dot)com" <Jelte(dot)Fennema(at)microsoft(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add non-blocking version of PQcancel
Date: 2022-03-25 18:46:59
Message-ID: 3453535.1648234019@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-03-25 19:11:47 Re: Add psql command to list constraints
Previous Message David G. Johnston 2022-03-25 18:40:01 Re: pg_dump new feature: exporting functions only. Bad or good idea ?