Re: Reviewing freeze map code

On Wed, Jun 15, 2016 at 9:43 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Wed, Jun 15, 2016 at 6:26 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> > I spent some time chasing down the exact circumstances. I suspect
>> > that there may be an interlocking problem in heap_update. Using the
>> > line numbers from cae1c788 [1], I see the following interaction
>> > between the VACUUM, UPDATE and SELECT (pg_check_visible) backends, all
>> > in reference to the same block number:
>> >
>> > [VACUUM] sets all visible bit
>> >
>> > [UPDATE] heapam.c:3931 HeapTupleHeaderSetXmax(oldtup.t_data,
>> > xmax_old_tuple);
>> > [UPDATE] heapam.c:3938 LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
>> >
>> > [SELECT] LockBuffer(buffer, BUFFER_LOCK_SHARE);
>> > [SELECT] observes VM_ALL_VISIBLE as true
>> > [SELECT] observes tuple in HEAPTUPLE_DELETE_IN_PROGRESS state
>> > [SELECT] barfs
>> >
>> > [UPDATE] heapam.c:4116 visibilitymap_clear(...)
>>
>> Yikes: heap_update() sets the tuple's XMAX, CMAX, infomask, infomask2,
>> and CTID without logging anything or clearing the all-visible flag and
>> then releases the lock on the heap page to go do some more work that
>> might even ERROR out.
>
> Can't we clear the all-visible flag before releasing the lock? We can use
> logic of already_marked as it is currently used in code to clear it just
> once.

That just kicks the can down the road. Then you have PD_ALL_VISIBLE
clear but the VM bit is still set. And you still haven't WAL-logged
anything.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company