Re: Inconsistent error handling in START_REPLICATION command

On 20 January 2016 at 15:28, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:

>
> For that reason I'd actually like to enter COPY BOTH mode before the
> startup callback, as is currently the case. So I'd like to wrap the
> decoding startup callback in a PG_TRY that catches an ERROR raised by the
> startup callback (if any) and exits COPY BOTH mode before re-throwing.
>
>
I hoped this'd be as simple as:

PG_TRY();
{
logical_decoding_ctx = CreateDecodingContext(
cmd->startpoint,
cmd->options,

logical_read_xlog_page,
WalSndPrepareWrite,
WalSndWriteData);
}
PG_CATCH();
{
pq_putmessage_noblock('c', NULL, 0);
pq_flush();
PG_RE_THROW();
}
PG_END_TRY();

but that's doesn't look sufficient, presumably as the client never has a
chance to ack the exit from CopyBoth mode. Or maybe libpq just isn't
prepared to cope with existing COPY then immediately getting an error.

What you have now works and fixes the immediate problem. Preserving the
early start of COPY mode isn't actually useful until we're also prepared to
accept writes from the output plugin's startup callback at that point,
which we are not yet.

Maybe we should just apply your patch as-is, then move the start of COPY
back if/when support for emitting output from within a logical decoding
startup callback gets added.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services