On Fri, Jul 1, 2016 at 10:20 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: >>>> Right, something like that. I think Andres actually wants something >>>> like this in 9.6, and I'm inclined to think it might be a good idea, >>>> too. 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. >>>> >>>> Other opinions on the concept or the patch? >>> >>> +1 on the idea. >>> >>> + PageClearAllVisible(page); >>> + MarkBufferDirty(buf); >>> >>> What is the need to clear the Page level bit, if it is already >>> cleared, doesn't '!all_frozen' indicate that? >> >> No, I don't think so. I think all_frozen indicates whether we think >> that all tuples on the page qualify as fully frozen. I don't think it >> tells us anything about whether PD_ALL_VISIBLE is set on the page. > > Then, can we decide to clear it on that basis? Isn't it possible that > page is marked as all_visible, even if it contains frozen tuples?In > the other nearby code (refer below part of code), we only clear the > page level bit after ensuring it is set. Am I missing something? > > else if (PageIsAllVisible(page) && has_dead_tuples) > { > elog(WARNING, "page containing dead tuples is marked as all-visible in > relation \"%s\" page %u", > relname, blkno); > PageClearAllVisible(page); > MarkBufferDirty(buf); > visibilitymap_clear(onerel, blkno, vmbuffer); > }
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. 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. -- 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