On Thu, 4 Jul 2024 at 14:43, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
> I don't have any immediate feedback regarding this patch, but I'm
> wondering about one thing related to cancellations - we talk cancelling
> a query, but we really target a PID (or a particular backend, no matter
> how we identify it).
>
> I occasionally want to only cancel a particular query, but I don't think
> that's really possible - the query may complete meanwhile, and the
> backend may even get used for a different user connection (e.g. with a
> connection pool)? Or am I missing something important?
No, you're not missing anything. Having the target of the cancel
request be the backend instead of a specific query is really annoying
and can cause all kinds of race conditions. I had to redesign and
complicate the cancellation logic in PgBouncer significantly, to make
sure that one client could not cancel a connection from another client
anymore: https://github.com/pgbouncer/pgbouncer/pull/717
> Anyway, I wonder if making the cancellation key longer (or variable
> length) might be useful for this too - it would allow including some
> sort of optional "query ID" in the request, not just the PID. (Maybe
> st_xact_start_timestamp would work?)
Yeah, some query ID would be necessary. I think we'd want a dedicated
field for it though. Instead of encoding it in the secret. Getting the
xact_start_timestamp would require communication with the server to
get it, which you would like to avoid since the server might be
unresponsive. So I think a command counter that both sides keep track
of would be better. This counter could then be incremented after every
Query and Sync message.
> Obviously, that's not up to this patch, but it's somewhat related.
Yeah, let's postpone more discussion on this until we have the
currently proposed much simpler change in, which only changes the
secret length.
