Re: Reviewing freeze map code
От | Amit Kapila |
---|---|
Тема | Re: Reviewing freeze map code |
Дата | |
Msg-id | CAA4eK1+4ZxVaJGn+nQ7-9sCj6i1BsZy2zoT+hxcBvc1+5vE5xw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Reviewing freeze map code (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: Reviewing freeze map code
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
On Wed, Jun 15, 2016 at 7:13 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Jun 15, 2016 at 9:43 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Wed, Jun 15, 2016 at 6:26 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> >> On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro
> >> <thomas.munro@enterprisedb.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.
I mean to say clear both as we are doing currently in code:
>
> On Wed, Jun 15, 2016 at 9:43 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Wed, Jun 15, 2016 at 6:26 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> >> On Wed, Jun 15, 2016 at 2:41 AM, Thomas Munro
> >> <thomas.munro@enterprisedb.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.
I mean to say clear both as we are doing currently in code:
if (PageIsAllVisible(BufferGetPage(buffer)))
{
all_visible_cleared = true;
PageClearAllVisible(BufferGetPage(buffer));
visibilitymap_clear(relation, BufferGetBlockNumber(buffer),
vmbuffer);
}
{
all_visible_cleared = true;
PageClearAllVisible(BufferGetPage(buffer));
visibilitymap_clear(relation, BufferGetBlockNumber(buffer),
vmbuffer);
}
>
> And you still haven't WAL-logged
> anything.
>
Yeah, I think WAL requirement is more difficult to meet and I think releasing the lock on buffer before writing WAL could lead to flush of such a buffer before WAL.
I feel this is an existing-bug and should go to Older Bugs Section in open items page.
В списке pgsql-hackers по дате отправления: