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

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

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.

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

In response to

Responses

Browse pgsql-admin by date

  From Date Subject
Next Message Simon Riggs 2004-07-20 09:04:29 Re: PITR COPY Failure (was Point in Time Recovery)
Previous Message Christopher Kings-Lynne 2004-07-20 01:22:42 Re: [HACKERS] Point in Time Recovery

Browse pgsql-hackers by date

  From Date Subject
Next Message Rod Taylor 2004-07-20 04:34:23 Re: pg_dump bug fixing
Previous Message Christopher Kings-Lynne 2004-07-20 04:06:40 Re: Patch for pg_dump: Multiple -t options and new -T option

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2004-07-20 04:55:17 Re: Patch for pg_dump: Multiple -t options and new -T option
Previous Message Christopher Kings-Lynne 2004-07-20 04:06:40 Re: Patch for pg_dump: Multiple -t options and new -T option