Re: Timing of relcache inval at parallel worker init

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Timing of relcache inval at parallel worker init
Date: 2020-10-24 15:29:10
Message-ID: 20201024152910.GA2593276@rfd.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Oct 21, 2020 at 11:31:54AM -0400, Robert Haas wrote:
> On Sat, Oct 17, 2020 at 7:53 AM Noah Misch <noah(at)leadboat(dot)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.

My comment edits did attempt that. I could enlarge those comments, at the
risk of putting undue weight on the topic. One could also arrange for an
assertion failure if something takes a snapshot in the unwelcome period,
between StartParallelWorkerTransaction() and InvalidateSystemCaches().
Looking closer, we have live bugs from lookups during RestoreGUCState():

$ echo "begin; create user alice; set session authorization alice; set force_parallel_mode = regress; select 1" | psql -X
BEGIN
CREATE ROLE
SET
SET
ERROR: role "alice" does not exist
CONTEXT: while setting parameter "session_authorization" to "alice"

$ echo "begin; create text search configuration foo (copy=english); set default_text_search_config = foo; set force_parallel_mode = regress; select 1" | psql -X
BEGIN
CREATE TEXT SEARCH CONFIGURATION
SET
SET
ERROR: invalid value for parameter "default_text_search_config": "public.foo"
CONTEXT: while setting parameter "default_text_search_config" to "public.foo"

I gather $SUBJECT is the tip of an iceberg, so I'm withdrawing the patch and
abandoning the topic.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Dolgov 2020-10-24 16:45:53 Re: Index Skip Scan (new UniqueKeys)
Previous Message Peter Geoghegan 2020-10-24 15:01:06 Re: Deleting older versions in unique indexes to avoid page splits