Re: [PATCH] pageinspect function to decode infomasks

On Thu, Oct 12, 2017 at 7:35 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
> On Tue, Aug 15, 2017 at 10:54 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> 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 do. -1 to not just showing what's on the page -- if the
> HEAP_XMIN_COMMITTED and HEAP_XMIN_ABORTED bits are set, then I think
> we should show them. Yeah, I accept that there is a real danger of
> confusing people with that. Unfortunately, I think that displaying
> HEAP_XMIN_FROZEN will cause even more confusion. I don't think that
> HEAP_XMIN_FROZEN is an abstraction at all. It's a notational
> convenience.

Well, *I* think that HEAP_XMIN_FROZEN is an abstraction. That's why
we have #define -- to help us create abstractions.

> I don't think it's our place to "interpret" the bits. Are we *also*
> going to show HEAP_XMIN_FROZEN when xmin is physically set to
> FrozenTransactionId?

No, of course not. We're talking about how to display the 256 and 512
bits of t_infomask. Those have four states: nothing, committed,
invalid, frozen. You're arguing that frozen isn't a real state, that
it's somehow just a combination of committed and invalid, but I think
that's the wrong way of thinking about it. When the 256-bit is clear,
the 512-bit tells you whether the xmin is known invalid, but when the
256-bit is set, the 512-bit tells you whether the tuple is also
frozen.

Before HEAP_XMIN_FROZEN existed, it would have been right to display
the state where both bits are set as committed|invalid, because that
would clearly show you that two things had been set that should never
both be set at the same time. But now that's a valid state with a
well-defined meaning and I think we should display the actual meaning
of that state.

> Where does it end?

I guess it ends wherever we decide to stop. This isn't some kind of
crazy slippery slope we're talking about here, where one day we're
labeling informask bits and the next day it's global thermonuclear
war.

> I think that we should prominently document that HEAP_XMIN_COMMITTED
> |HEAP_XMIN_ABORTED == HEAP_XMIN_FROZEN, rather than trying to hide
> complexity that we have no business hiding in a tool like pageinspect.

I respect that opinion, but I don't think I'm trying to hide anything.
I think I'm proposing that we display the information in what I
believed to be the clearest and most accurate way.

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