On Wed, 2013-05-01 at 11:33 -0400, Robert Haas wrote:
> >> The only time the VM and the data page are out of sync during vacuum is
> >> after a crash, right? If that's the case, I didn't think it was a big
> >> deal to dirty one extra page (should be extremely rare). Am I missing
> >> something?
> >>
> >> The reason I removed that special case was just code
> >> complexity/readability. I tried preserving the previous behavior, and
> >> it's not so bad, but it seemed unnecessarily ugly for the benefit of a
> >> rare case.
> >
> > Well, I don't find it that ugly, but then again
>
> ...I've done a fair amount of hacking on this code. The scenario that
> I find problematic here is that you could easily lose a couple of
> visibility map pages in a crash. Each one you lose, with this patch,
> potentially involves half a gigabyte of unnecessary disk writes, if
> the relation is being vacuumed at the time. For the few extra lines
> of code it takes to protect against that scenario, I think we should
> protect against it.
I'm still unclear on how we'd lose so many VM bits at once, because they
are logged. I see how we can lose one if the heap page makes it to disk
before the VM bit's WAL is flushed, but that seems to be bounded to me.
Regardless, you have a reasonable claim that my patch had effects that
were not necessary. I have attached a draft patch to remedy that. Only
rudimentary testing was done.
Regards,
Jeff Davis