On Wed, May 3, 2017 at 3:49 PM, Robert Haas <robertmh...@gmail.com> wrote:
> It seems pretty clear to me that we are not going to fix all of these
> issues in one patch.  Here's a sketch of an idea for how to start
> making things better:

Patch attached.

> - Add an in_abort_cleanup flag to ConnCacheEntry.

Ended up renaming this to abort_cleanup_incomplete.

> - Before pgfdw_xact_callback() or pgfdw_subxact_callback() begin abort
> cleanup, check whether the flag is set.  If so, slam the connection
> shut unless that's already been done; furthermore, if the flag is set
> and we're in pgfdw_xact_callback (i.e. this is a toplevel abort),
> forget about the connection entry entirely.  On the other hand, if the
> flag is not set, set it flag and attempt abort cleanup.  If we
> succeed, clear the flag.

Did approximately this.  It turned out not to be necessary to add any
new calls to PQfinish(); the existing one was adequate.

> - Before pgfdw_xact_callback() or pgfdw_subxact_callback() begin
> pre-commit processing, check whether the flag is set.  If so, throw an
> ERROR, so that we switch over to abort processing.

Did this.

> - Change uses of PQexec() in the abort path to use pgfdw_exec_query()
> instead.  If we exit pgfdw_exec_query() with an error, we'll re-enter
> the abort path, but now in_abort_cleanup will be set, so we'll just
> drop the connection (and force any outer transaction levels to abort
> as well).

Created a new function pgfdw_exec_cleanup_query() for this, also
incorporating a timeout.

Also fixed things so that after issuing PQcancel() we actually throw
away the pending result from the cancelled query, and added some error
recursion protection.

Review would be appreciated.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment: improve-pgfdw-abort-behavior-v1.patch
Description: Binary data

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to