From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | Florents Tselai <florents(dot)tselai(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: like pg_shmem_allocations, but fine-grained for DSM registry ? |
Date: | 2025-06-03 13:52:58 |
Message-ID: | aD7-Ora7hH_9ywi0@nathan |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Mar 15, 2025 at 10:41:15AM +0200, Florents Tselai wrote:
> Here's an updated v3 that fixes some white spaces v2 had-no other changes.
Thanks for the patch.
> I'm wondering though if it also makes sense to expose:
> - backend_pid (who created the segment)
> - is_pinned bool
> - created_at
created_at might be interesting. I'm not sure the others have much use.
It would be cool to surface which library/function created the segment
IMHO. But for now, I'd keep the view simple and just show the contents of
the DSM registry's hash table.
+CREATE VIEW pg_dsm_registry AS
+SELECT * FROM pg_get_dsm_registry();
I'd suggest pg_dsm_registry_allocations and
pg_get_dsm_registry_allocations() to match pg_shmem_allocations.
+void
+iterate_dsm_registry(void (*callback)(DSMRegistryEntry *, void *), void *arg);
+void
+iterate_dsm_registry(void (*callback)(DSMRegistryEntry *, void *), void *arg)
+{
I'm not sure what's going on here. Presumably we could just mark
iterate_dsm_registry() as static.
Taking a step back, I think the loop is quite overengineered. You have a
function for calling dshash_seq_init/next/term, a callback function for
storing the tuple, and a special struct for some SRF context. I don't see
any need to abstract things to this extent. I think we should instead
open-code the loop in pg_get_dsm_registry().
+/* SQL SRF showing DSM registry allocated memory */
+PG_FUNCTION_INFO_V1(pg_get_dsm_registry);
There should be no need to do this. Its pg_proc.dat entry will
automatically generate the required prototype.
+ if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))
+ ereport(ERROR, (errmsg("pg_get_dsm_registry must be used in a SRF context")));
+
+ /* Set up tuple descriptor */
+ tupdesc = CreateTemplateTupleDesc(2);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name", TEXTOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 2, "size", INT8OID, -1, 0);
This SRF-related code can be simplified by using InitMaterializedSRF() and
friends. Check out pg_ls_dir() in genfile.c for an example.
+-- 20 bytes = int (4 bytes) + LWLock (16bytes)
+SELECT * FROM pg_dsm_registry;
+ name | size
+-------------------+------
+ test_dsm_registry | 20
+(1 row)
I'm a little worried about the portability of this test. I would suggest
changing the query to something like
SELECT size > 0 FROM pg_dsm_registry WHERE name = 'test_dsm_registry';
--
nathan
From | Date | Subject | |
---|---|---|---|
Next Message | Andrei Lepikhov | 2025-06-03 13:53:50 | Re: MergeAppend could consider sorting cheapest child path |
Previous Message | Alexander Korotkov | 2025-06-03 13:38:47 | Re: MergeAppend could consider sorting cheapest child path |