Horton

Re: [EXTERNAL] Re: Add non-blocking version of PQcancel

Jelte Fennema-Nio <postgres@jeltef.nl>

View thread View on postgr.es
From: Jelte Fennema-Nio <postgres@jeltef.nl>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alexander Lakhin <exclusion@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Noah Misch <noah@leadboat.com>, Jacob Champion <jacob.champion@enterprisedb.com>, Denis Laxalde <denis.laxalde@dalibo.com>, vignesh C <vignesh21@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Greg Stark <stark@mit.edu>, "Gregory Stark (as CFM)" <stark.cfm@gmail.com>, Jelte Fennema <Jelte.Fennema@microsoft.com>, Daniel Gustafsson <daniel@yesql.se>, Peter Eisentraut <peter@eisentraut.org>, Andres Freund <andres@anarazel.de>, Justin Pryzby <pryzby@telsasoft.com>, Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Etsuro Fujita <etsuro.fujita@gmail.com>
Date: 2024-08-30T21:24:58Z
Lists: pgsql-hackers 
On Fri, 30 Aug 2024 at 22:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Jelte Fennema-Nio <postgres@jeltef.nl> writes:
> > On Fri, Aug 30, 2024, 21:21 Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> While we're piling on, has anyone noticed that *non* Windows buildfarm
> >> animals are also failing this test pretty frequently?
>
> > Yes. Fixes are here (see the ~10 emails above in the thread for details):
> > https://www.postgresql.org/message-id/CAGECzQQO8Cn2Rw45xUYmvzXeSSsst7-bcruuzUfMbGQc3ueSdw@mail.gmail.com
>
> Hmm.  I'm not convinced that 0001 is an actual *fix*, but it should
> at least reduce the frequency of occurrence a lot, which'd help.

I also don't think it's an actual fix, but I couldn't think of a way
to fix this. And since this only happens if you cancel right at the
start of a postgres_fdw query, I don't think it's worth investing too
much time on a fix.

> I don't want to move the test case to where you propose, because
> that's basically not sensible.  But can't we avoid remote estimates
> by just cross-joining ft1 to itself, and not using the tables for
> which remote estimate is enabled?

Yeah that should work too (I just saw your next email, where you said
it's committed like this).

> I think 0002 is probably outright wrong, or at least the change to
> disable_statement_timeout is.  Once we get to that, we don't want
> to throw a timeout error any more, even if an interrupt was received
> just before it.

The disable_statement_timeout change was not the part of that patch
that was necessary for stable output, only the change in the first
branch of enable_statement_timeout was necessary. The reason being
that enable_statement_timeout is called multiple times for a query,
because start_xact_command is called multiple times in
exec_simple_query. The change to disable_statement_timeout just seemed
like the logical extension of that change, especially since there was
basically a verbatim copy of disable_statement_timeout in the second
branch of enable_statement_timeout.

To make sure I understand your suggestion correctly: Are you saying
you would want to completely remove the outstanding interrupt if it
was caused by de statement_timout when disable_statement_timeout is
called? Because I agree that would probably make sense, but that
sounds like a more impactful change. But the current behaviour seems
strictly worse than the behaviour proposed in the patch to me, because
currently the backend would still be interrupted, but the error would
indicate a reason for the interrupt that is simply incorrect i.e. it
will say it was cancelled due to a user request, which never happened.

Commits

  1. postgres_fdw: re-issue cancel requests a few times if necessary.

  2. Make postgres_fdw's query_cancel test less flaky.

  3. postgres_fdw: Split out the query_cancel test to its own file

  4. Fix copy-paste mistake in PQcancelCreate

  5. Make libpqsrv_cancel's return const char *, not char *

  6. Stabilize postgres_fdw test

  7. libpq-be-fe-helpers.h: wrap new cancel APIs

  8. dblink/isolationtester/fe_utils: Use new cancel API

  9. Put libpq_pipeline cancel test back

  10. Hopefully make libpq_pipeline's new cancel test more reliable

  11. libpq: Add encrypted and non-blocking query cancellation routines

  12. libpq: Move pg_cancel to fe-cancel.c

  13. Add tests for libpq query cancellation APIs

  14. Add missing connection statuses to docs

  15. libpq: Change some static functions to extern

  16. libpq: Add pqReleaseConnHosts function

  17. libpq: Move cancellation related functions to fe-cancel.c

  18. Make spelling of cancelled/cancellation consistent

  19. Be more wary about OpenSSL not setting errno on error.

  20. libpq: Use modern socket flags, if available.

  21. Drop test view when done with it.

  22. Doc: add some doco about using the libpq_pipeline test module.

  23. postgres_fdw: Allow cancellation of transaction control commands.