Обсуждение: [PATCH] Writing changes of decoding plugin in the memory context where data is kept?

Hi all,

When working on a decoder plugin, I have been pointed that it is
incorrect to write changes in an output plugin while not being in the
memory context where changes are written.
In pg_decode_change@test_decoding.c, we do the following:
old = MemoryContextSwitchTo(data->context);
OutputPluginPrepareWrite(ctx, true);
[...]
MemoryContextSwitchTo(old);
MemoryContextReset(data->context);
OutputPluginWrite(ctx, true);

Wouldn't it be better to call OutputPluginWrite before switching back
to the old context? The attached patch for test_decoding does so.
Comments welcome.
Regards,
--
Michael

On 2014-05-15 09:11:15 +0900, Michael Paquier wrote:
> When working on a decoder plugin, I have been pointed that it is
> incorrect to write changes in an output plugin while not being in the
> memory context where changes are written.

That was a misunderstanding. It's perfectly alright to do so. The 'bug'
you have is that you sometimes wrote empty messages. Which
pg_recvlogical doesn't accept. There's little reason to allow doing so,
but for consistency we could probably ok to allow it.

Greetings,

Andres Freund

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

On Thu, May 15, 2014 at 9:18 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2014-05-15 09:11:15 +0900, Michael Paquier wrote:
>> When working on a decoder plugin, I have been pointed that it is
>> incorrect to write changes in an output plugin while not being in the
>> memory context where changes are written.
>
> That was a misunderstanding. It's perfectly alright to do so. The 'bug'
> you have is that you sometimes wrote empty messages. Which
> pg_recvlogical doesn't accept. There's little reason to allow doing so,
> but for consistency we could probably ok to allow it.
Yeah, thanks. Pushed a fix. Also I imagine that this is not really a
problem, but is there an impact on the plugin if
OutputPluginPrepareWrite is called and OutputPluginWrite is not for a
single change? By looking at the code I am guessing that we use
WalSndPrepareWrite for a plugin to prepare a message, and then
WalSndWriteData to finish the work, so we end up with an empty message
that is not sent to the backend. It doesn't matter much for
test_decoding as it always writes a change, but shouldn't we
discourage users to do so in the documentation? Something like a
big-fat warning mentioning that using OutputPluginPrepareWrite called
without OutputPluginWrite may result in stall messages stuck in the
queue?
-- 
Michael