Re: Force streaming every change in logical decoding

On Wed, Dec 7, 2022 at 7:31 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Wed, Dec 7, 2022 at 8:46 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > >
> > > 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.
>

Ideally, that is true but if we want to combine the multiple modes in
one parameter, is there a harm in keeping it as 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.
>

But if we want to have a separate variable for testing/debugging
streaming like force_stream_mode, why not for serializing as well? And
if we want for both then we can even think of combining them in one
variable as logical_decoding_mode with values as 'stream' and
'serialize'. The first one specified would be given preference. Also,
the name force_stream_mode doesn't seem to convey that it is for
logical decoding. We can probably have a separate variable for the
subscriber side.

On one side having separate GUCs for publisher and subscriber seems to
give better flexibility but having more GUCs also sometimes makes them
less usable. Here, my thought was to have a single or as few GUCs as
possible which can be extendible by providing multiple values instead
of having different GUCs. I was trying to map this with the existing
string parameters in developer options.

--
With Regards,
Amit Kapila.