Mark Kirkwood <markir@coretech.co.nz> writes:
> I have been doing some re-testing with CVS HEAD from about 1 hour ago
> using the simplified example posted previously.
> It is quite interesting:
The problem seems to be that the computation of checkPoint.redo at
xlog.c lines 4162-4169 (all line numbers are per CVS tip) is not
allowing for the possibility that XLogInsert will decide it doesn't
want to split the checkpoint record across XLOG files, and will then
insert a WASTED_SPACE record to avoid that (see comment and following
code at lines 758-795). This wouldn't really matter except that there
is a safety crosscheck at line 4268 that tries to detect unexpected
insertions of other records during a shutdown checkpoint.
I think the code in CreateCheckPoint was correct when it was written,
because we only recently changed XLogInsert to not split records
across files. But it's got a boundary-case bug now, which your test
scenario is able to exercise by making the recovery run try to write
a shutdown checkpoint exactly at the end of a WAL file segment.
The quick and dirty solution would be to dike out the safety check at
4268ff. I don't much care for that, but am too tired right now to work
out a better answer. I'm not real sure whether it's better to adjust
the computation of checkPoint.redo or to smarten the safety check
... but one or the other needs to allow for file-end padding, or maybe
we could hack some update of the state in WasteXLInsertBuffer(). (But
at some point you have to say "this is more trouble than it's worth",
so maybe we'll end up taking out the safety check.)
In any case this isn't a fundamental bug, just an insufficiently
smart safety check. But thanks for finding it! As is, the code has
a nonzero probability of failure in the field :-( and I don't know
how we'd have tracked it down without a reproducible test case.
regards, tom lane
