Re: Rethinking sinval callback hook API

On Sun, Aug 21, 2011 at 6:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Fri, Aug 19, 2011 at 2:13 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> 1. Robert was speculating the other day about wanting to be able to
>>> snoop the inval traffic.  Right now, callbacks can only snoop a fairly
>>> small subset of it.
>
>> Is that true?  It appears to me that the events that aren't exposed at
>> all are smgr and relmap invalidations, which don't seem terribly
>> important, and presumably not a majority of the traffic.
>
> Well, "important" is in the eye of the beholder here --- if you did need
> to see one of those, you're flat outta luck.  It's also the case that
> you can only snoop one catcache per registration, so there aren't enough
> slots available in the fixed-size list to watch all the catcache
> traffic.

Yeah, I'm not opposed to making this more generic; regardless of
whether we have an immediate use case for it, it seems like a pretty
good idea. I was just surprised that you described the available
portion as a "small subset".

The one-catcache-per-registration limitation is an interesting point.
I doubt that we want to move the "is this the relevant catcache?" test
inside all the callbacks, but we might want to have a special value
that means "call me back when there's a change that affects ANY
catcache"... or even more generally "call me back when there's a
change that affects ANY system catalog, regardless of whether there is
an associated catcache or not." sepgsql, for example, really wants to
be able to get a callback when pg_seclabel or pg_shseclabel is
updated, precisely because it wants to maintain its own
special-purpose cache on a catalog that on which we DON'T want to add
a catcache.

> Exposing SharedInvalidationMessage could be going too far in the other
> direction, but I'm thinking we really ought to do something.

I think the best option might be to expose it as an opaque struct. In
other words, the header file available to other backends would have
something like:

struct SharedInvalidationMessage;
typedef struct SharedInvalidationMessage SharedInvalidationMessage;

typedef enum
{
SIM_CATCACHE,
SIM_CATALOG,
SIM_RELCACHE,
SIM_SMGR,
SIM_RELMAP
} SIMType;

SIMType SIMGetType(SharedInvalidationMessage *);
Oid SIMGetDatabase(SharedInvalidationMessage *);
BackendId SIMGetBackendId(SharedInvalidationMessage *);
/* etc. */

That allows us to do things like change the number of bits we use to
store the backend ID (e.g. from the current 24 to 32 or 16) without
needing to change the callers. In fact, you could probably even add
whole new message types and most callers wouldn't need to care, since
the typical caller is going to do something like ... if
(SIMGetType(&msg) != SIM_SOMETHING) return right off the bat.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company