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 <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
