Re: xlog.c: removing ReadRecPtr and EndRecPtr

On Thu, Nov 18, 2021 at 4:49 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Thu, Nov 18, 2021 at 3:14 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I'm a little dubious that this test case is valuable enough to
> >> mess around with a nonstandard postmaster startup protocol, though.
>
> > The problem that I have with the present situation is that the test
> > coverage of xlog.c is pretty abysmal.
>
> Agreed, but this one test case isn't going to move the needle much.
> To get to reasonable coverage we're going to need more tests, and
> I definitely don't want multiple versions of ad-hoc postmaster startup
> code. If we need that, it'd be smarter to extend Cluster.pm so that
> the mainline code could do what's needful.

Perhaps so. I don't have a clear view on what a full set of good tests
would look like, so it's hard for me to guess which needs are general
and which are not.

> Having said that, it wasn't entirely clear to me why you needed a
> weird startup method. Why couldn't you drop the bogus history file
> into place *before* starting the charlie postmaster? If you have
> to do it after, aren't there race/timing problems anyway?

Well, I need rescanLatestTimeLine() to be called. I'm not sure that
will happen if the file is there originally -- that sounds like more
of a scan than a rescan, but I haven't poked at that angle. I also
think it doesn't matter whether the file is dropped in or whether it
is restored via restore_command, so having the server restore the file
rather than discover that it is appeared might be another and more
satisfying option, but I also have not tested whether that reproduces
the issue. This has been extremely time-consuming to hunt down.

--
Robert Haas
EDB: http://www.enterprisedb.com