pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: Rahila Syed <rahilasyed90@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Improve LWLock tranche name visibility across backends
Date: Tue, 12 Aug 2025 14:48:50 -0500
Message-ID: <aJuaoouYv8l8f0PS@nathan> (raw)
In-Reply-To: <CAA5RZ0siw2MrVjf9iPYRpe8sgZW+TJRqmxKfCr=QS9mX+ZF8Lw@mail.gmail.com>
References: <CAA5RZ0skN3ATpEvcFCUCiHDqvs6N95uzAo74wUSR93Bdgrq5Xg@mail.gmail.com>
	<CAA5RZ0uYVpKzQbP6JJhGv=JBGfOMVwzTtyd789P_hJxXXyKuhw@mail.gmail.com>
	<aJHSULQPfY9qUTFY@ip-10-97-1-34.eu-west-3.compute.internal>
	<CAA5RZ0tU9j-jEPUCS0=K6PCzYY=291O9Uky9ndbqH_BDp4Hc=Q@mail.gmail.com>
	<CAA5RZ0txvpg7pZGRt2aUmBck8rVK1nUZkwpu=ehyL6kmHd_apQ@mail.gmail.com>
	<aJMUvF0Pbswf4oUX@ip-10-97-1-34.eu-west-3.compute.internal>
	<CAH2L28uCaeuyz9iZOfHK_XPaJw06whE+92g0pGW6PytV1YHSbg@mail.gmail.com>
	<CAA5RZ0uh_WviMnh9z-oZP32SSE5MBEshzx5Cfe0rFYnPwQNzJw@mail.gmail.com>
	<CAA5RZ0tQ+EYSTOd2hQ8RXdsNfGBLAtOe-YmnsTE6ZVg0E-4qew@mail.gmail.com>
	<CAA5RZ0siw2MrVjf9iPYRpe8sgZW+TJRqmxKfCr=QS9mX+ZF8Lw@mail.gmail.com>

I haven't followed the latest discussion, but I took a look at the patch.

+      It is possible to use a <literal>tranche_id</literal> that was not retrieved
+      using <function>LWLockNewTrancheId</function>, but this is not recommended.
+      The ID may clash with an already registered tranche name, or the specified
+      name may not be found. In such cases, looking up the name will return a generic
+      "extension" tranche name.

Is there any reason to continue allowing this?  For example, maybe we could
ERROR if LWLockInitialize()/GetLWTrancheName() are given a tranche_id
greater than the number allocated.  I guess I'm not following why we should
gracefully handle these kinds of coding errors, especially when they result
in unhelpful behavior like an "extension" tranche.

+#else
+elog(ERROR, "injection points not supported");
+#endif

This causes compilation to fail when injection points are not enabled.

I haven't combed through the patch character-by-character, but upon a
read-through, the general shape looks reasonable to me.  As a general note,
I'd suggest adding more commentary throughout and finding opportunities to
simplify and/or clean up the code as much as possible.

-- 
nathan





view thread (107+ messages)  latest in thread

Message-ID: <aJuaoouYv8l8f0PS@nathan>
Permalink:  ../aJuaoouYv8l8f0PS@nathan/
Also on:    postgresql.org/message-id/aJuaoouYv8l8f0PS@nathan

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, samimseih@gmail.com, rahilasyed90@gmail.com, bertranddrouvot.pg@gmail.com
  Subject: Re: Improve LWLock tranche name visibility across backends
  In-Reply-To: <aJuaoouYv8l8f0PS@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox