Re: statement_timeout is not working as expected with postgres_fdw

On Wed, May 3, 2017 at 3:49 PM, Robert Haas <robertmhaas@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