On Tue, Nov 02, 2021 at 08:45:57AM -0400, Robert Haas wrote: > Also, XLogCtl->ThisTimeLineID isn't set until the end of recovery, so > calling this function during recovery would be a mistake. There seem > to be a number of interface functions in xlog.c that should only be > called when not in recovery, and neither their names nor their > comments make that clear. I wasn't bold enough to go label all the > ones I think fall into that category, but maybe we should.
I got to wonder whether it would be better to add in GetFlushRecPtr() the same assertion as GetWALInsertionTimeLine(). All the in-core callers of this routine already assume that, and it would be buggy if one passes down insertTLI to get the current TLI. > I thought about that, but I think it's pointless. I think we can just > say that if openLogFile == -1, openLogSegNo and openLogTLI are > meaningless. I don't think we're currently resetting openLogSegNo to > an invalid value either, so I don't think openLogTLI should be treated > differently. Wouldn't it be better to at least document that as a comment rather than letting the reader guess it, then? I agree that this is a minor point, though. > I'm not opposed to introducing InvalidTimeLineID on > principle, and I looked for a way of doing it in this patch set, but > it didn't seem all that valuable, and I feel that someone who cares > about it can do it as a separate effort after I commit these. No issues from me. I have bumped into a case just at the end of last week where I wanted a better way to tell if a TLI is invalid rather than leaving things to 0, FWIW. >> The handling of XLogCtl->ThisTimeLineID still seems a bit blurry when >> it comes to timeline switches because we don't update it while in >> recovery when replaying a end-of-recovery record. This could at least >> be documented better. > > We don't update it during recovery at all. There's exactly one > assignment statement for that variable and it's here: Indeed. It looks like my reading was sloppy here. -- Michael
signature.asc
Description: PGP signature
