On Wed, 2010-07-28 at 06:40 -0400, Robert Haas wrote:
> > fix1 -- Only call PageSetLSN/TLI inside log_newpage() and
> > heap_xlog_newpage() if the page is not zeroed.
> >
> > fix2 -- Don't call log_newpage() at all if the page is not zeroed.
> >
> > Please review. I don't have a strong opinion about which one should be
> > applied.
>
> Looks good. I still prefer fix2; it seems cleaner to me. It has
> another advantage, too, which is that copy_relation_data() is used
> ONLY by ALTER TABLE SET TABLESPACE. So if I stick to patching that
> function, that's the only thing I can possibly break, whereas
> log_newpage() is used in a bunch of other places. I don't think
> either fix is going to break anything at all, but considering that
> this is going to need back-patching, I'd rather be conservative.
>
Sounds good to me.
However, when Simon said "We definitely shouldn't do anything that
leaves standby different to primary." you said "obviously". Fix2 can
leave a difference between the two, because zeroed pages at the end of
the heap file on the primary will not be sent to the standby (the
standby will only create the zeroed pages if a higher block number is
sent; which won't be the case if the zeroed pages are at the end).
As we discussed before, that looks inconsequential, but I just want to
make sure that it's understood.
> Speaking of back-patching, the subject line describes this as an 8.3+
> problem, but it looks to me like this goes all the way back to 8.0.
> The code is a little different in 8.2 and prior, but ISTM it's
> vulnerable to the same issue. Don't we need a modified version of
> this patch for the 8.0 - 8.2 branches?
That sounds right. I just saw that the code in question was introduced
in 8.3.
Regards,Jeff Davis