Horton

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

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread View on postgr.es
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Jelte Fennema-Nio <postgres@jeltef.nl>
Cc: vignesh C <vignesh21@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Denis Laxalde <denis.laxalde@dalibo.com>, Greg Stark <stark@mit.edu>, Tom Lane <tgl@sss.pgh.pa.us>, "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>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2024-02-02T15:05:56Z
Lists: pgsql-hackers 
Hello,

The patched docs claim that PQrequestCancel is insecure, but neither the
code nor docs explain why.  The docs for PQcancel on the other hand do
mention that encryption is not used; does that apply to PQrequestCancel
as well and is that the reason?  If so, I think we should copy the
warning and perhaps include a code comment about that.  Also, maybe that
final phrase in PQcancel should be a <caution> box: remove from "So, for
example" and add <caution><para>Because gssencmode and sslencmode are
not preserved from the original connection, the cancel request is not
encrypted.</para></caution> or something like that.


I wonder if Section 33.7 Canceling Queries in Progress should be split
in three subsections, and I propose the following order:

33.7.1 PGcancelConn-based Cancellation API
  PQcancelConn		-- we first document the basics
  PQcancelSend
  PQcancelFinish
  PQcancelPoll		-- the nonblocking interface is documented next
  PQcancelReset		-- reuse a cancelconn, later in docs because it's more advanced
  PQcancelStatus	-- accessors go last
  PQcancelSocket
  PQcancelErrorMessage

33.7.2 Obsolete interface
  PQgetCancel
  PQfreeCancel
  PQcancel

33.7.3 Deprecated and Insecure Methods
  PQrequestCancel

I have a hard time coming up with good subsection titles though.

Now, looking at this list, I think it's surprising that the nonblocking
request for a cancellation is called PQcancelPoll.  PQcancelSend() is at
odds with the asynchronous query API, which uses the verb "send" for the
asynchronous variants.  This would suggest that PQcancelPoll should
actually be called PQcancelSend or maybe PQcancelStart (mimicking
PQconnectStart).  I'm not sure what's a good alternative name for the
blocking one, which you have called PQcancelSend.

I see upthread that the names of these functions were already quite
heavily debated.  Sorry to beat that dead horse some more ... I'm just
not sure it's decided matter.

Lastly -- the doc blurbs that say simply "a version of XYZ that can be
used for cancellation connections" are a bit underwhelming.  Shouldn't
we document these more fully instead of making users go read the docs
for the other functions and wonder what the differences might be, if
any?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"Before you were born your parents weren't as boring as they are now. They
got that way paying your bills, cleaning up your room and listening to you
tell them how idealistic you are."  -- Charles J. Sykes' advice to teenagers

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.