On Tue, 2004-07-20 at 05:14, Tom Lane wrote:
> 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.
>
Thanks for locating that, I was suspicious of that piece of code, but it
would have taken me longer than this to locate it exactly.
It was clear (to me) that it had to be of this nature, since I've done a
fair amount of recovery testing and not hit anything like that.
> 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.)
>
I'll take a look
> 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.
All code has a non-zero probability of failure in the field, its just
they don't tell you that usually. The main thing here is that we write
everything we need to write to the logs in the first place.
If that is true, then the code can always be adjusted or the logs dumped
and re-spliced to recover data.
Definitely: Thanks Mark! Reproducibility is key.
Best regards, Simon Riggs
