On 2018-09-06 17:38:38 -0400, Tom Lane wrote:
> I wrote:
> > So where are we on this? Should I proceed with my patch, or are we
> > going to do further investigation? Does anyone want to do an actual
> > patch review?
>
> [ crickets... ]
Sorry, bit busy with postgres open, and a few people being in town due
to that.
Could you attach the current version of the patch, or were there no
meaningful changes?
> So I took that as license to proceed, but while doing a final round of
> testing I found out that a CLOBBER_CACHE_RECURSIVELY build fails,
> because now that's an infinite recursion. On reflection it's a bit
> surprising that it wasn't so all along. What I'm inclined to do about
> it is to adjust AcceptInvalidationMessages so that there's a finite
> recursion depth limit in the CLOBBER_CACHE_RECURSIVELY case, as there
> already is in the CLOBBER_CACHE_ALWAYS case. Maybe 3 or so levels
> would be enough.
Hm, but wouldn't that pretty much mean that the risk exposed in this
thread would still be present? The reason your approach fixes things is
that if we're processing invalidation event N, which depends on
invalidation changes at N+y being processed, is that the recursion leads
to N+y being processed before the invalidation processing of N finishes,
due to the recursion. Now you can argue that your restriction would
only apply to CLOBBER_CACHE_RECURSIVELY, and thus not in normal builds,
but that still makes me uncomfortable.
Greetings,
Andres Freund