Re: POC: Cleaning up orphaned files using undo logs

On Fri, Aug 9, 2019 at 1:57 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Aug 8, 2019 at 9:31 AM Andres Freund <andres@anarazel.de> wrote:
> > I know that Robert is working on a patch that revises the undo request
> > layer somewhat, it's possible that this is best discussed afterwards.
>
> Here's what I have at the moment.  This is not by any means a complete
> replacement for Amit's undo worker machinery, but it is a significant
> redesign (and I believe a significant improvement to) the queue
> management stuff from Amit's patch.  I wrote this pretty quickly, so
> while it passes simple testing, it probably has a number of bugs, and
> to actually use it, it would need to be integrated with xact.c; right
> now it's just a standalone module that doesn't do anything except let
> itself be tested.
>
> Some of the ways it is different from Amit's patches include:
>
> * It uses RBTree rather than binaryheap, so when we look ahead, we
> look ahead in the right order.
>
> * There's no limit to the lookahead distance; when looking ahead, it
> will search the entirety of all 3 RBTrees for an entry from the right
> database.
>
> * It doesn't have a separate hash table keyed by XID.  I didn't find
> that necessary.
>
> * It's better-isolated, as you can see from the fact that I've
> included a test module that tests this code without actually ever
> putting an UndoRequestManager in shared memory.  I would've liked to
> expand this test module, but I don't have time to do that today and
> felt it better to get this much sent out.
>
> * It has a lot of comments explaining the design and how it's intended
> to integrate with the rest of the system.
>
> Broadly, my vision for how this would get used is:
>
> - Create an UndoRecordManager in shared memory.
> - Before a transaction first attaches to a permanent or unlogged undo
> log, xact.c would call RegisterUndoRequest(); thereafter, xact.c would
> store a pointer to the UndoRecord for the lifetime of the toplevel
> transaction.

So, for top-level transactions rollback, we can directly refer from
UndoRequest *, the start and end locations.  But, what should we do
for sub-transactions (rollback to savepoint)?  One related point is
that we also need information about last_log_start_undo_location to
update the undo apply progress (The basic idea is if the transactions
undo is spanned across multiple logs, we update the progress in each
of the logs.).  We can remember that in the transaction state or
undorequest *.  Any suggestion?

> - Immediately after attaching to a permanent or unlogged undo log,
> xact.c would call UndoRequestSetLocation.
> - xact.c would track the number of bytes of permanent and unlogged
> undo records the transaction generates.  If the transaction goes onto
> abort, it reports these by calling FinalizeUndoRequest.
> - If the transaction commits, it doesn't need that information, but
> does need to call UnregisterUndoRequest() as a post-commit step in
> CommitTransaction().
>

IIUC, for each transaction, we have to take a lock first time it
attaches to a log and then the same lock at commit time.  It seems the
work under lock is less, but still, can't this cause a contention?  It
seems to me this is similar to what we saw in ProcArrayLock where work
under lock was few instructions, but acquiring and releasing the lock
by each backend at commit time was causing a bottleneck.

It might be due to some reason this won't matter in a similar way in
which case we can find that after integrating it with other patches
from undo processing machinery and rebasing zheap branch over it?

How will computation of oldestXidHavingUnappliedUndo will work?

We can probably check the fxid queue and error queue to get that
value.  However, I am not sure if that is sufficient because incase we
perform the request in the foreground, it won't be present in queues.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com