Re: PATCH: logical_work_mem and logical streaming of largein-progress transactions

On Mon, Nov 4, 2019 at 3:46 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, Oct 24, 2019 at 7:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, Oct 22, 2019 at 10:30 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > I have merged bugs_and_review_comments_fix.patch changes to 0001 and 0002.
> > >
> >
> > I was wondering whether we have checked the code coverage after this
> > patch?  Previously, the existing tests seem to be covering most parts
> > of the function ReorderBufferSerializeTXN [1].  After this patch, the
> > timing to call ReorderBufferSerializeTXN will change, so that might
> > impact the testing of the same.  If it is already covered, then I
> > would like to either add a new test or extend existing test with the
> > help of new spill counters.  If it is not getting covered, then we
> > need to think of extending the existing test or write a new test to
> > cover the function ReorderBufferSerializeTXN.
> >
> I have run the tests with coverage and found that
> ReorderBufferSerializeTXN is not being hit.
> The reason it is not being hit is because of the following check in
> ReorderBufferCheckMemoryLimit:
>     /* bail out if we haven't exceeded the memory limit */
>     if (rb->size < logical_decoding_work_mem * 1024L)
>         return;
> Previously the tests from contrib/test_decoding could hit
> ReorderBufferSerializeTXN function.
> I'm checking if we can modify the test or add new test to hit
> ReorderBufferSerializeTXN function.

I have made one change to the configuration file in
contrib/test_decoding directory, with that the coverage seems to be
fine. I have seen that the coverage is almost like the code before
applying the patch. I have attached the test change and the coverage
report for reference. Coverage report includes the core logical work
memory files for base code and by applying
0001-Add-logical_decoding_work_mem-to-limit-ReorderBuffer and
0002-Track-statistics-for-spilling patches.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com