On Sat, Oct 17, 2020 at 7:53 AM Noah Misch <noah@leadboat.com> wrote:
> Let's move InvalidateSystemCaches() later.
> Invalidation should follow any worker initialization step that changes the
> results of relcache validation; otherwise, we'd need to ensure the
> InvalidateSystemCaches() will not validate any relcache entry.
The thinking behind the current placement was this: just before the
call to InvalidateSystemCaches(), we restore the active and
transaction snapshots. I think that we must now flush the caches
before anyone does any more lookups; otherwise, they might get wrong
answers. So, putting this code later makes the assumption that no such
lookups happen meanwhile. That feels a little risky to me; at the very
least, it should be clearly spelled out in the comments that no system
cache lookups can happen in the functions we call in the interim.
Would it be obvious to a future developer that e.g.
RestoreEnumBlacklist cannot perform any syscache lookups? It doesn't
seem so to me.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company