Re: add function for creating/attaching hash table in DSM registry
Sami Imseih <samimseih@gmail.com>
From: Sami Imseih <samimseih@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>, pgsql-hackers@postgresql.org
Date: 2025-06-09T20:10:30Z
Lists: 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
Commits
-
Fix cross-version upgrade test breakage from commit fe07100e82.
- 0c2b7174c362 19 (unreleased) landed
-
Add GetNamedDSA() and GetNamedDSHash().
- fe07100e82b0 19 (unreleased) landed
-
Remove dynamic_shared_memory_type=none
- bcbd940806a2 12.0 cited
-
Introduce dynamic shared memory areas.
- 13df76a537cc 10.0 cited
-
Relax the requirement that all lwlocks be stored in a single array.
- ea9df812d850 9.4.0 cited