Re: Improve LWLock tranche name visibility across backends

On Mon, Nov 10, 2025 at 12:05 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
>
> On Mon, Nov 10, 2025 at 10:49:58AM -0600, Nathan Bossart wrote:
> > On Mon, Nov 10, 2025 at 10:26:17AM -0600, Nathan Bossart wrote:
> >> 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.
>
> 0001 does this. When applied, Alexander's reproduction steps fail with
>
> ERROR: requested DSM segment failed initialization
>
> This one should probably get back-patched to v17, where the DSM registry
> was introduced.

I tested this and it seems like a good idea. The first time
GetNamedDSMSegment is called, the entry is created but it's
left in an incomplete state due to an error. You will see the
error. The next time it's called, the caller encounters
"ERROR: requested DSM segment failed initialization"

psql -c "SELECT test_dsa_resowners() FROM generate_series(1, 256)" >/dev/null

psql -c "SELECT autoprewarm_dump_now()"
ERROR: maximum number of tranches already registered
DETAIL: No more than 256 tranches may be registered.

psql -c "SELECT autoprewarm_dump_now()"
ERROR: requested DSM segment failed initialization

There is no likely way to actually delete the boken
entry at that point, and even so, I am not sure how
useful that will be.

Maybe we can improve the error message by showing
the "name", like this, In case, there are nested calls.

```
else if (!entry->initialized)
{
ereport(ERROR,
(errmsg("requested DSM segment \"%s\" failed initialization", name)));
}
```

```
psql -c "SELECT autoprewarm_dump_now()"
ERROR: requested DSM segment "autoprewarm" failed initialization
```

> > 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.

yes, I agree it should be in shared memory, and I did mention that
point in the v1 commit message, but was not sure if it was worth
doing so.

> 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.
>
> 0002 does this.

v2 LGTM.

--
Sami Imseih
Amazon Web Services (AWS)