Re: Improve LWLock tranche name visibility across backends

Hi,

I've begun reviewing this patch and have a few questions listed below:

1. + if (i < LWLockTrancheNames.shmem->allocated &&
DsaPointerIsValid(old_ptrs[i]))

Should an assert be used for the second condition instead?
Since for i < LWLockTrancheNames.shmem->allocated, the dsa pointer is
expected to be valid.

2. copied_ptr =
dsa_allocate(LWLockTrancheNames.dsa, len);
+
+ copied_addr =
dsa_get_address(LWLockTrancheNames.dsa, copied_ptr);
+ memcpy(copied_addr, old_name, len);
+
+ new_ptrs[i] = copied_ptr;
+
+ /* free old tranche names */
+ dsa_free(LWLockTrancheNames.dsa,
old_ptrs[i]);

Why is it necessary to allocate a new dsa_pointer for tranche names that
are the same size and then
free the old one?
Is there a reason we can't just assign new_ptrs[i] = old_ptrs[i]?

3.
>Additionally, while users should not pass arbitrary tranche IDs (that is,
>IDs not created via LWLockNewTrancheId) to LWLockInitialize, nothing
>technically prevents them from doing so. Therefore, we must continue to
>handle such cases gracefully by returning a default "extension" tranche
name.

Would it be possible to update LWLockInitialize so that it checks if
tranche_id is
already registered in the dsa, and if not, registers it during the
LWLockInitialize() process?

Thank you,
Rahila Syed