Re: Hot Standby 0.2.1 - Mailing list pgsql-hackers
From | Simon Riggs |
---|---|
Subject | Re: Hot Standby 0.2.1 |
Date | |
Msg-id | 1253874386.4449.602.camel@ebony.2ndQuadrant Whole thread Raw |
In response to | Re: Hot Standby 0.2.1 (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>) |
List | pgsql-hackers |
On Fri, 2009-09-25 at 11:49 +0300, Heikki Linnakangas wrote: > Looking at the startup sequence now. > > I see that you modified ExtendSUBTRANS so that it doesn't wipe out > previously set values if it's called with out-of-order xids. I guess > that works, although I think it can leave pages unzeroed if it's called > with a large enough gap between xids, so that the previous one was on > e.g page 10 and the next one on page 12. Page 11 would not be zeroed, > AFAICS. Not sure if such big gaps in the xid sequence are possible, but > seems so if you have a very large max_connections setting and a lot of > subtransactions. Yeh, it happens and I've seen it - which is why the code is there. > Nevertheless, I'd feel better if we kept ExtendSUBTRANS unmodified, and > instead arranged things so that ExtendSUBTRANS is always called in > xid-order. We can call it from RecordKnownAssignedTransactionIds, which > handles the out-of-order problem for other purposes already. > > I think we have similar problem with clog. ExtendCLOG is currently not > called during recovery, so isn't it possible for the problem with > subtransaction commits that's described in the comments in StartupCLOG > to arise during hot standby? Again, I think we should call ExtendCLOG() > in RecordKnownAssignedTransactionIds. RecordKnownAssignedTransactionIds > is the hot standby version of GetNewTransactionId(), so to speak. OK. We record xids in sequence, so it is now a much more natural place to do this. Zeroing them makes them dirty, unfortunately, but that is another issue. RecordKnownAssignedTransactionIds() only works once the snapshot has been initialised. That could leave a gap, so we will need to run it continuously if InHotStandby. Which may have knock-on changes also. > If we go with that, I think we'll need to call StartupSUBTRANS/CLOG > earlier in the startup sequence too, when we establish the startup > snapshot and let backends in. Yes, I think an earlier patch had that, but it turns out that there really isn't anything for those functions to do. Really we could rename those functions EndOfRecoverySUBTRANS and EndOfRecoveryCLOG to illustrate their role better. > - I removed the new DBConsistentUpToLSN() function and moved its > functionality into XLogNeedsFlush(). Since XLogFlush() updates > minRecoveryPoint instead of flushing WAL during recovery, it makes sense > for XLogNeedsFlush() to correspondingly check if minRecoveryPoint needs > to be updated when in recovery. That's a better definition for the call > in bufmgr.c too. > > - I went ahead with the changes with RecoveryInfoLock and tracking the > number of held AccessExclusive locks in lmgr.c instead of proc array. > > Can we find a better name for "loggable locks"? It means locks that need > to be WAL logged when acquired, for hot standby, and "loggable lock" > doesn't sound right for that. "Loggable" implies that it can be logged, > but it really means that it *must* be logged. The distinction of "loggable" is somewhat false since we rely on the fact that only one person is holding it. So we may as well just call em what they are: AccessExclusiveLocks. > Keep an eye on my git repository for latest changes. No, I'm not doing that. If you want to submit changes, please do so to the list or just mention what needs work and I will do it. Having multiple versions of a patch isn't helpful, as I have already said. I have already been burned multiple times by accepting trial code and you yourself have re-written particular parts multiple times. I am very, very grateful for your reviews and detailed suggestions, so this comment is only about coherency and not tripping each other up. (If you want to "editorialize", it needs to be just prior to commit, but making changes to a patch just prior to commit has historically shown to introduce bugs where there weren't any before). There's enough changes already to demand a full re-test, so everything discovered so far plus testing is about 2 weeks work. I will commit things onto git as agreed as I complete coding but that won't imply full testing. -- Simon Riggs www.2ndQuadrant.com
pgsql-hackers by date: