Skip site navigation (1) Skip section navigation (2)

Re: Display Pg buffer cache (WIP)

From: Neil Conway <neilc(at)samurai(dot)com>
To: Mark Kirkwood <markir(at)coretech(dot)co(dot)nz>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: Display Pg buffer cache (WIP)
Date: 2005-03-03 07:15:25
Message-ID: 4226B98D.4040304@samurai.com (view raw or flat)
Thread:
Lists: pgsql-patches
Mark Kirkwood wrote:
> does not worry too much about a consistent view of the buffer
> contents (as I didn't want it to block all other activity!).

I don't like accessing shared data structures without acquiring the 
appropriate locks; even if it doesn't break anything, it seems like just 
asking for trouble. In order to be able to inspect a buffer's tag in 
Tom's new locking scheme (not yet in HEAD, but will be in 8.1), you need 
only hold a per-buffer lock. That will mean acquiring and releasing a 
spinlock for each buffer, which isn't _too_ bad...

That means the data reported by the function might still be 
inconsistent; not sure how big a problem that is.

It might be nice for the patch to indicate whether the buffers are 
dirty, and what their shared reference count is.

> +extern Datum dump_cache(PG_FUNCTION_ARGS);

This declaration belongs in a header file (such as 
include/utils/builtins.h).

> +typedef struct {
> +	int				buffer;
> +	AttInMetadata	*attinmeta;
> +	BufferDesc		*bufhdr;
> +	RelFileNode		rnode;
> +	char			*values[3];
> +} dumpcache_fctx;

This should be values[4], no?

This is trivial, but I think most type names use camel case (NamesLikeThis).

Why does `rnode' need to be in the struct? You can also fetch the buffer 
number from the buffer desc, so you needn't store another copy of it.

> +		/* allocate the strings for tuple formation */
> +		fctx->values[0] = (char *) palloc(NAMEDATALEN + 1);
> +		fctx->values[1] = (char *) palloc(NAMEDATALEN + 1);
> +		fctx->values[2] = (char *) palloc(NAMEDATALEN + 1);
> +		fctx->values[3] = (char *) palloc(NAMEDATALEN + 1);

Is there a reason for choosing NAMEDATALEN + 1 as the size of these 
buffers? (There is no relation between NAMEDATALEN and the number of 
characters an OID will consume when converted via sprintf("%u") )

The patch doesn't treat unassigned buffers specially (i.e. those buffers 
whose tag contains of InvalidOids). Perhaps it should return NULL for 
their OID fields, rather than InvalidOid (which will be 0)? (An 
alternative would be to not include those rows in the result set, 
although perhaps administrators might want this information.)

-Neil

In response to

Responses

pgsql-patches by date

Next:From: Mark KirkwoodDate: 2005-03-03 07:39:12
Subject: Re: Display Pg buffer cache (WIP)
Previous:From: Mark KirkwoodDate: 2005-03-03 05:36:04
Subject: Display Pg buffer cache (WIP)

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group