Re: [HACKERS][PATCH]pg_buffercache add a buffer state column, Add fuction to decode buffer state

On Tue, Nov 14, 2017 at 6:36 PM, Moon Insung
<Moon_Insung_i3(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> # I add [hacker] to the mail subject.

You should avoid top-posting.

>
> Dear Andres Freund.
>
> Thank you for review!
>> I'm disinclined to exposing state that way. It's an internal representation that's not unlikely to change. Sure,
>> pg_buffercache is more of a debugging / investigatory tool, but I nevertheless see no reason to expose it that way.
>
> Okay!
> I'll not print(or add) the internal value directly.
> (and I'll be careful when create another patch).
> Thank you
>
>> One way around that would be to create a buffer_state type that's returned by pg_buffercache and then only decoded when
>> outputting. Doing that + having a cast to an array seems like it'd provide most of the needed functionality?

+1

>
> It's it better to output the decode state value from pg_buffercache view?
> For example to following output
>
> -----
> postgres=# select * from pg_buffercache where bufferid = 1;
> -[ RECORD 1 ]----+-----------
> bufferid | 1
> relfilenode | 1262
> reltablespace | 1664
> reldatabase | 0
> relforknumber | 0
> relblocknumber | 0
> isdirty | f
> usagecount | 5
> pinning_backends | 0
> buffer_state | {LOCKED,VALID,TAG_VALID,PERMANENT}
> -----
>
> It's right?
> If it is correct, I'll modify patch ASAP.

I think it's better to register this patch to the next commit fest so
as not to forget.

>> -----Original Message-----
>> From: Andres Freund [mailto:andres(at)anarazel(dot)de]
>> Sent: Tuesday, November 14, 2017 6:07 PM
>> To: Moon Insung
>> Cc: 'PostgreSQL Hackers'
>> Subject: Re: [PATCH]pg_buffercache add a buffer state column, Add fuction to decode buffer state
>>
>> Hi,
>>
>> On 2017-11-14 17:57:00 +0900, Moon Insung wrote:
>> > So I add a state column to pg_buffercache view so that I could print a value indicating the state of the buffer.
>> > This is outpu as an unit32 type, and examples are shown below.
>>
>> > -----
>> > postgres=# select * from pg_buffercache where bufferid = 1; -[ RECORD
>> > 1 ]----+-----------
>> > bufferid | 1
>> > relfilenode | 1262
>> > reltablespace | 1664
>> > reldatabase | 0
>> > relforknumber | 0
>> > relblocknumber | 0
>> > isdirty | f
>> > usagecount | 5
>> > pinning_backends | 0
>> > buffer_state | 2203320320 <- it's a new column
>> > -----
>>
>> I'm disinclined to exposing state that way. It's an internal representation that's not unlikely to change. Sure,
>> pg_buffercache is more of a debugging / investigatory tool, but I nevertheless see no reason to expose it that way.
>>
>> If we shared those flags more in a manner like you did below:
>> > 1 | 1262 | {LOCKED,VALID,TAG_VALID,PERMANENT}
>>
>> that'd be more acceptable. However doing that by default would have some performance downsides, because we'd need to
>> create these arrays for every row.
>>
>> One way around that would be to create a buffer_state type that's returned by pg_buffercache and then only decoded when
>> outputting. Doing that + having a cast to an array seems like it'd provide most of the needed functionality?
>>

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center