Re: Hot standby, slot ids and stuff
Re: Hot standby, slot ids and stuff
От:
Simon Riggs <simon@2ndQuadrant.com>
Дата:
On Sun, 2009-01-11 at 10:41 +0200, Heikki Linnakangas wrote: > Simon Riggs wrote: > > There's a confusion in the patch between top level xid and parent xid. > > The xl field is named parentxid but actually contains top level. That > > distinction is important because the code now uses the top level xid to > > locate the recovery proc, formerly the role of the slotid. > > True, I changed the meaning halfway through. My thinking was that we can > get away by only tracking the top-level xact of each subtransaction, not > the whole tree of subtransactions. > > > This leads to an error when we SubTransSetParent(child_xid, top_xid); > > since this assumes that the top_xid is the parent, which it is not. > > Mostly you wouldn't notice unless you were looking up the subtrans > > status for an xid that had committed but was the child of an aborted > > subtransaction, with the top level xid having > 64 subtransactions. > > Hmm. When a subtransaction aborts, RecordTransactionAbort is called and > clog is updated to show the subtransaction and all it's subcommitted > children as aborted. I think we're safe, though it wouldn't hurt to > double-check. That part of your argument is correct but we are not safe. But please look at XactLockTableWait() and see what you think. According to comments this would lead to different lock release behaviour. Beauty, thy name is not subtransaction. If you agree that we need the parent xid then we have further problems. Adding another xid onto the header of each WAL record will add 8 bytes onto each record because of alignment. This was the other reason for slotid, since I was able to fit that into just 2 bytes and avoid the 8 byte loss. Moving swiftly on, I have this morning though of an alternative, so at least we now have some choice. Rather than store the parent xid itself we store the difference between the current xid and the parent xid. Typically this will be less than 65535; when it is not we set it to zero but issue an xid assignment xlog record. However, I think XactLockTableWait() doesn't need to know the parent either. (This feels more like wishful thinking, but here goes anyway). We release locks *after* TransactionIdAbortTree() has fully executed, so the test for TransactionIdIsInProgress(xid) will always see the abort status, if set. Notice that if we are awake at all it is because the top-level transaction is complete or our subxid is aborted. So there is never any need to look at the status of the parent, nor in fact any need to look at the procarray at all, which is always a waste of effort. If you believe that, you'll want to commit the attached patch (or something similar with comments refactored etc). > For xl_rel_lock you add a field called xid and then ask > > /* xid of the *parent* transaction. XXX why parent? */. > > You've done this because it replaced slotid. But looking at that, I > > think the 6a patch had a bug there: a subtransaction abort record would > > release locks for the whole top level xact. So we need to pass both top > > level xid (or slotid) and xid for each lock, then release using the > > actual xid only. > > Hmm, would just the subtransaction xid be enough? If we use that to > release, what do we need the top-level xid for? So we can release all locks taken by subxids at once, when we learn that a top level xid has disappeared. A minor point. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support
Hot standby, slot ids and stuff
От:
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Дата:
I've mentioned before that I don't like the slotid stuff. From an architectural point of view, we should try to keep the extra communication between primary and standby to a minimum, for the sake of robustness. The primary shouldn't have such a big role in keeping track of which xids it has already emitted in WAL records, which subxids have been marked, and so on. Since I've been whining about that for some time, I figured I have to put my money where my mouth is, so here's a patch based on v6a that eliminates the concept of slotids, as well as the new xl_info2 field in XLogRecord. This seems much simpler to me. I haven't given it much testing, but seems to work. There's a whole bunch of comments marked with XXX that need resolving, though. Attached is a patch agains CVS HEAD (hs-noslotid-1.patch) as well as an incremental patch against your v6a (hs-noslotid-against-v6a.patch). Sorry if you were just working on some big changes and I joggled your elbow. I also didn't check what changes you had in v7 yet. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
