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

Reply via email to