Re: POC: Cleaning up orphaned files using undo logs

On Fri, Jul 5, 2019 at 5:20 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Jul 4, 2019 at 5:23 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Jul 1, 2019 at 1:24 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)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