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
