On Thu, Dec 22, 2022 5:24 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
> >
> > At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila
> <amit.kapila16@gmail.com> wrote in
> > > I have addressed these comments in the attached. Additionally, I have
> > > modified the docs and commit messages to make those clear. I think
> > > instead of adding new tests with this patch, it may be better to
> > > change some of the existing tests related to streaming to use this
> > > parameter as that will clearly show one of the purposes of this patch.
> >
> > Being late but I'm warried that we might sometime regret that the lack
> > of the explicit default. Don't we really need it?
> >
>
> For this, I like your proposal for "buffered" as an explicit default value.
>
> > + Allows streaming or serializing changes immediately in logical
> decoding.
> > + The allowed values of <varname>logical_decoding_mode</varname>
> are the
> > + empty string and <literal>immediate</literal>. When set to
> > + <literal>immediate</literal>, stream each change if
> > + <literal>streaming</literal> option is enabled, otherwise, serialize
> > + each change. When set to an empty string, which is the default,
> > + decoding will stream or serialize changes when
> > + <varname>logical_decoding_work_mem</varname> is reached.
> >
> > With (really) fresh eyes, I took a bit long time to understand what
> > the "streaming" option is. Couldn't we augment the description by a
> > bit?
> >
>
> Okay, how about modifying it to: "When set to
> <literal>immediate</literal>, stream each change if
> <literal>streaming</literal> option (see optional parameters set by
> CREATE SUBSCRIPTION) is enabled, otherwise, serialize each change.
>
I updated the patch to use "buffered" as the explicit default value, and include
Amit's changes about document.
Besides, I tried to reduce data size in streaming subscription tap tests by this
new GUC (see 0002 patch). But I didn't covert all streaming tap tests because I
think we also need to cover the case that there are lots of changes. So, 015* is
not modified. And 017* is not modified because streaming transactions and
non-streaming transactions are tested alternately in this test.
I collected the time to run these tests before and after applying the patch set
on my machine. In debug version, it saves about 5.3 s; and in release version,
it saves about 1.8 s. The time of each test is attached.
Regards,
Shi yu
