Re: PITR COPY Failure (was Point in Time Recovery)

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Mark Kirkwood <markir(at)coretech(dot)co(dot)nz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: PITR COPY Failure (was Point in Time Recovery)
Date: 2004-07-20 09:04:29
Message-ID: 1090314268.28049.2308.camel@stromboli
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-admin pgsql-hackers pgsql-patches

On Tue, 2004-07-20 at 05:14, Tom Lane wrote:
> Mark Kirkwood <markir(at)coretech(dot)co(dot)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

In response to

Browse pgsql-admin by date

  From Date Subject
Next Message Mark Kirkwood 2004-07-20 09:45:39 Re: PITR COPY Failure (was Point in Time Recovery)
Previous Message Tom Lane 2004-07-20 04:14:29 Re: PITR COPY Failure (was Point in Time Recovery)

Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Kirkwood 2004-07-20 09:45:39 Re: PITR COPY Failure (was Point in Time Recovery)
Previous Message Zeugswetter Andreas SB SD 2004-07-20 07:40:23 Re: Point in Time Recovery

Browse pgsql-patches by date

  From Date Subject
Next Message Mark Kirkwood 2004-07-20 09:45:39 Re: PITR COPY Failure (was Point in Time Recovery)
Previous Message Christopher Kings-Lynne 2004-07-20 06:47:21 Re: Patch for pg_dump: Multiple -t options and new -T option