On Mon, Jul 18, 2016 at 9:13 AM, Andres Freund <and...@anarazel.de> wrote: > On 2016-07-18 09:07:19 +0530, Amit Kapila wrote: >> + /* >> + * Before locking the buffer, pin the visibility map page if it may be >> + * necessary. >> + */ >> >> + if (PageIsAllVisible(BufferGetPage(*buffer))) >> + visibilitymap_pin(relation, block, &vmbuffer); >> + >> LockBuffer(*buffer, BUFFER_LOCK_EXCLUSIVE); >> >> I think we need to check for PageIsAllVisible and try to pin the >> visibility map after taking the lock on buffer. I think it is quite >> possible that in the time this routine tries to acquire lock on >> buffer, the page becomes all visible. > > I don't see how. Without a cleanup lock it's not possible to mark a page > all-visible/frozen. >
Consider the below scenario. Vacuum a. acquires a cleanup lock for page - 10 b. busy in checking visibility of tuples --assume, here it takes some time and in the meantime Session-1 performs step (a) and (b) and start waiting in step- (c) c. marks the page as all-visible (PageSetAllVisible) d. unlockandrelease the buffer Session-1 a. In heap_lock_tuple(), readbuffer for page-10 b. check PageIsAllVisible(), found page is not all-visible, so didn't acquire the visbilitymap_pin c. LockBuffer in ExlusiveMode - here it will wait for vacuum to release the lock d. Got the lock, but now the page is marked as all-visible, so ideally need to recheck the page and acquire the visibilitymap_pin -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
