Re: add function for creating/attaching hash table in DSM registry
От | Nathan Bossart |
---|---|
Тема | Re: add function for creating/attaching hash table in DSM registry |
Дата | |
Msg-id | aEH0qxQWpSalxT2i@nathan обсуждение исходный текст |
Ответ на | Re: add function for creating/attaching hash table in DSM registry (Sami Imseih <samimseih@gmail.com>) |
Список | pgsql-hackers |
On Thu, Jun 05, 2025 at 01:38:25PM -0500, Sami Imseih wrote: > I have a few early comments, but I plan on trying this out next. Thanks for reviewing. >> > +typedef struct NamedDSMHashState >> > +{ >> > + dsa_handle dsah; >> > + dshash_table_handle dshh; >> > + int dsa_tranche; >> > + char dsa_tranche_name[68]; /* name + "_dsa" */ >> > + int dsh_tranche; >> > + char dsh_tranche_name[68]; /* name + "_dsh" */ >> > +} NamedDSMHashState; >> >> I don't have enough knowledge to review the rest of the patch, but >> shouldn't this use NAMEDATALEN, rather than hard-coding the default >> length? I straightened this out in v2. I've resisted using NAMEDATALEN because this stuff is unrelated to the name type. But I have moved all the lengths and suffixes to macros. > NamedLWLockTrancheRequest uses NAMEDATALEN = 64 bytes for the > tranche_name > > typedef struct NamedLWLockTrancheRequest > { > char tranche_name[NAMEDATALEN]; > int num_lwlocks; > } NamedLWLockTrancheRequest; I think the NAMEDATALEN limit only applies to tranches requested at startup time. LWLockRegisterTranche() just saves whatever pointer you give it, so AFAICT there's no real limit there. > 2/ Can you group the dsa and dsh separately. I felt this was a bit > difficult to read? > > + /* Initialize LWLock tranches for the DSA and dshash table. */ > + state->dsa_tranche = LWLockNewTrancheId(); > + state->dsh_tranche = LWLockNewTrancheId(); > + sprintf(state->dsa_tranche_name, "%s_dsa", name); > + sprintf(state->dsh_tranche_name, "%s_dsh", name); > + LWLockRegisterTranche(state->dsa_tranche, > state->dsa_tranche_name); > + LWLockRegisterTranche(state->dsh_tranche, > state->dsh_tranche_name); Done. > 3/ It will be good to "Assert(dsh)" before "return dsh;" for safety? > > MemoryContextSwitchTo(oldcontext); > LWLockRelease(DSMRegistryLock); > > return dsh; Eh, I would expect the tests to start failing horribly if I managed to mess that up. -- nathan
Вложения
В списке pgsql-hackers по дате отправления: