Re: Enhancing Memory Context Statistics Reporting
| От | Rahila Syed |
|---|---|
| Тема | Re: Enhancing Memory Context Statistics Reporting |
| Дата | |
| Msg-id | CAH2L28tUUf8o0Ec-5d8XPe=hno3A9Ob4nQ7TgrM1Qt_vJ0Oo2Q@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Enhancing Memory Context Statistics Reporting (Rahila Syed <rahilasyed90@gmail.com>) |
| Список | pgsql-hackers |
Hi,
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 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.
Memory Leak in TopMemoryContext: A memory leak was reported in the TopMemoryContext and there were
PFA the updated and rebased patches.
To summarize the work done in this thread,
here are some key concerns discussed in threads [1] and [2] and steps taken to address those:
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.
Memory Leak in TopMemoryContext: A memory leak was reported in the TopMemoryContext and there were
concerns that memory allocated while executing the memory statistics reporting function could impact its output.
To address this, we create an exclusive memory context under the NULL context to handle all memory allocations in
`ProcessGetMemoryContextInterrupt`. This context does not fall under the TopMemoryContext tree, ensuring that
allocations do not affect the function's outcome. The memory context is reset at the end of the function, preventing
leaks.
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.
Other Improvements:
1. Simplified the user interface by removing the `timeout` argument and using a constant value instead.
2. Provided a default for the `get_summary` argument so users do not need to pass a value if they choose not to.
3. The dsm_registry APIs are used to create and attach to DSA and DSHASH tables, which helps avoid code duplication.
To address this, we create an exclusive memory context under the NULL context to handle all memory allocations in
`ProcessGetMemoryContextInterrupt`. This context does not fall under the TopMemoryContext tree, ensuring that
allocations do not affect the function's outcome. The memory context is reset at the end of the function, preventing
leaks.
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.
Other Improvements:
1. Simplified the user interface by removing the `timeout` argument and using a constant value instead.
2. Provided a default for the `get_summary` argument so users do not need to pass a value if they choose not to.
3. The dsm_registry APIs are used to create and attach to DSA and DSHASH tables, which helps avoid code duplication.
4. Replaced the static shared memory array with a DSHASH table, which holds metadata such as pointers to memory
containing statistics for each process.
5. The updates previously made to mcxt.c have been moved to mcxtfuncs.c, which now includes all the existing memory
statistics functions as well as the code for the new proposed function6. One function that relies on an unexported API is added in mcxt.c
Вложения
В списке pgsql-hackers по дате отправления: