Robert wrote: > I think there should probably be a test for > all_visible_according_to_vm at the beginning of that so that we don't > add more visibility map checks for pages where we already know the VM > bit can't possibly be set.
Yes, that looks like a good idea after more screening of this code. On Wed, Jul 6, 2016 at 12:21 AM, Robert Haas <robertmh...@gmail.com> wrote: > So I'm a bit confused about what you are saying here. If the page is > marked all-frozen but actually isn't all-frozen, then we should clear > the all-frozen bit in the VM. The easiest way to do that is to clear > both bits in the VM plus the page-level bit, as done here, because we > don't presently have a way of clearing just one of the visibility map > bits. Yes, that's my understanding as well for what is necessary: clear both bits in the vm as well as the bit on the page itself, and mark it as dirty. Way to go. > Now, since the heap_lock_tuple issue requires us to introduce a new > method to clear all-visible without clearing all-frozen, we could > possibly use that here too, once we have it. For 10.0, working on lower-level routine optimizations of this kind sounds good to me. But I vote against this level of code reordering in 9.6 post-beta2 if that's what you suggest. -- Michael
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index 32b6fdd..98cf9e8 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -1183,6 +1183,23 @@ lazy_scan_heap(Relation onerel, int options, LVRelStats *vacrelstats, } /* + * If the page contains non-frozen tuples but the visibility map is + * telling the contrary, inform of the situation and take preventive + * measures on the page and the visibility map. This situation should + * not happen however. + */ + else if (all_visible_according_to_vm && + VM_ALL_FROZEN(onerel, blkno, &vmbuffer) && + !all_frozen) + { + elog(WARNING, "page contains non-frozen tuples but visibility map bit is set in relation \"%s\" page %u", + relname, blkno); + PageClearAllVisible(page); + MarkBufferDirty(buf); + visibilitymap_clear(onerel, blkno, vmbuffer); + } + + /* * It's possible for the value returned by GetOldestXmin() to move * backwards, so it's not wrong for us to see tuples that appear to * not be visible to everyone yet, while PD_ALL_VISIBLE is already
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers