On Fri, Jul 5, 2019 at 5:20 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jul 4, 2019 at 5:23 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Mon, Jul 1, 2019 at 1:24 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > > > > > > Another small change/review: the function UndoLogGetNextInsertPtr() > > > previously took a transaction ID, but I'm not sure if that made sense, > > > I need to think about it some more. > > > > > > > The changes you have made related to UndoLogGetNextInsertPtr() doesn't > > seem correct to me. > > > > @@ -854,7 +854,9 @@ FindUndoEndLocationAndSize(UndoRecPtr start_urecptr, > > * has already started in this log then lets re-fetch the undo > > * record. > > */ > > - next_insert = UndoLogGetNextInsertPtr(slot->logno, uur->uur_xid); > > + next_insert = UndoLogGetNextInsertPtr(slot->logno); > > + > > + /* TODO this can't happen */ > > if (!UndoRecPtrIsValid(next_insert)) > > > > I think this is a possible case. Say while the discard worker tries > > to register the rollback request from some log and after it fetches > > the undo record corresponding to start location in this function, > > another backend adds the new transaction undo. The same is mentioned > > in comments as well. Can you explain what makes you think that this > > can't happen? If we don't want to pass the xid to > > UndoLogGetNextInsertPtr, then I think we need to get the insert > > location before fetching the record. I will think more on it to see > > if there is any other problem with the same. > > > > Pushed the fixed on above lines in the undoprocessing branch. >
Just in case anyone wants to look at the undoprocessing branch, it is available at https://github.com/EnterpriseDB/zheap/tree/undoprocessing -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com