On Thursday, June 07, 2012 04:27:32 PM Robert Haas wrote: > On Thu, Jun 7, 2012 at 9:41 AM, Andres Freund <and...@2ndquadrant.com> wrote: > >> Proposed patch attached. This adds some more comments in various > >> places, and implements your suggestion of retesting the visibility-map > >> bit when we detect a possible mismatch with the page-level bit. > > > > Thanks, will look at it in a bit. I wonder if /* mark page all-visible, if appropriate */ if (all_visible && !PageIsAllVisible(page)) { PageSetAllVisible(page); MarkBufferDirty(buf); visibilitymap_set(onerel, blkno, InvalidXLogRecPtr, vmbuffer, visibility_cutoff_xid); } shouldn't test if (all_visible && (!PageIsAllVisible(page) || !all_visible_according_to_vm) instead.
Commentwise I am not totally content with the emphasis on memory ordering because some of the stuff is more locking than memory ordering. Except that I think its a pretty clear improvement. I can reformulate the places where I find that relevant but I have the feeling that wouldn't help the legibility. Its the big comment in vacuumlazy.c, the comment in nodeIndexonly.c and the one in the header of visibilitymap_test. Should be s/memory- ordering/concurrency/ except in nodeIndexonlyscan.c The visibilitymap_clear/PageClearAllVisible in heap_multi_insert should be moved into the critical section, shouldn't it? Regards, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers