Hi,
On Thu, Jul 31, 2025 at 04:24:38PM -0500, Nathan Bossart wrote:
> I've attached a rebased version of the patch.
>
> On Mon, Jul 21, 2025 at 11:26:44PM -0500, Sami Imseih wrote:
> > Unlike the local list of tranche names, appending to and searching the
> > shared memory list requires an LWLock; in exclusive mode to append, and
> > shared mode to search.
>
> The thing that stood out the most to me is how much more expensive looking
> up the lock name is. At the risk of suggesting even more complexity, I'm
> wondering if we should add some sort of per-backend cache to avoid the need
> for locking and dsa_get_address() calls every time you need to look up a
> lock name.
Yeah, I 've the same concern that GetLWTrancheNameByID() might be too costly.
OTOH it's not really used in a hot path, "only" when displaying wait events.
But still, I agree that the extra overhead (and possible contention) is
concerning and that, for example, a per-backend cache (with fallback to the
dsa if needed) could help.
> Also, I suspect that there might be some concerns about the API changes.
> While it should be a very easy fix, that seems likely to break a lot of
> extensions.
> I don't know if it's possible to make this stuff backward
> compatible, and I also don't know if we really want to, as that'll both
> introduce more complexity and keep folks using the old API.
Yeah, I'm not sure that would be worth the extra complexity and using the old API
would "keep" the issue we're trying to solve here.
I don't think we should be worried that much by the number of extensions impacted
but more about the change complexity and it looks pretty simple.
So, I don't think we should worry that much in that regard.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
