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. It will be available in the next set of patches we post. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com