Hi,
On 2018-07-31 19:29:37 -0700, Peter Geoghegan wrote:
> On Tue, Jul 31, 2018 at 7:02 PM, Andres Freund <andres@anarazel.de> wrote:
> > Not a fan of this comment. It doesn't really explain that well why it's
> > needed here, but then goes on to a relatively general explanation of why
> > cache invalidation is necessary. Why not just go for something like
> > "register relcache invalidation on the indexes' heap relation, to
> > maintain consistency of its index list"?
>
> That seems much more generic to me!
>
> The comment is supposed to convey that the stuff within
> index_update_stats() isn't enough because of xact abort specifically.
> SI invalidation is very much part of the index_update_stats() contract
> already.
I don't think that's particularly relevant. We should always register an
invalidation before the relevant CommandCounterIncrement(), because that
is what makes catalog changes visible, and therefore requires
registering invalidations for coherency. That we possibly later, in
another CCI cycle, reach yet another invalidation point, doesn't matter
much (except that it can hide a lot of omissions). By your logic we'd
need an equivalent of the above comment at most of those places.
Greetings,
Andres Freund