Date: Mon, 1 Dec 2025 17:04:23 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Rahila Syed <rahilasyed90@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: show size of DSAs and dshash tables in
 pg_dsm_registry_allocations
Message-ID: <aS4e9z74V-Vn7NEi@nathan>
References: <aSeEDeznAsHR1_YF@nathan>
 <CAH2L28tr5q965MbD2TroyTc-Ey+b-z3=3kpWAuGg6w7qQwwChA@mail.gmail.com>
 <aS20Z42rPHecTljS@nathan>
 <E4CEC5F7-BCCE-47D7-B206-48FD31F06F3C@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="jJt2Y/LqGp3oC5LK"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <E4CEC5F7-BCCE-47D7-B206-48FD31F06F3C@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aS4e9z74V-Vn7NEi%40nathan>
Precedence: bulk


--jJt2Y/LqGp3oC5LK
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On Tue, Dec 02, 2025 at 05:35:34AM +0800, Chao Li wrote:
> V2 overall looks good to me. I just got a question and a few nit comments.

Thanks for reviewing.

> Why do you error out instead of reporting 0/NULL usage here?

If attaching to the control segment fails, something has gone horribly
wrong.  I don't think returning 0 for the size is appropriate in that case.
Also note that other functions in this file ERROR when attaching fails
(e.g., dsa_attach()).

> ```
> +		else if (entry->type == DSMR_ENTRY_TYPE_DSH &&
> +				 entry->dsh.dsa_handle !=DSA_HANDLE_INVALID)
> ```
> 
> Missing a white space after !=.

I agree, but for some reason, pgindent insists on removing that space.  I'm
leaving that for another thread.

> ```
> -       Size of the allocation in bytes.  NULL for entries of type
> -       <literal>area</literal> and <literal>hash</literal>.
> +       Size of the allocation in bytes.  NULL for entries that failed
> +       initialization.
> ```
> 
> “Failed initialization”, I guess “to” is needed after “failed”.

IMHO it's fine as-is.  We could say "failed to initialize", but I'm not
sure that's materially better at conveying the information.

> ```
> -SELECT name, type, size IS DISTINCT FROM 0 AS size
> +SELECT name, type, size > 0 AS size
> ```
> 
> As you changed the third column from “size” to “size > 0”, now it’s a
> bool column, so maybe rename the column alias from “size” to something
> indicating a bool type, such as “size_ok”, “has_size”, etc.

It was already a bool column, but your point still stands.  I changed it to
"size_ok".

-- 
nathan

--jJt2Y/LqGp3oC5LK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v3-0001-rework-DSM-registry-view.patch

From f28e45a16271265a7d79b4e00594b5a515909df0 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 26 Nov 2025 16:48:13 -0600
Subject: [PATCH v3 1/1] rework DSM registry view

---
 doc/src/sgml/system-views.sgml                |  4 +--
 src/backend/storage/ipc/dsm_registry.c        | 22 ++++--------
 src/backend/utils/mmgr/dsa.c                  | 35 +++++++++++++++++++
 src/include/utils/dsa.h                       |  1 +
 .../expected/test_dsm_registry.out            | 12 +++----
 .../sql/test_dsm_registry.sql                 |  4 +--
 6 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml
index 7db8f73eba2..162c76b729a 100644
--- a/doc/src/sgml/system-views.sgml
+++ b/doc/src/sgml/system-views.sgml
@@ -1150,8 +1150,8 @@ AND c1.path[c2.level] = c2.path[c2.level];
        <structfield>size</structfield> <type>int8</type>
       </para>
       <para>
-       Size of the allocation in bytes.  NULL for entries of type
-       <literal>area</literal> and <literal>hash</literal>.
+       Size of the allocation in bytes.  NULL for entries that failed
+       initialization.
       </para></entry>
      </row>
     </tbody>
diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c
index ef6533b1100..0f3e08c44d8 100644
--- a/src/backend/storage/ipc/dsm_registry.c
+++ b/src/backend/storage/ipc/dsm_registry.c
@@ -463,26 +463,18 @@ pg_get_dsm_registry_allocations(PG_FUNCTION_ARGS)
 		Datum		vals[3];
 		bool		nulls[3] = {0};
 
-		/* Do not show partially-initialized entries. */
-		if (entry->type == DSMR_ENTRY_TYPE_DSM &&
-			entry->dsm.handle == DSM_HANDLE_INVALID)
-			continue;
-		if (entry->type == DSMR_ENTRY_TYPE_DSA &&
-			entry->dsa.handle == DSA_HANDLE_INVALID)
-			continue;
-		if (entry->type == DSMR_ENTRY_TYPE_DSH &&
-			entry->dsh.dsa_handle == DSA_HANDLE_INVALID)
-			continue;
-
 		vals[0] = CStringGetTextDatum(entry->name);
 		vals[1] = CStringGetTextDatum(DSMREntryTypeNames[entry->type]);
 
-		/*
-		 * Since we can't know the size of DSA/dshash entries without first
-		 * attaching to them, return NULL for those.
-		 */
+		/* Be careful to only return the sizes of initialized entries. */
 		if (entry->type == DSMR_ENTRY_TYPE_DSM)
 			vals[2] = Int64GetDatum(entry->dsm.size);
