Re: Reviewing freeze map code

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Reviewing freeze map code
Date: 2016-06-14 12:08:15
Message-ID: CA+TgmoYOHNANcr503GDoxwyynLHBwgWeB-6sGF2ikBb=sMQLjw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 14, 2016 at 2:53 AM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> On Tue, Jun 14, 2016 at 4:02 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Sat, Jun 11, 2016 at 5:00 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>>> How about changing the return tuple of heap_prepare_freeze_tuple to
>>>> a bitmap? Two flags: "Freeze [not] done" and "[No] more freezing
>>>> needed"
>>>
>>> Yes, I think something like that sounds about right.
>>
>> Here's a patch. I took the approach of adding a separate bool out
>> parameter instead. I am also attaching an update of the
>> check-visibility patch which responds to assorted review comments and
>> adjusting it for the problems found on Friday which could otherwise
>> lead to false positives. I'm still getting occasional TIDs from the
>> pg_check_visible() function during pgbench runs, though, so evidently
>> not all is well with the world.
>
> I'm still working out how half this stuff works, but I managed to get
> pg_check_visible() to spit out a row every few seconds with the
> following brute force approach:
>
> CREATE TABLE foo (n int);
> INSERT INTO foo SELECT generate_series(1, 100000);
>
> Three client threads (see attached script):
> 1. Run VACUUM in a tight loop.
> 2. Run UPDATE foo SET n = n + 1 in a tight loop.
> 3. Run SELECT pg_check_visible('foo'::regclass) in a tight loop, and
> print out any rows it produces.
>
> I noticed that the tuples that it reported were always offset 1 in a
> page, and that the page always had a maxoff over a couple of hundred,
> and that we called record_corrupt_item because VM_ALL_VISIBLE returned
> true but HeapTupleSatisfiesVacuum on the first tuple returned
> HEAPTUPLE_DELETE_IN_PROGRESS instead of the expected HEAPTUPLE_LIVE.
> It did that because HEAP_XMAX_COMMITTED was not set and
> TransactionIdIsInProgress returned true for xmax.

So this seems like it might be a visibility map bug rather than a bug
in the test code, but I'm not completely sure of that. How was it
legitimate to mark the page as all-visible if a tuple on the page
still had a live xmax? If xmax is live and not just a locker then the
tuple is not visible to the transaction that wrote xmax, at least.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-06-14 12:11:06 Re: Reviewing freeze map code
Previous Message Kyotaro HORIGUCHI 2016-06-14 11:31:11 Re: [BUG] pg_basebackup from disconnected standby fails