Re: like pg_shmem_allocations, but fine-grained for DSM registry ?

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

In response to

Responses

Browse pgsql-hackers by date

  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