On Tue, Aug 6, 2019 at 1:26 PM Andres Freund <and...@anarazel.de> wrote: > > Hi, > > On 2019-08-05 11:29:34 -0700, Andres Freund wrote: > > Need to do something else for a bit. More later. > > > + * false, otherwise. > > + */ > > +static bool > > +UndoAlreadyApplied(FullTransactionId full_xid, UndoRecPtr to_urecptr) > > +{ > > + UnpackedUndoRecord *uur = NULL; > > + UndoRecordFetchContext context; > > + > > + /* Fetch the undo record. */ > > + BeginUndoFetch(&context); > > + uur = UndoFetchRecord(&context, to_urecptr); > > + FinishUndoFetch(&context); > > Literally all the places that fetch a record, fetch them with exactly > this combination of calls. If that's the pattern, what do we gain by > this split? Note that UndoBulkFetchRecord does *NOT* use an > UndoRecordFetchContext, for reasons that are beyond me.
Actually, for the zheap or any other AM, where it needs to traverse the transactions undo the chain. For example, in zheap we will get the latest undo record pointer from the slot but we need to traverse the undo record chain backward using the prevundo pointer store in the undo record and find the undo record for a particular tuple. Earlier, there was a loop in UndoFetchRecord which were traversing the chain of the undo until it finds the matching record and record was matched using a callback. There was also an optimization that if the current record doesn't satisfy the callback then we keep the pin hold on the buffer and go to the previous record in the chain. Later based on the review comments by Robert we have decided that finding the matching undo record should be caller's responsibility so we have moved the loop out of the UndoFetchRecord and kept it in the zheap code. The reason for keeping the context is that we can keep the buffer pin held and remember that buffer in the context so that the caller can call UndoFetchRecord in a loop and the pin will be held on the buffer from which we have read the last undo record. I agree that in undoprocessing patch set we always need to fetch one record so instead of repeating this pattern everywhere we can write one function and move this sequence of calls in that function. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com