On Tue, Apr 30, 2013 at 6:58 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 9 April 2013 08:36, Jeff Davis <pgsql@j-davis.com> wrote:
>
>> 1. I believe that the issue I brought up at the end of this email:
>>
>> http://www.postgresql.org/message-id/1365035537.7580.380.camel@sussancws0025
>>
>> is a real issue. In lazy_vacuum_page(), the following sequence can
>> happen when checksums are on:
>>
>> a. PageSetAllVisible
>> b. Pass heap page to visibilitymap_set
>> c. visibilitymap_set logs the heap page and sets the LSN
>> d. MarkBufferDirty
>>
>> If a checkpoint happens between (c) and (d), then we have a problem. The
>> fix is easy: just mark the heap buffer dirty first. There's another call
>> site that looks like a potential problem, but I don't think it is. I
>> simplified the code there to make it (hopefully) more clearly correct.
>
> Applied
Uh, wait a minute. I think this is completely wrong. The buffer is
LOCKED for this entire sequence of operations. For a checkpoint to
"happen", it's got to write every buffer, which it will not be able to
do for so long as the buffer is locked.
The effect of the change to lazy_scan_heap is to force the buffer to
be written even if we're only updating the visibility map page.
That's a bad idea and should be reverted. The change to
lazy_vacuum_page is harmless, but the comment is wrong.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company