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

On Tue, Nov 19, 2019 at 5:23 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Nov 18, 2019 at 5:02 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Fri, Nov 15, 2019 at 4:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Nov 15, 2019 at 4:01 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > > > On Fri, Nov 15, 2019 at 3:50 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > >
> > > > >
> > > > > Few other comments on this patch:
> > > > > 1.
> > > > > + case REORDER_BUFFER_CHANGE_INVALIDATION:
> > > > > +
> > > > > + /*
> > > > > + * Execute the invalidation message locally.
> > > > > + *
> > > > > + * XXX Do we need to care about relcacheInitFileInval and
> > > > > + * the other fields added to ReorderBufferChange, or just
> > > > > + * about the message itself?
> > > > > + */
> > > > > + LocalExecuteInvalidationMessage(&change->data.inval.msg);
> > > > > + break;
> > > > >
> > > > > Here, why are we executing messages individually? Can't we just
> > > > > follow what we do in DecodeCommit which is to record the invalidations
> > > > > in ReorderBufferTXN as we encounter them and then allow them to
> > > > > execute on each REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID. Is there a
> > > > > reason why we don't do ReorderBufferXidSetCatalogChanges when we
> > > > > receive any invalidation message?
> >
> > I think it's fine to call ReorderBufferXidSetCatalogChanges, only on
> > commit. Because this is required to add any committed transaction to
> > the snapshot if it has done any catalog changes.
> >
>
> Hmm, this is also used to build cid hash map (see
> ReorderBufferBuildTupleCidHash) which we need to use while streaming
> changes for the in-progress transactions. So, I think that it would
> be required earlier (before commit) as well.
>
Oh right, I guess I missed that part.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com