On Tue, Apr 30, 2013 at 5:54 PM, 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.
On further review, I think you're right about this. We'd have a problem if the following sequence of events were to occur: 1. vacuum writes the WAL record, but does not yet mark the buffer dirty, and then goes into the tank 2. checkpointer decides where the checkpoint will begin 3. buffer pool is scanned as part of the checkpoint process, observing target buffer not to be dirty 4. vacuum finally wakes up and marks the buffer dirty 5. crash, after WAL is flushed but before buffer is written However, on looking at this a little more, shouldn't we be doing log_heap_clean() *before* we do visibilitymap_set()? >> 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. Well, I don't find it that ugly, but then again -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
