> > and instead reuse the existing static hash table, which is > > capped at 128 custom wait events: > > > > ``` > > #define WAIT_EVENT_CUSTOM_HASH_MAX_SIZE 128 > > ``` > > That's probably still high enough, thoughts?
I have no reason to believe that this number could be too low.
I am not aware of an extension that will initialize more than a
couple of LWLocks.
> > or maybe we can just allow WaitEventCustomNew to take in the eventId, and
> > if it's > 0, then use the passed in value, otherwise generate the next
> > eventId.
> >
> > I do like the latter approach more, what do you think?
>
> I think I do prefer it too, but in both cases we'll have to make sure there
> is no collision on the eventID (LWTRANCHE_FIRST_USER_DEFINED is currently
> 95).
As far as collisions are concerned, the key of the hash is the wait_event_info,
which is a bitwise OR of classId and eventId
```
wait_event_info = classId | eventId;
```
Do you think collision can still be possible?
Now, what I think will be a good API is to provide an alternative to
LWLockRegisterTranche,
which now takes in both a tranche ID and tranche_name. The new API I propose is
LWLockRegisterTrancheWaitEventCustom which takes only a tranche_name
and internally
calls WaitEventCustomNew to add a new wait_event_info to the hash
table. The wait_event_info
is made up of classId = PG_WAIT_LWLOCK and LWLockNewTrancheId().
I prefer we implement a new API for this to make it explicit that this
API will both register
a tranche and create a custom wait event. I do not think we should get
rid of LWLockRegisterTranche
because it is used by CreateLWLocks during startup and I don't see a
reason to change that.
See the attached v1.
What is missing from the attached v1 is:
1/ the documentation changes required in [0].
2/ in dsm_registry.c, we are going to have to modify GetNamedDSHash and
GetNamedDSA to use the new API. Here is an example of how that looks like
```
@@ -389,14 +385,12 @@ GetNamedDSHash(const char *name, const
dshash_parameters *params, bool *found)
entry->type = DSMR_ENTRY_TYPE_DSH;
/* Initialize the LWLock tranche for the DSA. */
- dsa_state->tranche = LWLockNewTrancheId();
sprintf(dsa_state->tranche_name, "%s%s", name,
DSMR_DSA_TRANCHE_SUFFIX);
- LWLockRegisterTranche(dsa_state->tranche,
dsa_state->tranche_name);
+ dsa_state->tranche =
LWLockRegisterTrancheWaitEventCustom(dsa_state->tranche_name);
```
Is there a concern with a custom wait event to be created implicitly
via the GetNamed* APIs?
3/ I still did not workout the max length requirement of the tranche
name, but I think
it would have to be as large as the GetNamed* APIs from the dsm
registry support.
I hope with v1, we can agree to the general direction of this change.
[0] https://www.postgresql.org/docs/current/xfunc-c.html
--
Sami
v1-0001-Create-LWLock-tranche-in-shared-memory.patch
Description: Binary data
