Re: [PATCH] pageinspect function to decode infomasks

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] pageinspect function to decode infomasks
Date: 2017-08-16 01:37:49
Message-ID: CAMsr+YH1OCFZwC+L5AKAsAZiUJFm99m-3QJjnWQhppHUtNA+5A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 16 August 2017 at 03:42, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
wrote:

>
>
> On 08/15/2017 07:54 PM, Robert Haas wrote:
>
>> On Tue, Aug 15, 2017 at 9:59 AM, Tomas Vondra
>> <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
>>
>>> I don't think so -- the "committed" and "invalid" meanings are
>>>> effectively canceled when the "frozen" mask is present.
>>>>
>>>> I mean, "committed" and "invalid" contradict each other...
>>>>
>>>
>>> FWIW I agree with Craig - the functions should output the masks raw,
>>> without
>>> any filtering. The reason is that when you're investigating data
>>> corruption
>>> or unexpected behavior, all this is very useful when reasoning about what
>>> might (not) have happened.
>>>
>>> Or at least make the filtering optional.
>>>
>>
>> I don't think "filtering" is the right way to think about it. It's
>> just labeling each combination of bits with the meaning appropriate to
>> that combination of bits.
>>
>> I mean, if you were displaying the contents of a CLOG entry, would you
>> want the value 3 to be displayed as COMMITTED ABORTED SUBCOMMITTED
>> because TRANSACTION_STATUS_COMMITTED|TRANSACTION_STATUS_ABORTED ==
>> TRANSACTION_STATUS_SUB_COMMITTED?
>>
>> I realize that you may be used to thinking of the HEAP_XMIN_COMMITTED
>> and HEAP_XMAX_COMMITTED bits as two separate bits, but that's not
>> really true any more. They're a 2-bit field that can have one of four
>> values: committed, aborted, frozen, or none of the above.
>>
>>
> All I'm saying is that having the complete information (knowing which bits
> are actually set in the bitmask) is valuable when reasoning about how you
> might have gotten to the current state. Which I think is what Craig is
> after.
>
> What I think we should not do is interpret the bitmasks (omitting some of
> the information) assuming all the bits were set correctly.

I agree, and the patch already does half of this: it can output just the
raw bit flags, or it can interpret them to show HEAP_XMIN_FROZEN etc.

So the required change, which seems to have broad agreement, is to have the
"interpret the bits" mode show only HEAP_XMIN_FROZEN when it sees
HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID, etc. We can retain raw-flags output
as-is for when seriously bogus state is suspected.

Any takers?

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2017-08-16 02:13:41 Re: Moving relation extension locks out of heavyweight lock manager
Previous Message Moon Insung 2017-08-16 01:33:23 Re: [PATCH] pageinspect function to decode infomasks