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

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.

> > I will work on
> > that soon.
> >
>
> Cool, I think having a regression test for this will be a good idea.

ok

> @@ -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. However, we haven't done any activity for that
transaction so we don't need to execute the invalidations same as the
code before, but assert is no more valid.

> Apart from above, I have made a number of changes in
> 0002-WAL-Log-invalidations-at-command-end-with-wal_le to remove some
> unnecessary changes, edited comments, ran pgindent and updated the
> commit message. If you are fine with these changes, then do include
> them in your next version.

Thanks, I will check those.

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