On Mon, Jun 23, 2014 at 6:51 PM, Andres Freund <and...@2ndquadrant.com> wrote: > On 2014-06-23 18:44:24 +0900, Fujii Masao wrote: >> On Sat, Apr 12, 2014 at 9:25 PM, Andres Freund <and...@2ndquadrant.com> >> wrote: >> > Hi, >> > >> > The last week I twice had the need to see how many backends had some >> > buffers pinned. Once during development and once while analyzing a stuck >> > vacuum (waiting for a cleanup lock). >> > I'd like to add a column to pg_buffercache exposing that. The name I've >> > come up with is 'pinning_backends' to reflect the fact that it's not the >> > actual pincount due to the backend private arrays. >> >> This name sounds good to me. >> >> +CREATE OR REPLACE VIEW pg_buffercache AS >> + SELECT P.* FROM pg_buffercache_pages() AS P >> + (bufferid integer, relfilenode oid, reltablespace oid, reldatabase oid, >> + relforknumber int2, relblocknumber int8, isdirty bool, usagecount int2, >> + pincount int8); >> >> pincount should be pinning_backends here? > > Yes. I'd changed my mind around a couple of times and apparently didn't > send the right version of the patch. Thanks. > >> This may be harmless but pinning_backends should be defined as int4 >> rather than int8 >> because BufferDesc->refcount is just defined as unsigned and it's >> converted to Datum >> by Int32GetDatum(). > > Well, in theory a uint32 can't generally be converted to a int32. That's > why I chose a int64 because it's guaranteed to have sufficient > range. It's fairly unlikely to have that many pins, but using a int64 > seems cheap enough here.
Yep, you're right. >> +-- complain if script is sourced in psql, rather than via CREATE EXTENSION >> >> s/CREATE/ALTER >> >> +\echo Use "CREATE EXTENSION pg_buffercache" to load this file. \quit > > Hm, right. > >> The message should be something like "ALTER EXTENSION pg_buffercache >> UPDATE TO '1.1'". >> >> + /* might not be used, but the array is long enough */ >> + values[8] = Int32GetDatum(fctx->record[i].pinning_backends); >> + nulls[8] = false; >> >> Why is the above source comment needed? > > It tries to explain that while the caller doesn't necessarily look at > values[8] (if it's the old pg_proc entry) but we're guaranteed to have > allocated a long enough values/nulls array. Understood. I think you can commit this patch after fixing some minor things. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