+		else if (entry->type == DSMR_ENTRY_TYPE_DSA &&
+				 entry->dsa.handle != DSA_HANDLE_INVALID)
+			vals[2] = Int64GetDatum(dsa_get_total_size_from_handle(entry->dsa.handle));
+		else if (entry->type == DSMR_ENTRY_TYPE_DSH &&
+				 entry->dsh.dsa_handle !=DSA_HANDLE_INVALID)
+			vals[2] = Int64GetDatum(dsa_get_total_size_from_handle(entry->dsh.dsa_handle));
 		else
 			nulls[2] = true;
 
diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c
index be43e9351c3..030f9463712 100644
--- a/src/backend/utils/mmgr/dsa.c
+++ b/src/backend/utils/mmgr/dsa.c
@@ -1050,6 +1050,41 @@ dsa_get_total_size(dsa_area *area)
 	return size;
 }
 
+/*
+ * Same as dsa_get_total_size(), but accepts a DSA handle.  The area must have
+ * been created with dsa_create (not dsa_create_in_place).
+ */
+size_t
+dsa_get_total_size_from_handle(dsa_handle handle)
+{
+	size_t		size;
+	bool		already_attached;
+	dsm_segment *segment;
+	dsa_area_control *control;
+
+	already_attached = dsa_is_attached(handle);
+	if (already_attached)
+		segment = dsm_find_mapping(handle);
+	else
+		segment = dsm_attach(handle);
+
+	if (segment == NULL)
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("could not attach to dynamic shared area")));
+
+	control = (dsa_area_control *) dsm_segment_address(segment);
+
+	LWLockAcquire(&control->lock, LW_SHARED);
+	size = control->total_segment_size;
+	LWLockRelease(&control->lock);
+
+	if (!already_attached)
+		dsm_detach(segment);
+
+	return size;
+}
+
 /*
  * Aggressively free all spare memory in the hope of returning DSM segments to
  * the operating system.
diff --git a/src/include/utils/dsa.h b/src/include/utils/dsa.h
index f2104dacbfc..42449ff22de 100644
--- a/src/include/utils/dsa.h
+++ b/src/include/utils/dsa.h
@@ -161,6 +161,7 @@ extern dsa_pointer dsa_allocate_extended(dsa_area *area, size_t size, int flags)
 extern void dsa_free(dsa_area *area, dsa_pointer dp);
 extern void *dsa_get_address(dsa_area *area, dsa_pointer dp);
 extern size_t dsa_get_total_size(dsa_area *area);
+extern size_t dsa_get_total_size_from_handle(dsa_handle handle);
 extern void dsa_trim(dsa_area *area);
 extern void dsa_dump(dsa_area *area);
 
diff --git a/src/test/modules/test_dsm_registry/expected/test_dsm_registry.out b/src/test/modules/test_dsm_registry/expected/test_dsm_registry.out
index ca8abbb377e..9128e171b1b 100644
--- a/src/test/modules/test_dsm_registry/expected/test_dsm_registry.out
+++ b/src/test/modules/test_dsm_registry/expected/test_dsm_registry.out
@@ -1,8 +1,8 @@
-SELECT name, type, size IS DISTINCT FROM 0 AS size
+SELECT name, type, size > 0 AS size_ok
 FROM pg_dsm_registry_allocations
 WHERE name like 'test_dsm_registry%' ORDER BY name;
- name | type | size 
-------+------+------
+ name | type | size_ok 
+------+------+---------
 (0 rows)
 
 CREATE EXTENSION test_dsm_registry;
@@ -32,11 +32,11 @@ SELECT get_val_in_hash('test');
 (1 row)
 
 \c
-SELECT name, type, size IS DISTINCT FROM 0 AS size
+SELECT name, type, size > 0 AS size_ok
 FROM pg_dsm_registry_allocations
 WHERE name like 'test_dsm_registry%' ORDER BY name;
-          name          |  type   | size 
-------------------------+---------+------
+          name          |  type   | size_ok 
+------------------------+---------+---------
  test_dsm_registry_dsa  | area    | t
  test_dsm_registry_dsm  | segment | t
  test_dsm_registry_hash | hash    | t
diff --git a/src/test/modules/test_dsm_registry/sql/test_dsm_registry.sql b/src/test/modules/test_dsm_registry/sql/test_dsm_registry.sql
index 965a3f1ebb6..a606e8872a1 100644
--- a/src/test/modules/test_dsm_registry/sql/test_dsm_registry.sql
+++ b/src/test/modules/test_dsm_registry/sql/test_dsm_registry.sql
@@ -1,4 +1,4 @@
-SELECT name, type, size IS DISTINCT FROM 0 AS size
+SELECT name, type, size > 0 AS size_ok
 FROM pg_dsm_registry_allocations
 WHERE name like 'test_dsm_registry%' ORDER BY name;
 CREATE EXTENSION test_dsm_registry;
@@ -8,6 +8,6 @@ SELECT set_val_in_hash('test', '1414');
 SELECT get_val_in_shmem();
 SELECT get_val_in_hash('test');
 \c
-SELECT name, type, size IS DISTINCT FROM 0 AS size
+SELECT name, type, size > 0 AS size_ok
 FROM pg_dsm_registry_allocations
 WHERE name like 'test_dsm_registry%' ORDER BY name;
-- 
2.39.5 (Apple Git-154)


--jJt2Y/LqGp3oC5LK--