Horton

Re: Add non-blocking version of PQcancel

Jacob Champion <pchampion@vmware.com>

View thread View on postgr.es
From: Jacob Champion <pchampion@vmware.com>
To: "Jelte.Fennema@microsoft.com" <Jelte.Fennema@microsoft.com>, "andres@anarazel.de" <andres@anarazel.de>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2022-03-09T00:27:42Z
Lists: pgsql-hackers 
On Thu, 2022-01-13 at 14:51 +0000, Jelte Fennema wrote:
> Attached is an updated patch which I believe fixes windows and the other test failures.
> At least on my machine make check-world passes now when compiled with --enable-tap-tests
> 
> I also included a second patch which adds some basic documentation for the libpq tests.

This is not a full review by any means, but here are my thoughts so
far:

> NOTE: I have not tested this with GSS for the moment. My expectation is
> that using this new API with a GSS connection will result in a
> CONNECTION_BAD status when calling PQcancelStatus. The reason for this
> is that GSS reads will also need to communicate back that an EOF was
> found, just like I've done for TLS reads and unencrypted reads.

For what it's worth, I did a smoke test with a Kerberos environment via


    ./libpq_pipeline cancel '... gssencmode=require'

and the tests claim to pass.

>     2. Cancel connections benefit automatically from any improvements made
>        to the normal connection establishment codepath. Examples of things
>        that it currently gets for free currently are TLS support and
>        keepalive settings.

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.

And does the backend actually handle cancel requests via TLS (or GSS)?
It didn't look that way from a quick scan, but I may have missed
something.

> @@ -1555,6 +1665,7 @@ print_test_list(void)
>     printf("singlerow\n");
>     printf("transaction\n");
>     printf("uniqviol\n");
> +   printf("cancel\n");
>  }

This should probably go near the top; it looks like the existing list
is alphabetized.

The new cancel tests don't print any feedback. It'd be nice to get the
same sort of output as the other tests.

>  /* issue a cancel request */
>  extern int PQcancel(PGcancel *cancel, char *errbuf, int errbufsize);
> +extern PGcancelConn * PQcancelConnectStart(PGconn *conn);
> +extern PGcancelConn * PQcancelConnect(PGconn *conn);
> +extern PostgresPollingStatusType PQcancelConnectPoll(PGcancelConn * cancelConn);
> +extern ConnStatusType PQcancelStatus(const PGcancelConn * cancelConn);
> +extern int PQcancelSocket(const PGcancelConn * cancelConn);
> +extern char *PQcancelErrorMessage(const PGcancelConn * cancelConn);
> +extern void PQcancelFinish(PGcancelConn * cancelConn);

That's a lot of new entry points, most of which don't do anything
except call their twin after a pointer cast. How painful would it be to
just use the existing APIs as-is, and error out when calling
unsupported functions if conn->cancelRequest is true?

--Jacob

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.