Re: Force streaming every change in logical decoding

On Wed, Dec 7, 2022 at 8:46 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Tue, Dec 6, 2022 at 9:29 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Tue, Dec 6, 2022 at 11:53 AM shiy(dot)fnst(at)fujitsu(dot)com
> > <shiy(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > Hi hackers,
> > >
> > > In logical decoding, when logical_decoding_work_mem is exceeded, the changes are
> > > sent to output plugin in streaming mode. But there is a restriction that the
> > > minimum value of logical_decoding_work_mem is 64kB. I tried to add a GUC to
> > > allow sending every change to output plugin without waiting until
> > > logical_decoding_work_mem is exceeded.
> > >
> > > This helps to test streaming mode. For example, to test "Avoid streaming the
> > > transaction which are skipped" [1], it needs many XLOG_XACT_INVALIDATIONS
> > > messages. With the new option, it can be tested with fewer changes and in less
> > > time. Also, this new option helps to test more scenarios for "Perform streaming
> > > logical transactions by background workers" [2].
> > >
>
> +1
>
> >
> > Yeah, I think this can also help in reducing the time for various
> > tests in test_decoding/stream and
> > src/test/subscription/t/*_stream_*.pl file by reducing the number of
> > changes required to invoke streaming mode. Can we think of making this
> > GUC extendible to even test more options on server-side (publisher)
> > and client-side (subscriber) cases? For example, we can have something
> > like logical_replication_mode with the following valid values: (a)
> > server_serialize: this will serialize each change to file on
> > publishers and then on commit restore and send all changes; (b)
> > server_stream: this will stream each change as currently proposed in
> > your patch. Then if we want to extend it for subscriber-side testing
> > then we can introduce new options like client_serialize for the case
> > being discussed in the email [1].
> >
> > Thoughts?
>
> There is potential for lots of developer GUCs for testing/debugging in
> the area of logical replication but IMO it might be better to keep
> them all separated. Putting everything into a single
> 'logical_replication_mode' might cause difficulties later when/if you
> want combinations of the different modes.

I think we want the developer option that forces streaming changes
during logical decoding to be PGC_USERSET but probably the developer
option for testing the parallel apply feature would be PGC_SIGHUP.
Also, since streaming changes is not specific to logical replication
but to logical decoding, I'm not sure logical_replication_XXX is a
good name. IMO having force_stream_mode and a different GUC for
testing the parallel apply feature makes sense to me.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com