From: | Andres Freund <andres(at)2ndquadrant(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Add the number of pinning backends to pg_buffercache's output |
Date: | 2014-06-23 09:51:53 |
Message-ID: | 20140623095153.GL16260@awork2.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2014-06-23 18:44:24 +0900, Fujii Masao wrote:
> On Sat, Apr 12, 2014 at 9:25 PM, Andres Freund <andres(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2014-06-23 09:53:56 | Re: [Fwd: Re: proposal: new long psql parameter --on-error-stop] |
Previous Message | Fujii Masao | 2014-06-23 09:44:24 | Re: Add the number of pinning backends to pg_buffercache's output |