Re: Rethinking sinval callback hook API

On Fri, Aug 19, 2011 at 2:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Currently, we have two types of callbacks that can be registered to get
> control when an invalidation message is received: syscache callbacks and
> relcache callbacks.  It strikes me that we might be better advised to
> unify these into a single type of callback that gets a
> SharedInvalidationMessage struct pointer (we could pass NULL to signify
> a cache reset event).  That would avoid having to add another
> registration list every time we decide that there's a reason for
> callbacks to see another type of inval message.  There are a couple of
> reasonably near-term reasons why we might want to do this:
>
> 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.

> 2. In conjunction with what I'm doing with plancache, it struck me that
> it might be nice to subdivide relcache inval messages into two types,
> one indicating a schema (DDL) change and one that just indicates that
> statistics changed; this would allow us to avoid redoing parse analysis
> and rewrite for a cached query as a consequence of autovacuum stats
> updates.  With the current scheme, plancache.c would need to register
> two different kinds of callbacks to handle that, or we'd need some other
> API change for relcache callbacks so they could distinguish.

Would this be enough for us to find a noticeable performance improvement?

> A small disadvantage of this is that callbacks would have to know about
> struct SharedInvalidationMessage, which right now isn't tremendously
> widely known, but that doesn't seem like a fatal objection to me.
> In practice that struct definition has been pretty stable.

I'm not opposed to trying to create a better/more universal API, but I
find that a bit grotty.  We've already resorted to some ugly
bit-space-preserving hacks in that structure, and I'm not sure we
won't have more in the future.  In particular, exposing the
backend_lo/backend_hi thing seems like a recipe for distributed
confusion.  Would it be too expensive to expose an opaque struct with
accessor functions?  Or pass the extracted values as separate
arguments?

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