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

On Thu, Jul 9, 2020 at 8:55 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Thu, Jul 9, 2020 at 8:47 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Jul 9, 2020 at 8:18 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> > >
> > > On Thu, Jul 9, 2020 at 12:28 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >>
> > >> On Wed, Jul 8, 2020 at 7:31 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> > >>
> > >> Thanks for showing the interest in patch. How have you ensured that
> > >> streaming is happening? I don't think the proposed patch can ensure
> > >> it for every case because we also rely on logical_decoding_work_mem to
> > >> decide whether to stream/spill, see ReorderBufferCheckMemoryLimit. I
> > >> think with your patch it will allow streaming for cases where we have
> > >> large amount of WAL to decode.
> > >>
> > >
> > > Maybe I missed something but I looked at ReorderBufferCheckMemoryLimit, even there it checks the same function ReorderBufferCanStream () and decides whether to stream or spill. Did I miss something?
> > >
> > > while (rb->size >= logical_decoding_work_mem * 1024L)
> > > {
> >
> > There is a check before above loop:
> >
> > ReorderBufferCheckMemoryLimit(ReorderBuffer *rb)
> > {
> > ReorderBufferTXN *txn;
> >
> > /* bail out if we haven't exceeded the memory limit */
> > if (rb->size < logical_decoding_work_mem * 1024L)
> > return;
> >
> > This will prevent the streaming/spill to occur.
>
> I think if the GUC is set then maybe we can bypass this check so that
> it can try to stream every single change?
>

Yeah and probably we need to do something for the check "while
(rb->size >= logical_decoding_work_mem * 1024L)" as well.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com