| From: | Rahila Syed <rahilasyed90(at)gmail(dot)com> |
|---|---|
| To: | Daniel Gustafsson <daniel(at)yesql(dot)se>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Enhancing Memory Context Statistics Reporting |
| Date: | 2025-12-29 08:56:40 |
| Message-ID: | CAH2L28vF7R08NUo6Sme8LydRaFcKhS6fu0psfxUQoxsVVqJMwA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
I've included some additional description inline and attached rebased
patches after
CFbot reported a conflict.
> *DSA APIs and CFI Handler Safety*: DSA APIs, being high-level, are unsafe
> to call from the CFI handler,
> which can be invoked from low-level code. This concern was particularly
> raised for APIs like `dsa_allocate()`
> and `dsa_create()`.
> To resolve this, these APIs have been moved out of the CFI handler
> function. Now, the dynamic shared memory
> needed to store the statistics is allocated and deleted in the client
> function. The only operation performed in the CFI
> handler is `dsm_attach()`, which attaches to DSA for copying statistics.
> Since dsm_attach() only maps the existing
> DSM into the current process address space and does not create a new DSM,
> I don't see any specific reason why
> it would be unsafe to call it from the CFI handler.
>
>
Following are the details about the use of DSM in the patch:
- DSA Creation: A Dynamic Shared Area (DSA) is used to store memory context
statistics.
- Client Process: When fetching memory context statistics, the client
allocates a 1 MB chunk
in the DSA, reads the statistics from the memory chunk, copies it into a
tuple store, and then
frees the chunk.
- Storage: Pointers to these chunks are stored in a DSHASH table indexed by
the client’s
proc number. Each entry in the DSHASH table also stores additional
metadata related to the
client’s request.
- Attachment: Backends only attach to the DSM segments for the DSA and
DSHASH table
when necessary i.e when a process queries memory context statistics or is
queried by
another backend.
Once attached, they remain so until the session ends, at which point they
remove their DSHASH
entry if any and detach from DSA and DSHASH segments.
- Lifecycle: The DSA and DSHASH structures are created upon the first SQL
function invocation
and destroyed on server restart.
> *Error Reported in Thread [2]*: This issue has been fixed by switching to
> a NULL resource owner before attaching
> to DSM in the CFI handler.
>
>
This error mentioned in thread [2] is triggered during CFI() call from
secure_read() when a
backend is waiting for commands and it has an open transaction which is
going to abort
Below are some details about this fix.
It is safe to temporarily set the resource owner to NULL before attaching
to the DSA
and DSHASH, since these segments are intended to be attached for the full
session
and are detached only when the session ends.
We also restore the original resource owner immediately after the attach
completes.
Other possible fixes include:
1.Adjusting resource‑owner behavior
Either allow resource‑owner enlargement during release, or delay marking it
as releasing until
the abort actually begins.
2. Updating DSM registry APIs (e.g., GetNamedDSA)
Detect when the current resource owner is in a releasing state and
temporarily set
CurrentResourceOwner to NULL before calling dsa_attach.
3. Handling it in the DSA layer
This was discussed in thread [2], but concerns were raised that DSA should
not compensate
for incorrect caller state; the caller must ensure the resource owner is
valid.
Kindly let me know your views.
Thank you,
Rahila Syed
[2]. PostgreSQL: Re: Prevent an error on attaching/creating a DSM/DSA from
an interrupt handler
<https://www.postgresql.org/message-id/flat/8B873D49-E0E5-4F9F-B8D6-CA4836B825CD%40yesql.se#7026d2fe4ab0de6dd5decd32eb9c585a>
| Attachment | Content-Type | Size |
|---|---|---|
| v46-0002-Test-module-to-test-memory-context-reporting-wit.patch | application/octet-stream | 8.8 KB |
| v46-0001-Add-function-to-report-memory-context-statistics.patch | application/octet-stream | 57.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bertrand Drouvot | 2025-12-29 08:58:08 | Re: More const-marking cleanup |
| Previous Message | Dmitry Koval | 2025-12-29 08:43:00 | Re: Assert when executing query on partitioned table |