Обсуждение: Duplicate RequestNamedLWLocktranche() names and test_lwlock_tranches improvements

Поиск
Список
Период
Сортировка

Duplicate RequestNamedLWLocktranche() names and test_lwlock_tranches improvements

От
Heikki Linnakangas
Дата:
Starting new thread for this thing that Matthias noticed in my 
work-in-progress patch at 
https://www.postgresql.org/message-id/CAEze2WjgCROMMXY0+j8FFdm3iFcr7By-+6Mwiz=PgGSEydiW3A@mail.gmail.com.

On 05/04/2026 02:17, Matthias van de Meent wrote:
> 0006: I don't think it is a great idea to make the LwLock machinery
> the first to get allocation requests:
> It has the RequestNamedLWLockTranche infrastructure, which can only
> register new requests while process_shmem_requests_in_progress, and
> making it request its memory ahead of everything else is likely to
> cause an undersized tranche to be allocated. You could make sure that
> this isn't an issue by maintaining a flag in lwlock.c that's set when
> the shmem request is made (and reset on shmem exit), which must be
> false when RequestNamedLWLockTranche() is called, and if not then it
> should throw an error.

Good catch, RequestNamedLWLocktranche() was quite broken with the patch. 
I'm surprised it didn't cause test failures. We even have unit tests for 
that at src/test/modules/test_lwlock_tranches.

Looking at src/test/modules/test_lwlock_tranches, I realized that we 
don't currently check that the tranche name registered with 
RequestNamedLWLocktranche() is unique. If two extensions registered a 
tranche with same name, we'd allocate two separate tranches for them, 
but GetNamedLWLockTranche() would always return the first one.

Attached patches add a uniqueness check, and improves 
test_lwlock_tranches so that it actually uses the requested LWLocks. And 
I couldn't resist doing some more refactoring of the test while I was at 
it; IMO it's more readable now.

Barring objections, I will commit these shortly.

- Heikki
Вложения

Starting new thread for this thing that Matthias noticed in my
work-in-progress patch at
https://www.postgresql.org/message-id/CAEze2WjgCROMMXY0+j8FFdm3iFcr7By-+6Mwiz=PgGSEydiW3A@mail.gmail.com.

On 05/04/2026 02:17, Matthias van de Meent wrote:
> 0006: I don't think it is a great idea to make the LwLock machinery
> the first to get allocation requests:
> It has the RequestNamedLWLockTranche infrastructure, which can only
> register new requests while process_shmem_requests_in_progress, and
> making it request its memory ahead of everything else is likely to
> cause an undersized tranche to be allocated. You could make sure that
> this isn't an issue by maintaining a flag in lwlock.c that's set when
> the shmem request is made (and reset on shmem exit), which must be
> false when RequestNamedLWLockTranche() is called, and if not then it
> should throw an error.

Good catch, RequestNamedLWLocktranche() was quite broken with the patch.
I'm surprised it didn't cause test failures. We even have unit tests for
that at src/test/modules/test_lwlock_tranches.

Looking at src/test/modules/test_lwlock_tranches, I realized that we
don't currently check that the tranche name registered with
RequestNamedLWLocktranche() is unique. If two extensions registered a
tranche with same name, we'd allocate two separate tranches for them,
but GetNamedLWLockTranche() would always return the first one.

Yes, while that is not very likely scenario, it’s wrong. The caller will get the wrong pointer to the locks.


Attached patches add a uniqueness check, and improves
test_lwlock_tranches so that it actually uses the requested LWLocks. And
I couldn't resist doing some more refactoring of the test while I was at
it; IMO it's more readable now.

Barring objections, I will commit these shortly.

LGTM

--
Sami Imseih
Amazon Web Services (AWS)



Re: Duplicate RequestNamedLWLocktranche() names and test_lwlock_tranches improvements

От
Heikki Linnakangas
Дата:
On 05/04/2026 19:05, Sami Imseih wrote:
> LGTM

Committed, thanks!

- Heikki