Re: Add the number of pinning backends to pg_buffercache's output

On 2014-06-23 18:44:24 +0900, Fujii Masao wrote:
> On Sat, Apr 12, 2014 at 9:25 PM, Andres Freund <andres@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.

> +-- 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.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services