On Thu, Dec 22, 2022 5:24 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Dec 22, 2022 at 1:15 PM Kyotaro Horiguchi > <horikyota....@gmail.com> wrote: > > > > At Thu, 22 Dec 2022 12:35:46 +0530, Amit Kapila > <amit.kapil...@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
v5-0001-Add-logical_decoding_mode-GUC.patch
Description: v5-0001-Add-logical_decoding_mode-GUC.patch
v5-0002-Converting-streaming-tap-tests-by-using-logical_d.patch
Description: v5-0002-Converting-streaming-tap-tests-by-using-logical_d.patch
The unit is milliseconds. - debug +---------+-------+---------+ | test_no | HEAD | patched | +---------+-------+---------+ | 016 | 3425 | 3118 | | 018 | 4873 | 3159 | | 019 | 3241 | 3116 | | 022 | 8273 | 7373 | | 023 | 7156 | 4823 | | | | | | sum | 26968 | 21589 | +---------+-------+---------+ - release +---------+-------+---------+ | test_no | HEAD | patched | +---------+-------+---------+ | 016 | 1776 | 1649 | | 018 | 2248 | 1689 | | 019 | 1653 | 1648 | | 022 | 4858 | 4462 | | 023 | 3992 | 3292 | | | | | | sum | 14527 | 12740 | +---------+-------+---------+
