I attach a series of proposed patches to slightly improve some minor things related to the CLOG code.
0001 - Always call StartupCLOG() just after initializing ShmemVariableCache. Right now, the hot_standby=off code path does this at end of recovery, and the hot_standby=on code path does it at the beginning of recovery. It's better to do this in only one place because (a) it's simpler, (b) StartupCLOG() is trivial so trying to postpone it in the hot_standby=off case has no value, and (c) it allows for 0002 and therefore 0003, which make things even simpler. 0002 - In clog_redo(), don't set XactCtl->shared->latest_page_number. The value that is being set here is actually the oldest page we're not truncating, not the newest page that exists, so it's a bogus value (except when we're truncating all but one page). The reason it's like this now is to avoid having SimpleLruTruncate() see an uninitialized value that might trip a sanity check, but after 0001 that won't be possible, so we can just let the sanity check do its thing. 0003 - In TrimCLOG(), don't reset XactCtl->shared->latest_page_number. After we stop doing 0002 we don't need to do this either, because the only thing this can be doing for us is correcting the error introduced by the code which 0002 removes. Relying on the results of replaying (authoritative) CLOG/EXTEND records seems better than relying on our (approximate) value of nextXid at end of recovery. 0004 - In StartupCLOG(), correct an off-by-one error. Currently, if the nextXid is exactly a multiple of the number of CLOG entries that fit on a page, then the value we compute for XactCtl->shared->latest_page_number is higher than it should be by 1. That's because nextXid represents the first XID that hasn't yet been allocated, not the last one that gets allocated. So, the CLOG page gets created when nextXid advances from the first value on the page to the second value on the page, not when it advances from the last value on the previous page to the first value on the current page. Note that all of 0001-0003 result in a net removal of code. 0001 comes out to more lines total because of the comment changes, but fewer executable lines. I don't plan to back-patch any of this because, AFAICS, an incorrect value of XactCtl->shared->latest_page_number has no real consequences. The SLRU code uses latest_page_number for just two purposes. First, the latest page is never evicted; but that's just a question of performance, not correctness, and the performance impact is small. Second, the sanity check in SimpleLruTruncate() uses it. The present code can make the value substantially inaccurate during recovery, but only in a way that can make the sanity check pass rather than failing, so it's not going to really bite anybody except perhaps if they have a corrupted cluster where they would have liked the sanity check to catch some problem. When not in recovery, the value can be off by at most one. I am not sure whether there's a theoretical risk of this making SimpleLruTruncate()'s sanity check fail when it should have passed, but even if there is the chances must be extremely remote. Some of the other SLRUs have similar issues as a result of copy-and-paste work over the years. I plan to look at tidying that stuff up, too. However, I wanted to post (and probably commit) these patches first, partly to get some feedback, and also because all the cases are a little different and I want to make sure to do a proper analysis of each one. Any review very much appreciated. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
v1-0003-In-TrimCLOG-don-t-reset-XactCtl-shared-latest_pag.patch
Description: Binary data
v1-0002-In-clog_redo-don-t-set-XactCtl-shared-latest_page.patch
Description: Binary data
v1-0004-In-StartupCLOG-correct-an-off-by-one-error.patch
Description: Binary data
v1-0001-Move-StartupCLOG-calls-to-just-after-we-initializ.patch
Description: Binary data
