Re: [PATCH] pageinspect function to decode infomasks

On 15 August 2017 at 09:11, Moon Insung <Moon_Insung_i3@lab.ntt.co.jp>
wrote:

> Dear Craig Ringer
>
>
>
> Frist, thank you for implementing the necessary function.
>
>
>
> but, i have some question.
>
>
>
> question 1) vacuum freeze hint bits
>
> If run a vacuum freeze, bits in the infomask will be 0x0300.
>
> in this case, if output the value of informsk in the run to you modified,
>
> HEAP_XMIN_COMMITTED(0x0100), HEAP_XMIN_INVALID(0x0200),
> HEAP_XMIN_FROZEN(0x0300)
>
> all outputs to hint bits.
>
>
>
> is it normal to output values?
>
>
>
> if look at htup_details.h code,
>
>
>
> #define HeapTupleHeaderXminInvalid(tup) \
>
> ( \
>
>               ((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID))
> == \
>
>                             HEAP_XMIN_INVALID \
>
> )
>
> #define HeapTupleHeaderSetXminCommitted(tup) \
>
> ( \
>
>               AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
>
>               ((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
>
> )
>
>
>
> HEAP_XMIN_INVALID and HEAP_XMIN_COMMITTED can not be write simultaneously.
>

The bits are set, those macros just test to exclude the special meaning of
both bits being set at once to mean "frozen".

I was reluctant to filter out  HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID
when we detect that it's frozen, because that could well be misleading when
debugging.

If you think that is useful, then I suggest you add an option so that when
it's outputting the interpreted mask not the raw mask, it suppresses output
of HEAP_XMIN_COMMITTED and HEAP_XMIN_INVALID if HEAP_XMIN_FROZEN.

question 2) xmax lock hint bits
>
> similar to the vacuum freezeze question..
>
> Assume that the infomask has a bit of 0x0050
>
>
>
> In this case, if run on the code that you modified,
>
> HEAP_XMAX_KEYSHR_LOCK(0x0010), HEAP_XMAX_EXCL_LOCK(0x0040),
> HEAP_XMAX_IS_LOCKED_ONLY
>
> three hint bits are the output.
>
>
>
> if look at htup_details.h code,
>
>
>
> #define HEAP_XMAX_IS_SHR_LOCKED(infomask) \
>
>               (((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_SHR_LOCK)
>
> #define HEAP_XMAX_IS_EXCL_LOCKED(infomask) \
>
>               (((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_EXCL_LOCK)
>
> #define HEAP_XMAX_IS_KEYSHR_LOCKED(infomask) \
>
>               (((infomask) & HEAP_LOCK_MASK) == HEAP_XMAX_KEYSHR_LOCK)
>
>
>
> It is divided into to hint bits.
>
> so I think this part needs to fix.
>

It's the same issue as above, with the same answer IMO.

If we're showing the raw mask bits we should show the raw mask bits only.

But if we're showing combined bits and masks too, I guess we should filter
out the raw bits when matched by some mask.

I'm not entirely convinced by that, since I think hiding information could
create more confusion than it fixes. I welcome others' views here.

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