Date: Mon, 13 May 2024 11:54:03 -0700
From: Noah Misch <noah@leadboat.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Smolkin Grigory <smallkeen@gmail.com>,
	pgsql-hackers@postgresql.org
Subject: Re: race condition in pg_class
Message-ID: <20240513185403.d7@rfd.leadboat.com>
References: 
 <CAMp+ueZQz3yDk7qg42hk6-9gxniYbp-=bG2mgqecErqR5gGGOA@mail.gmail.com>
 <20231027044404.7e.nmisch@google.com>
 <20231027184832.9b.nmisch@google.com>
 <1596629.1698435146@sss.pgh.pa.us>
 <20231027214946.79.nmisch@google.com>
 <1617596.1698446455@sss.pgh.pa.us>
 <20231027232612.99.nmisch@google.com>
 <20231102030915.d3.nmisch@google.com>
 <20240512232923.aa.nmisch@google.com>
 <ZkHIf7GSvd7Q1-Xj@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZkHIf7GSvd7Q1-Xj@paquier.xyz>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240513185403.d7%40rfd.leadboat.com>
Precedence: bulk

On Mon, May 13, 2024 at 04:59:59PM +0900, Michael Paquier wrote:
> About inplace050-tests-inj-v1.patch.
> 
> +	/* Check if blocked_pid is in injection_wait(). */
> +	proc = BackendPidGetProc(blocked_pid);
> +	if (proc == NULL)
> +		PG_RETURN_BOOL(false);	/* session gone: definitely unblocked */
> +	wait_event =
> +		pgstat_get_wait_event(UINT32_ACCESS_ONCE(proc->wait_event_info));
> +	if (wait_event && strncmp("INJECTION_POINT(",
> +							  wait_event,
> +							  strlen("INJECTION_POINT(")) == 0)
> +		PG_RETURN_BOOL(true);
> 
> Hmm.  I am not sure that this is the right interface for the job
> because this is not only related to injection points but to the
> monitoring of a one or more wait events when running a permutation
> step.

Could you say more about that?  Permutation steps don't monitor wait events
today.  This patch would be the first instance of that.

> Perhaps this is something that should be linked to the spec
> files with some property area listing the wait events we're expected
> to wait on instead when running a step that we know will wait?

The spec syntax doesn't distinguish contention types at all.  The isolation
tester's needs are limited to distinguishing:

  (a) process is waiting on another test session
  (b) process is waiting on automatic background activity (autovacuum, mainly)

Automatic background activity doesn't make a process enter or leave
injection_wait(), so all injection point wait events fall in (a).  (The tester
ignores (b), since those clear up without intervention.  Failing to ignore
them, as the tester did long ago, made output unstable.)