Re: add function for creating/attaching hash table in DSM registry
От | Sami Imseih |
---|---|
Тема | Re: add function for creating/attaching hash table in DSM registry |
Дата | |
Msg-id | CAA5RZ0sRzBoWgDxioNXQRcMj+gsOhDy4U44jbCStmU5QBL47bw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: add function for creating/attaching hash table in DSM registry (Nathan Bossart <nathandbossart@gmail.com>) |
Ответы |
Re: add function for creating/attaching hash table in DSM registry
|
Список | pgsql-hackers |
Thanks, I tested v2 a bit more and did a quick hack of pg_stat_statements just to get a feel for what it would take to use the new API to move the hash table from static to dynamic. One thing I noticed, and I’m not too fond of, is that the wait_event name shows up with the _dsh suffix: ``` postgres=# select query, wait_event, wait_event_type from pg_stat_activity ; query | wait_event | wait_event_type -------------------------------------------------------------------+------------------------ +----------------- select 1; | pg_stat_statements_dsh | LWLock ``` So the suffix isn’t just an internal name, it’s user-facing now. If we really need to keep this behavior, I think it’s important to document it clearly in the code. A few nits also: 1/ + +static dshash_table *tdr_hash; + Isn't it better to initialize this to NULL? 2/ The comment: ``` params is ignored; a new tranche ID will be generated if needed. ``` The "if needed" part isn't necessary here. A new tranche ID will always be generated, right? 3/ GetNamedDSMSegment is called with "found" as the last argument: ``` state = GetNamedDSMSegment(name, sizeof(NamedDSMHashState), NULL, found); ``` I think it should use a different bool here instead of "found", since that value isn’t really needed from GetNamedDSMSegment. Also, it refers to whether the dynamic hash was found, and is set later in the routine. -- Sami
В списке pgsql-hackers по дате отправления: