Re: Injection point locking
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-07-10T03:44:22Z
Lists: pgsql-hackers
On Tue, Jul 09, 2024 at 12:12:04PM +0300, Heikki Linnakangas wrote: > I thought about it, but no. If the generation number doesn't match, there > are a few possibilities: > > 1. The entry was what we were looking for, but it was concurrently detached. > Return NULL is correct in that case. > > 2. The entry was what we were looking for, but it was concurrently detached, > and was then immediately reattached. NULL is a fine return value in that > case too. When Run runs concurrently with Detach+Attach, you don't get any > guarantee whether the actual apparent order is "Detach, Attach, Run", > "Detach, Run, Attach", or "Run, Detach, Attach". NULL result corresponds to > the "Detach, Run, Attach" ordering. > > 3. The entry was not actually what we were looking for. The name comparison > falsely matched just because the slot was concurrently detached and recycled > for a different injection point. We must continue the search in that case. > > I added a comment to the top of the loop to explain scenario 2. And a > comment to the "continue" to explain scnario 3, because that's a bit subtle. Okay. I am fine with your arguments here. There is still an argument imo about looping back at the beginning of ActiveInjectionPoints entries if we find an entry with a matching name but the generation does not match with the local copy for the detach-attach concurrent case, but just moving on with the follow-up entries is also OK by me, as well. The new comments in InjectionPointCacheRefresh() are nice improvements. Thanks for that. -- Michael
Commits
-
Improve locking around InjectionPointRun()
- d85fc4be11b3 17.0 landed