On Fri, Aug 9, 2019 at 1:57 AM Robert Haas <robertmh...@gmail.com> wrote: > > On Thu, Aug 8, 2019 at 9:31 AM Andres Freund <and...@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
