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
