On Mon, Aug 23, 2021 at 10:45 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Do we actually need new GUCs, though? I think we should never let an
> unresponsive client dictate what the server does, because that opens the
> door for uncooperative or malicious clients to wreak serious havoc. I
> think the implementation should wait until time now+X to cancel the
> query, but if by time now+2X (or whatever we deem reasonable -- maybe
> now+1.1X) we're still waiting, then it's okay to just close the
> connection. This suggests a completely different implementation, though.
I don't quite understand what you mean by waiting until time now+X to
cancel the query. We don't know a priori when query cancels are going
to happen, but when they do happen we want to respond to them as
quickly as we can. It seems reasonable to say that if we can't handle
them within X amount of time we can resort to emergency measures, but
that's basically what the patch does, except it uses a GUC instead of
hardcoding X.
> I wonder if it's possible to write a test for this. We would have to
> send a query and then hang the client somehow. I recently added a TAP
> test that uses SIGSTOP to a walsender ... can we use SIGSTOP with a
> background psql that's running SELECT pg_sleep() perhaps?
> (Or maybe it's sufficient to start background psql and not pump() it?)
Starting a background process and not pumping it sounds promising,
because it seems like it would be more likely to be portable. I think
we'd want to be careful not to assume very much about how large the
output buffer is, because I'm guessing that varies by platform and
that it might in some cases be fairly large. Perhaps we could use
pg_stat_activity to wait until we block in a ClientWrite state,
although I wonder if we might find out that we sometimes block on a
different wait state than what we expect to see.
--
Robert Haas
EDB: http://www.enterprisedb.com