Thread: [PATCH] Writing changes of decoding plugin in the memory context where data is kept?
[PATCH] Writing changes of decoding plugin in the memory context where data is kept?
From
Michael Paquier
Date:
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
Attachment
Re: [PATCH] Writing changes of decoding plugin in the memory context where data is kept?
From
Andres Freund
Date:
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
Re: [PATCH] Writing changes of decoding plugin in the memory context where data is kept?
From
Michael Paquier
Date:
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