Re: page corruption on 8.3+ that makes it to standby

On Wed, Jul 28, 2010 at 2:21 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Jul 28, 2010 at 12:36 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I understand it, and I don't like it one bit.  I haven't caught up on
>>> this thread yet, but I think the only acceptable solution is one that
>>> leaves the slave in the *same* state as the master.
>
>> I might be missing something here, but I don't see how you're going to
>> manage that.  In Jeff's original example, he crashes the database
>> after extending the relation but before initializing and writing the
>> new page.  I believe that at that point no XLOG has been written yet,
>> so the relation has been extended but there is no WAL to be sent to
>> the standby.  So now you have the exact situation you're concerned
>> about - the relation has been extended on the master but not on the
>> standby.
>
> You're right that we cannot prevent that situation --- or at least,
> the cure would be worse than the disease.  (The cure would be to
> XLOG the extension action, obviously, but then out-of-disk-space
> has to be a PANIC condition.)

Not to mention that performance would probably be atrocious.

> However, it doesn't follow that it's
> a good idea to make copy_relation_data *intentionally* make the slave
> and master different.
>
> I've caught up on the thread now, and I think that fix2 (skip logging
> the page) is extremely dangerous and has little if anything in its
> favor.

Why do you think that? They will be different only in terms of
whether the uninitialized bytes are before or after the nominal EOF,
and we know we have to be indifferent to that case anyway.

> fix1 seems reasonable given the structure of the page validity
> checks.
>
> However, what about Jeff's original comment
>
> : On second thought, why are PageSetLSN and PageSetTLI being called from
> : log_newpage(), anyway?
>
> I think it is appropriate to be setting the LSN/TLI in the case of a
> page that's been constructed by the caller as part of the WAL-logged
> action, but doing so in copy_relation_data seems rather questionable.
> We certainly didn't change the source page so changing its LSN seems
> rather wrong --- wouldn't it be better to just copy the source pages
> with their original LSNs?  So perhaps the best fix is to add a bool
> parameter to log_newpage telling it whether to update LSN/TLI, and
> have copy_relation_data pass false while the other callers pass true.
> (Although I guess we'd need to propagate that flag in the WAL record,
> so maybe this is more trouble than its worth.)

It seems like if log_newpage() were to set the LSN/TLI before calling
XLogInsert() - or optionally not - then it wouldn't be necessary to
set them also in heap_xlog_newpage(); the memcpy operation would by
definition have copied the right information onto the page. That
seems like it would be a cleaner design, but back-patching a change to
the interpretation of WAL records that might already be on someone's
disk seems dicey at best.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company