On Sat, Oct 17, 2020 at 7:53 AM Noah Misch <n...@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