Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions

On Thu, Oct 24, 2019 at 7:07 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Oct 22, 2019 at 10:30 AM Dilip Kumar <dilipbalaut(at)gmail(dot)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.

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