Re: POC: Cleaning up orphaned files using undo logs

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