Thread: RE: [HACKERS][PATCH]pg_buffercache add a buffer state column, Add fuction to decode buffer state
RE: [HACKERS][PATCH]pg_buffercache add a buffer state column, Add fuction to decode buffer state
From
"Moon Insung"
Date:
# I add [hacker] to the mail subject.
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?
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.
Regards.
Moon.
> -----Original Message-----
> From: Andres Freund [mailto:andres@anarazel.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?
>
> Greetings,
>
> Andres Freund
Re: [HACKERS][PATCH]pg_buffercache add a buffer state column, Addfuction to decode buffer state
From
Masahiko Sawada
Date:
On Tue, Nov 14, 2017 at 6:36 PM, Moon Insung
<Moon_Insung_i3@lab.ntt.co.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@anarazel.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