Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Aug 11, 2011 at 5:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What's bothering me at the moment is that the CLOBBER_CACHE_ALWAYS hack,
>> which was meant to expose exactly this sort of problem, failed to do so
>> --- buildfarm member jaguar has been running with that flag for ages and
>> never showed this problem. �I'm thinking that we should take out the
>> hack in AcceptInvalidationMessages and instead put in #ifdeffed code
>> that causes ReceiveSharedInvalidMessages to forcibly always call the
>> reset function. �Any thoughts about that?
> I'm OK with that, but while we're whacking this around, is there any
> chance that we could reduce the number of layers of function calls
> that are happening in here?
I'm not excited about that. We have got three layers that each have
significant complexities of their own to deal with. The only way to
reduce the layer count is to make some piece of code deal with more of
those complexities at once. I don't want to go there, especially not
in the name of very marginal cycle-shaving.
> It would be nice to move the short-circuit test I recently inserted at
> the top of SIGetDataEntries() somewhere higher up in the call stack,
> but right now the layers of abstraction are so thick that it's not
> exactly clear how to do that.
Glad you didn't try, because it would be wrong. The fact that there
are no outstanding messages in sinvaladt.c doesn't prove there are none
yet unprocessed inside ReceiveSharedInvalidMessages.
Anyway, to get back to the original point: I'm getting less excited
about redoing the CLOBBER_CACHE_ALWAYS code at a different level.
The only thing that can really promise is that we find places outside
the cache code that are mistakenly holding onto cache entry pointers.
It can't do very much for the sort of problems I've been finding over
the past week, first because you need some other process actively
changing the underlying information (else the use of a stale cache entry
isn't a problem), and second because when you're looking for bugs
involving use of stale cache entries, over-enthusiastic flushing of the
cache can actually mask the bugs.
I'd still like to think of a better test methodology, but I don't think
"force clobbers inside ReceiveSharedInvalidMessages" is it.
regards, tom lane