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

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: Erik Rijkers <er(at)xs4all(dot)nl>, Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions
Date: 2020-06-22 11:55:58
Message-ID: CAA4eK1+K+_6UT_WSYGsgodS96mYZ__Bs_69U5J_JwckK+sE9Ag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 22, 2020 at 4:41 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Mon, Jun 22, 2020 at 4:26 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Jun 18, 2020 at 9:02 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > Yes, I have made the changes. Basically, now I am only using the
> > > XLOG_XACT_INVALIDATIONS for generating all the invalidation messages.
> > > So whenever we are getting the new set of XLOG_XACT_INVALIDATIONS, we
> > > are directly appending it to the txn->invalidations. I have tested
> > > the XLOG_INVALIDATIONS part but while sending this mail I realized
> > > that we could write some automated test for the same.
> > >
> >
> > Can you share how you have tested it?
>
> I just ran create index concurrently and decoded the changes.
>

Hmm, I think that won't reproduce the exact problem. What I wanted
was to run another command after "create index concurrently" which
depends on that and see if the decoding fails by removing the
XLOG_INVALIDATIONS code. Once you get some failure, you can apply the
0002 patch and see if the test is passed?

>
> > @@ -2012,8 +2014,6 @@ ReorderBufferForget(ReorderBuffer *rb,
> > TransactionId xid, XLogRecPtr lsn)
> > if (txn->base_snapshot != NULL && txn->ninvalidations > 0)
> > ReorderBufferImmediateInvalidation(rb, txn->ninvalidations,
> > txn->invalidations);
> > - else
> > - Assert(txn->ninvalidations == 0);
> >
> > Why this Assert is removed?
>
> Even if the base_snapshot is NULL, now we are collecting the
> txn->invalidation.
>

But there doesn't seem to be any check even before this patch which
directly prohibits accumulating invalidations in DecodeCommit. We
have check for base_snapshot in ReorderBufferCommit. Did you get any
failure with that check?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-06-22 12:01:16 Re: min_safe_lsn column in pg_replication_slots view
Previous Message Ashutosh Bapat 2020-06-22 11:55:42 Re: suggest to rename enable_incrementalsort