| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | Rahila Syed <rahilasyed90(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: show size of DSAs and dshash tables in pg_dsm_registry_allocations |
| Date: | 2025-12-01 21:35:34 |
| Message-ID: | E4CEC5F7-BCCE-47D7-B206-48FD31F06F3C@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Nathan,
V2 overall looks good to me. I just got a question and a few nit comments.
> On Dec 1, 2025, at 23:29, Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> --
> nathan
> <v2-0001-rework-DSM-registry-view.patch>
1 - dsa.c
```
+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")));
```
Why do you error out instead of reporting 0/NULL usage here? When users call this function as shown in the test script:
```
CREATE VIEW pg_dsm_registry_allocations AS
SELECT * FROM pg_get_dsm_registry_allocations();
```
User doesn’t pass in a handle, if the DSA has been released, it should treat a missing mapping as “no size available” and maybe return NULL.
2
```
+ else if (entry->type == DSMR_ENTRY_TYPE_DSH &&
+ entry->dsh.dsa_handle !=DSA_HANDLE_INVALID)
```
Missing a white space after !=.
3
```
- 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”.
4
```
-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.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Daniel Gustafsson | 2025-12-01 21:51:04 | Re: Changing the state of data checksums in a running cluster |
| Previous Message | Heikki Linnakangas | 2025-12-01 21:20:16 | Re: Bug in amcheck? |