Re: Rethinking sinval callback hook API

Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Aug 19, 2011 at 2:13 PM, Tom Lane <tgl@sss.pgh.pa.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.

>> 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?

Impossible to say without coding it up and trying it ...

>> 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?

Well, that's exactly the reasoning that led to the current API.  But
we've not done well at extending that API to handle newly-added sinval
message types, and it's also designed firmly around the notion that
individual callbacks only want to see a small part of the traffic.
If I wanted to write a callback that saw *all* the traffic, the current
API is just not supportive of that.

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