On Wed, May 1, 2019 at 9:29 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, May 1, 2019 at 6:02 AM Robert Haas <robertmh...@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 <robertmh...@gmail.com> wrote: > > > > > > On Tue, Apr 30, 2019 at 2:16 AM Dilip Kumar <dilipbal...@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
