Re: [HACKERS] [PATCH] pageinspect function to decode infomasks

[Moon Insung]

I checked for code related to infomask.
(add flag state -- HEAP_XMIN_COMMITTED, HEAP_XMIN_INVALID, HEAP_XMIN_FROZEN)

first i'm still beginner level about postgresql, so my opinion may be wrong.

if the "HEAP_XMIN_COMMITTED" flag is added, check the function of 
"HeapTupleHeaderXminInvalid"
if the "HEAP_XMIN_INVALID" flag is added, check the function of 
"HeapTupleHeaderXminCommitted"
if the "HEAP_XMIN_FROZEN" flag is added, use the "HeapTupleHeaderSetXminFrozen" 
function or
use the code as 
--
xid = HeapTupleHeaderGetXmin(tuple);
if (TransactionIdIsNormal(xid))
{
if (TransactionIdPrecedes(xid, cutoff_xid))
{
frz->t_infomask |= HEAP_XMIN_FROZEN;
changed = true;
}
else
totally_frozen = false;
} 
--
to add the flag.

so as a result, HEAP_XMIN_INVALID and HEAP_XMIN_COMMITTED is cannot coexist.
unfortunately, i don't know if HEAP_XMIN_COMMITTED and HEAP_XMIN_FROZEN flags 
can coexist.

so i think it's also a good idea to output the raw masks, without any filtering.
however, i think the information that is presented to the user should inform us 
which flags was entered.

Regards.
Moon

> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tomas Vondra
> Sent: Wednesday, August 16, 2017 5:36 AM
> To: Robert Haas
> Cc: pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] [PATCH] pageinspect function to decode infomasks
> 
> 
> 
> On 08/15/2017 09:55 PM, Robert Haas wrote:
> > On Tue, Aug 15, 2017 at 3:42 PM, Tomas Vondra
> >  wrote:
> >> What I think we should not do is interpret the bitmasks (omitting
> >> some of the information) assuming all the bits were set correctly.
> >
> > I'm still confused. HEAP_XMIN_COMMITTED|HEAP_XMIN_ABORTED ==
> > HEAP_XMIN_FROZEN. Nobody is proposing to omit anything; to the
> > contrary, what's being proposed is not to display the same thing twice
> > (and in a misleading fashion, to boot).
> >
> 
> I understand your point. Assume you're looking at this bit of code:
> 
>  if (HeapTupleHeaderXminCommitted(enumval_tup->t_data))
>  return;
> 
> which is essentially
> 
>  if (enumval_tup->t_data & HEAP_XMIN_COMMITTED)
>  return;
> 
> If the function only gives you HEAP_XMIN_FROZEN, how likely is it you miss
> this actually evaluates as true?
> 
> You might say that people investigating issues in this area of code should
> be aware of how HEAP_XMIN_FROZEN is defined, and perhaps you're right ...
> 
> regards
> 
> --
> Tomas Vondra  http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> 
> 
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make
> changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [PATCH] pageinspect function to decode infomasks

[Moon Insung]

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.

 

So I think the value of 0x0300 is to HEAP_XMIN_COMMITTED, HEAP_XMIN_FROZEN

Only output needs to be values.

 

 

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.

 

If my opinion may be wrong. So plz check one more time.

 

Regards.

Moon

 

From: pgsql-hackers-ow...@postgresql.org 
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Craig Ringer
Sent: Thursday, July 20, 2017 8:53 PM
To: Ashutosh Sharma
Cc: PostgreSQL Hackers; Julien Rouhaud; Pavan Deolasee; Álvaro Herrera; Peter 
Eisentraut; Masahiko Sawada; abhijit Menon-Sen; Peter Geoghegan
Subject: Re: [HACKERS] [PATCH] pageinspect function to decode infomasks

 

 

 

On 20 Jul. 2017 19:09, "Ashutosh Sharma"  > wrote:

I had a quick look into this patch and also tested it and following
are my observations.

 

Thanks very much.

 

I'll expand the tests to cover various normal and nonsensical masks and 
combinations and fix the identified issues.

 

This was a quick morning's work in amongst other things so not surprised I 
missed a few details. The check is appreciated.