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

On Fri, Nov 15, 2019 at 4:19 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Nov 15, 2019 at 4:01 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Fri, Nov 15, 2019 at 3:50 PM Amit Kapila <amit.kapila16@gmail.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.  So I think there is
no point in setting that flag every time we get an invalidation
message.


> > IMHO, the reason is that in DecodeCommit, we get all the invalidation
> > at one time so, at REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID, we don't
> > know which invalidation message to execute so for being safe we have
> > to execute all.  But, since we are logging all invalidation
> > individually, we exactly know at this stage which cache to invalidate.
> > So it is better to only invalidate required cache not all.
> >
>
> In that case, invalidations can be processed multiple times, the first
> time when these individual WAL logs for invalidation are processed and
> then later at commit time when we accumulate all invalidation messages
> and then execute them for REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID.
> Can we avoid to execute invalidations from other places after this
> patch which also includes executing them as part of XLOG_INVALIDATIONS
> processing?
I think we can avoid invalidation which is done as part of
REORDER_BUFFER_CHANGE_INTERNAL_COMMAND_ID.  I need to further
investigate the invalidation which is done as part of
XLOG_INVALIDATIONS.

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