Re: Reviewing freeze map code

On Tue, Jun 14, 2016 at 8:08 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> 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.

Ah, wait a minute. I see how this could happen. Hang on, let me
update the pg_visibility patch.

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