Skip to content

Make backend query execution interruptible. - #454

Open
nmisch wants to merge 3 commits into
2ndQuadrant:REL2_x_STABLEfrom
nmisch:pqexec-helpers
Open

nmisch wants to merge 3 commits into
2ndQuadrant:REL2_x_STABLEfrom
nmisch:pqexec-helpers

Conversation

@nmisch

@nmisch nmisch commented Jan 8, 2024

Copy link
Copy Markdown
Collaborator

This prevents v15+ DROP DATABASE hanging when a pglogical process waits inside
one of these calls. (See #418
for fixes of that symptom under different conditions.) In all versions, it
brings responsiveness of these calls to query cancel, backend termination, and
fast shutdown.

Future work should include adopting libpqsrv_connect_params(). I didn't bundle
that in this pull request, because that function doesn't support all the
connection parameters that PQconnectdbParams() supports. Specifically, since
libpqsrv_connect_params() wraps PQconnectStartParams(), it has the limitations
discussed in
https://postgr.es/m/flat/CA%2Bmi_8YyGKA9dWELu63e%3DKL2oN-%2BFe4uca4EtFfb6uQD4Up8pw%40mail.gmail.com.
Future work should also bring interruptibility to calls of
PQgetCopyData(async=false), PQputCopyData(), and PQputCopyEnd().

No new test failures in:

  • "make check" @ v9.5+
  • "make installcheck" @ v9.4, using Fix build on v9.4. #450 for v9.4 support
  • "make prove_check" @ v9.6 - v14

When v17 support appears, compat17/pglogical_compat.h can just #include
"libpq/libpq-be-fe-helpers.h" rather than use the local copy.

The compatibility macros mess up some postgresql headers, yielding compilation
failures.  pglogical doesn't use affected headers now, so there's no live bug.
The next change will add use of such a header.
https://www.postgresql.org/docs/current/libpq-copy.html directs COPY callers to
do this.  By not doing this, pglogical risked ignoring a late failure in the
COPY FROM query.  This change unblocks replacing PQexec() with PQsendQuery().
The former silently discards any prior query result, but the latter fails.

Since plain PQgetResult() blocks without responding to interrupts, import the
PostgreSQL v17 libpq/libpq-be-fe-helpers.h for its interruptible helper.  Adapt
the header to v9.4+ with PG_VERSION_NUM conditions and changing
WL_EXIT_ON_PM_DEATH to explicit proc_exit().  Divide the header into two
include-guarded regions, one for v16 content and one for v17+ content.
This prevents v15+ DROP DATABASE hanging when a pglogical process waits inside
one of these calls.  (See 2ndQuadrant#418
for fixes of that symptom under different conditions.)  In all versions, it
brings responsiveness of these calls to query cancel, backend termination, and
fast shutdown.
nmisch added a commit that referenced this pull request May 23, 2026
This serves a few distinct purposes:

- Beta-test the wrappers for PostgreSQL's benefit.

- Since libpq-be-fe-helpers.h activates the libpqsrv_PGresult wrappers
  unconditionally and #454 uses libpq-be-fe-helpers.h, #454 would
  activate the wrappers in some files.  By activating the wrappers
  first, this change isolates any wrapper-related side effects from
  other effects of #454.

- When postgr.es/c/7d8f5957792421ec3bb9d1b9b6ca25d689d974b7 introduced
  the wrappers to PostgreSQL 19, the motive was stopping memory leaks.
  The same benefit tends to apply here, though there have been no
  reports of PGresult leaks in pglogical.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant