Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions - Mailing list pgsql-hackers

From Dilip Kumar
Subject Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Date
Msg-id CAFiTN-vERgK+4Q60ox1kc8rXHO7rbVLcCiYC7=vTkV8AFz1oBg@mail.gmail.com
Whole thread Raw
In response to Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions  (Dilip Kumar <dilipbalaut@gmail.com>)
Responses Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
List pgsql-hackers
On Tue, Jun 30, 2020 at 10:13 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, Jun 30, 2020 at 9:20 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Mon, Jun 29, 2020 at 4:24 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Mon, Jun 22, 2020 at 4:30 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > > >
> > > >
> > > > Other than above tests, can we somehow verify that the invalidations
> > > > generated at commit time are the same as what we do with this patch?
> > > > We have verified with individual commands but it would be great if we
> > > > can verify for the regression tests.
> > >
> > > I have verified this using a few random test cases.  For verifying
> > > this I have made some temporary code changes with an assert as shown
> > > below.  Basically, on DecodeCommit we call
> > > ReorderBufferAddInvalidations function only for an assert checking.
> > >
> > > -void
> > >  ReorderBufferAddInvalidations(ReorderBuffer *rb, TransactionId xid,
> > >                                                           XLogRecPtr
> > > lsn, Size nmsgs,
> > > -
> > > SharedInvalidationMessage *msgs)
> > > +
> > > SharedInvalidationMessage *msgs, bool commit)
> > >  {
> > >         ReorderBufferTXN *txn;
> > >
> > >         txn = ReorderBufferTXNByXid(rb, xid, true, NULL, lsn, true);
> > > -
> > > +       if (commit)
> > > +       {
> > > +               Assert(txn->ninvalidations == nmsgs);
> > > +               return;
> > > +       }
> > >
> > > The result is that for a normal local test it works fine.  But with
> > > regression suit, it hit an assert at many places because if the
> > > rollback of the subtransaction is involved then at commit time
> > > invalidation messages those are not logged whereas with command time
> > > invalidation those are logged.
> > >
> >
> > Yeah, somehow, we need to ignore rollback to savepoint tests and
> > verify for others.
>
> Yeah, I have run the regression suite,  I can see a lot of failure
> maybe we can somehow see the diff and confirm that all the failures
> are due to rollback to savepoint only.  I will work on this.

I have compared the changes logged at command end vs logged at commit
time.  I have ignored the invalidation for the transaction which has
any aborted subtransaction in it.  While testing this I found one
issue, the issue is that if there are some invalidation generated
between last command counter increment and the commit transaction then
those were not logged.  I have fixed the issue by logging the pending
invalidation in RecordTransactionCommit.  I will include the changes
in the next patch set.

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

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [Patch] Invalid permission check in pg_stats for functional indexes
Next
From: Stephen Frost
Date:
Subject: Re: Ideas about a better API for postgres_fdw remote estimates