Re: POC: Cleaning up orphaned files using undo logs - Mailing list pgsql-hackers

Amit Kapila <amit.kapila16@gmail.com> wrote:

> On Thu, Nov 12, 2020 at 2:45 PM Antonin Houska <ah@cybertec.at> wrote:
> >
> >
> > No background undo
> > ------------------
> >
> > Reduced complexity of the patch seems to be the priority at the moment. Amit
> > suggested that cleanup of an orphaned relation file is simple enough to be
> > done on foreground and I agree.
> >
>
> Yeah, I think we should try and see if we can make it work but I
> noticed that there are few places like AbortOutOfAnyTransaction where
> we have the assumption that undo will be executed in the background.
> We need to deal with it.

I think this is o.k. if we always check for unapplied undo during startup.

> > "undo worker" is still there, but it only processes undo requests after server
> > restart because relation data can only be changed in a transaction - it seems
> > cleaner to launch a background worker for this than to hack the startup
> > process.
> >
>
> But, I see there are still multiple undoworkers that are getting
> launched and I am not sure if that works correctly because a
> particular undoworker is connected to a database and then it starts
> processing all the pending undo.

Each undo worker applies only transactions for its own database, see
ProcessExistingUndoRequests():

    /* We can only process undo of the database we are connected to. */
    if (xact_hdr.dboid != MyDatabaseId)
        continue;

Nevertheless, as I've just mentioned in my response to Thomas, I admit that we
should try to live w/o the undo worker altogether.

> > Discarding
> > ----------
> >
> > There's no discard worker for the URS infrastructure yet. I thought about
> > discarding the undo log during checkpoint, but checkpoint should probably do
> > more straightforward tasks than the calculation of a new discard pointer for
> > each undo log, so a background worker is needed. A few notes on that:
> >
> >   * until the zheap AM gets added, only the transaction that creates the undo
> >     records needs to access them. This assumption should make the discarding
> >     algorithm a bit simpler. Note that with zheap, the other transactions need
> >     to look for old versions of tuples, so the concept of oldestXidHavingUndo
> >     variable is needed there.
> >
> >   * it's rather simple to pass pointer the URS pointer to the discard worker
> >     when transaction either committed or the undo has been executed.
> >
>
> Why can't we have a separate discard worker which keeps on scanning
> the undorecords and discard accordingly? Giving the onus of foreground
> process might be tricky because say discard worker is not up to speed
> and we ran out of space to pass such information for each commit/abort
> request.

Sure, there should be a discard worker. The question is how to make its work
efficient. The initial run after restart probably needs to scan everything
between 'discard' and 'insert' pointers, but then it should process only the
parts created by individual transactions.

> >
> > Do not execute the same undo record multiple times
> > --------------------------------------------------
> >
> > Although I've noticed in the zheap code that it checks whether particular undo
> > action was already undone, I think this functionality fits better in the URS
> > layer.
> >
>
> If you want to track at undo record level, then won't it lead to
> performance overhead and probably additional WAL overhead considering
> this action needs to be WAL-logged. I think recording at page-level
> might be a better idea.

I'm not worried about WAL because the undo execution needs to be WAL-logged
anyway - see smgr_undo() in the 0005- part of the patch set. What needs to be
evaluated regarding performance is the (exclusive) locking of the page that
carries the progress information. I'm still not sure whether this info should
be on every page or only in the chunk header. In either case, we have a
problem if there are two or more chunks created by different transactions on
the same page, and if more than on of these transactions need to perform
undo. I tend to believe that this should happen rarely though.

> > I can spend more time on this project, but need a hint which part I should
> > focus on.
> >
>
> I can easily imagine that this needs a lot of work and I can try to
> help with this as much as possible from my side. I feel at this stage
> we should try to focus on undo-related work (to start with you can
> look at finishing the undoprocessing work for which I have shared some
> thoughts) and then probably at some point in time we need to rebase
> zheap over this.

I agree, thanks!

--
Antonin Houska
Web: https://www.cybertec-postgresql.com