On 30 April 2013 22:54, Jeff Davis <pg...@j-davis.com> wrote: > On Tue, 2013-04-30 at 08:34 -0400, Robert Haas wrote: >> 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. > > I went back and forth on this, so you could be right, but here was my > reasoning: > > I was worried because SyncOneBuffer checks whether it needs writing > without taking a content lock, so the exclusive lock doesn't help. That > makes sense, because you don't want a checkpoint to have to get a > content lock on every buffer in the buffer pool. But it also means we > need to follow the rules laid out in transam/README and dirty the pages > before writing WAL. > >> 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 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.
All of that makes perfect sense to me. Waiting to hear back from Robert whether he still has an objection. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
