On Wed, May 1, 2019 at 9:29 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, May 1, 2019 at 6:02 AM Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > Replying to myself to resend to the list, since my previous attempt
> > seems to have been eaten by a grue.
> >
> > On Tue, Apr 30, 2019 at 11:14 AM Robert Haas <robertmhaas@gmail.com> wrote:
> > >
> > > On Tue, Apr 30, 2019 at 2:16 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > Like previous version these patch set also applies on:
> > > > https://github.com/EnterpriseDB/zheap/tree/undo
> > > > (b397d96176879ed5b09cf7322b8d6f2edd8043a5)
> > >
> > > Some more review of 0003:
> > >
>
> Another suggestion:
>
> +/*
> + * Insert a previously-prepared undo records. This will write the actual undo
> + * record into the buffers already pinned and locked in PreparedUndoInsert,
> + * and mark them dirty. This step should be performed after entering a
> + * criticalsection; it should never fail.
> + */
> +void
> +InsertPreparedUndo(void)
> +{
> ..
> ..
> +
> + /* Advance the insert pointer past this record. */
> + UndoLogAdvance(urp, size);
> + }
> ..
> }
>
> UndoLogAdvance internally takes LWLock and we don't recommend doing
> that in the critical section which will happen as this function is
> supposed to be invoked in the critical section as mentioned in
> comments.
I think we can call UndoLogAdvanceFinal in FinishUndoRecordInsert
because this function will be called outside the critical section.
And, now we already have the undo record size inside
UndoRecordInsertContext.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com