Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

On Mon, Jul 13, 2020 at 2:56 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Jul 13, 2020 at 2:32 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 13, 2020 at 11:10 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > >
> > > I think you can refer to commit message as well for that "We however
> > > must explicitly disable streaming replication during replication slot
> > > creation, even if the plugin supports it. We don't need to replicate
> > > the changes accumulated during this phase, and moreover, we don't have
> > > a replication connection open so we don't have where to send the data
> > > anyway.". I don't think this is a good way to hack the streaming flag
> > > because for SQL API's, we don't have a good reason to disable the
> > > streaming in this way. I guess if we had a condition related to
> > > reaching CONSISTENT snapshot during streaming then we won't need to
> > > hack the streaming flag in this way. Once we reach the CONSISTENT
> > > snapshot state, we come out of the creation of a replication slot (see
> > > how we use DecodingContextReady to achieve that) phase. So, I feel we
> > > should remove the ctx->streaming setting to false and add a CONSISTENT
> > > snapshot check during streaming unless you have a reason for not doing
> > > so.
> >
> > I was worried about the point that streaming on/off is sent by the
> > subscriber on START REPLICATION, not on CREATE REPLICATION SLOT, so if
> > we keep streaming on during create then it may not be right.
> >
>
> Then, how is that used on the publisher-side? AFAICS, the streaming
> is enabled based on whether streaming callbacks are provided and we do
> that in 0003-Extend-the-logical-decoding-output-plugin-API-wi patch.

Basically, first, we enable based on whether we have the callbacks or
not but later once we get the START REPLICATION command from the
subscriber then we set it to false if the streaming is not enabled
from the subscriber side. You can refer below code in patch 0007.

pgoutput_startup
{
parse_output_parameters(ctx->output_plugin_options,
&data->protocol_version,
- &data->publication_names);
+ &data->publication_names,
+ &enable_streaming);
/* Check if we support requested protocol */
if (data->protocol_version > LOGICALREP_PROTO_VERSION_NUM)
@@ -222,6 +284,27 @@ pgoutput_startup(LogicalDecodingContext *ctx,
OutputPluginOptions *opt,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("publication_names parameter missing")));
+ /*
+ * Decide whether to enable streaming. It is disabled by default, in
+ * which case we just update the flag in decoding context. Otherwise
+ * we only allow it with sufficient version of the protocol, and when
+ * the output plugin supports it.
+ */
+ if (!enable_streaming)
+ ctx->streaming = false;
}

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com