On Tue, Nov 14, 2017 at 6:36 PM, Moon Insung <moon_insung...@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:and...@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