On Sun, Nov 7, 2021 at 11:24 PM Michael Paquier <michael@paquier.xyz> wrote:
> 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.
Perhaps. That's related to the point I made before, that it might be a
good idea to be more clear about which of these functions are intended
to be used in recovery and which ones are intended to be used in
normal running. I don't rule out the possibility of doing some more
research here and tightening that up, but I don't want to go start
tweaking things unless I'm sure I know what I'm talking about. This
stuff is so confusing that I don't want to take any risk of making
changes that turn out to be incorrect.
> > 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 don't think it's very confusing, but if you want to improve the
comments, go for it!
> >> 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.
I can hardly blame you. There are comments all over the place claiming
that this thing and that thing are the same when they're only
sometimes the same. Or, as in this case, things that are named the
same when they're actually different. I'm actually tempted to make a
pass over the comments related to this topic and just try to make
things more comprehensible. It's super-confusing as things stand.
--
Robert Haas
EDB: http://www.enterprisedb.com