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


Reply via email to