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

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

In response to

Responses

Browse pgsql-hackers by date

  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