On 2016-07-13 23:06:07 -0700, Andres Freund wrote: > > + /* Clear only the all-frozen bit on visibility map if needed */ > > + if (PageIsAllVisible(BufferGetPage(buffer)) && > > + VM_ALL_FROZEN(relation, block, &vmbuffer)) > > + { > > + visibilitymap_clear_extended(relation, block, vmbuffer, > > + > > VISIBILITYMAP_ALL_FROZEN); > > + } > > + > > FWIW, I don't think it's worth introducing visibilitymap_clear_extended. > As this is a 9.6 only patch, i think it's better to change > visibilitymap_clear's API.
Besides that easily fixed issue, the code also has the significant issue that it's only performing the the visibilitymap processing in the BLK_NEEDS_REDO case. But that's not ok, because both in the BLK_RESTORED and the BLK_DONE cases the visibilitymap isn't guaranteed (or even likely in the former case) to have been updated. I think we have two choices how to deal with that: First, we can add a new flags variable to xl_heap_lock similar to xl_heap_insert/update/... and bump page magic, or we can squeeze the information into infobits_set. The latter seems fairly ugly, and fragile to me; so unless somebody protests I'm going with the former. I think due to padding the additional byte doesn't make any size difference anyway. Regards, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers