Date: Mon, 10 Nov 2025 10:49:58 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>,
	Rahila Syed <rahilasyed90@gmail.com>,
	Bertrand Drouvot <bertranddrouvot.pg@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Improve LWLock tranche name visibility across backends
Message-ID: <aRIXtvkYn1mmyNG5@nathan>
References: <aLcPbyWLawp5_rdt@nathan>
 <aLiQeizOQKbKF-ae@nathan>
 <aLmzwC2dRbqk14y6@nathan>
 <CAA5RZ0t6s2Z38oRq883Ocq7J4xjn1FJF17Q4YR6++vrw5KYCtQ@mail.gmail.com>
 <aLn4n-5UFzKHFdR5@nathan>
 <dd36d384-55df-4fc2-825c-5bc56c950fa9@gmail.com>
 <CAA5RZ0u8+sC_FBwoaUeLhTi5bmjba9Yk3N-TbdsTm7c1no0zcg@mail.gmail.com>
 <CAA5RZ0v4kwF06o4a+Xybd++ipC7TYkn879Wz_2BZK412+ULXgg@mail.gmail.com>
 <CAA5RZ0va93+wk3_6UYi52=b=xqbKtXvaM-Cj5moTir7nM-LA=Q@mail.gmail.com>
 <aRISKd2MEeIqRcOh@nathan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <aRISKd2MEeIqRcOh@nathan>
Archived-At: <https://www.postgresql.org/message-id/aRIXtvkYn1mmyNG5%40nathan>
Precedence: bulk

On Mon, Nov 10, 2025 at 10:26:17AM -0600, Nathan Bossart wrote:
> On Mon, Nov 03, 2025 at 11:50:48AM -0600, Sami Imseih wrote:
>>>> I am not sure we need to do anything about this.
>>>
>>> Or maybe we just avoid the tranche_id from leaking
>>> in test_dsa_resowners() by making it a static variable
>>> and checking if we have a valid tranche id before calling
>>> LWLockNewTrancheId()? That is the proper pattern.
>> 
>> Like the attached.
> 
> It's probably a good idea to avoid tranche leaks, but IMHO there's room for
> improvement in the DSM registry, too.  IIUC the problem is that the DSM
> segment is still being added to the registry and found by other backends
> despite the initialization callback failing.  My first instinct is that we
> should keep track of whether the DSM segments/DSAs/dshash tables in the
> registry have been fully initialized and to just ERROR in other backends
> when attaching if they aren't.  That shouldn't really happen in practice,
> but it'd be good to avoid the strange errors, anyway.

BTW if we really wanted to avoid leaking tranches in test_dsa, we'd need to
store the ID in shared memory.  Your patch helps in the case where a single
backend is repeatedly calling test_dsa_resowners(), but other backends
would still allocate their own tranche.  I don't see a strong need to fix
this on back-branches, given these functions run exactly once as part of a
test, but fixing it on master seems worthwhile so that extension authors
don't copy/paste this broken code.

-- 
nathan