Hi, The global variable ThisTimeLineID is rather confusing. During recovery, in the startup process, when we're reading a WAL record, it is the timeline of the WAL record we are trying to read or have just read, except when we're trying to read the initial checkpoint record, when it's zero. In other processes, it's typically 0 throughout recovery, but sometimes it's not, because for example CreateRestartPoint temporarily sets it to the timeline from which we're replaying WAL, since there's no other way to get RemoveOldXlogFiles() to recycle files onto the right timeline, or PreallocXlogFiles() to preallocate onto the right timeline. Similarly, walreceiver and walsender find it convenient to make ThisTimeLineID the timeline from which WAL is being replayed or at least the one from which it was being replayed at last check. logicalfuncs.c and slotfuncs.c set the global variable sometimes too, apparently just for fun, as the value doesn't seem to get used for anything. In normal running, once the startup process exits, the next call to RecoveryInProgress() sets the value to the timeline into which new WAL can be inserted. Note also that all of this is different from XLogCtl->ThisTimeLineID, which is set at the end of recovery and thus, in the startup process, generally different from ThisTImeLineID, but in other processes, generally the same as ThisTimeLineID.
At the risk of stating the obvious, using the same variable for different purposes at different times is not a great programming practice. Commits 2f5c4397c39dea49c5608ba583868e26d767fc32 and 902a2c280012557b85c7e0fce3f6f0e355cb2d69 show that the possibility of programmer error is not zero, even though neither of those issues are serious. Moreover, the global variable itself seems to do nothing but invite programming errors. The name itself is a disaster. What is "this" timeline ID? Well, uh, the right one, of course! We should be more clear about what we mean: the timeline into which we are inserting, the timeline from which we are replaying, the timeline from which we are performing logical decoding. I suspect that having a global variable here isn't even really saving us anything much, as a value that does not change can be read from shared memory without a lock. I would like to clean this up. Attached is a series of patches which try to do that. For ease of review, this is separated out into quite a few separate patches, but most likely we'd combine some of them for commit. Patches 0001 through 0004 eliminate all use of the global variable ThisTimeLineID outside of xlog.c, and patch 0005 makes it "static" so that it ceases to be visible outside of xlog.c. Patches 0006 to 0010 clean up uses of ThisTimeLineID itself, passing it around as a function parameter, or otherwise arranging to fetch the relevant timeline ID from someplace sensible rather than using the global variable as a scratchpad. Finally, patch 0011 deletes the global variable. I have not made a serious attempt to clear up all of the terminological confusion created by the term ThisTimeLineID, which also appears as part of some structs, so you'll still see that name in the source code after applying these patches, just not as the name of a global variable. I have, however, used names like insertTLI and replayTLI in many places changed by the patch, and I think that makes it significantly more clear which timeline is being discussed in each case. In some places I have endeavored to improve the comments. There is doubtless more that could be done here, but I think this is a fairly respectable start. Opinions welcome. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
0003-walsender.c-Don-t-rely-on-the-global-variable-ThisTi.patch
Description: Binary data
0004-walreceiver.c-Don-t-rely-on-the-global-variable-This.patch
Description: Binary data
0002-Add-GetWALInsertionTimeLine-also-return-it-via-GetFl.patch
Description: Binary data
0001-Don-t-set-ThisTimeLineID-when-there-s-no-reason-to-d.patch
Description: Binary data
0006-xlog.c-Change-assorted-functions-to-take-an-explicit.patch
Description: Binary data
0005-Change-ThisTimeLineID-to-be-static-rather-than-exter.patch
Description: Binary data
0009-xlog.c-Make-xlog_redo-not-depend-on-ThisTimeLineID-g.patch
Description: Binary data
0008-xlog.c-Use-XLogCtl-ThisTimeLineID-in-various-places.patch
Description: Binary data
0010-xlog.c-Adjust-some-more-functions-to-pass-the-TLI-ar.patch
Description: Binary data
0007-xlog.c-Invent-openLogTLI.patch
Description: Binary data
0011-Demote-ThisTimeLineID-to-a-local-variable.patch
Description: Binary data
