Thread: Re: pgsql: Add function to get memory context stats for processes
On Tue, Apr 8, 2025 at 5:10 AM Daniel Gustafsson <dgustafsson@postgresql.org> wrote: > Add function to get memory context stats for processes Apologies if this has already been discussed, but what is the argument that it is safe to do everything in ProcessGetMemoryContextInterrupt() at an arbitrary CHECK_FOR_INTERRUPTS() call? We have CHECK_FOR_INTERRUPTS() calls in some quite low-level places, such as walkdir() and copydir(). I don't think there's any guarantee that it's safe to perform DSA operations at an arbitrary place where CHECK_FOR_INTERRUPTS() is called, and I'm not even quite sure that it's safe to assume that the local memory-context tree is in a consistent state when CHECK_FOR_INTERRUPTS() is called. If there is some existing discussion of this that I should read, please point me in the right direction; I didn't see anything in a quick look through the commit. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
Hi, On 2025-04-10 09:31:00 -0400, Robert Haas wrote: > On Tue, Apr 8, 2025 at 5:10 AM Daniel Gustafsson > <dgustafsson@postgresql.org> wrote: > > Add function to get memory context stats for processes > > Apologies if this has already been discussed, but what is the argument > that it is safe to do everything in ProcessGetMemoryContextInterrupt() > at an arbitrary CHECK_FOR_INTERRUPTS() call? We have > CHECK_FOR_INTERRUPTS() calls in some quite low-level places, such as > walkdir() and copydir(). I don't think there's any guarantee that it's > safe to perform DSA operations at an arbitrary place where > CHECK_FOR_INTERRUPTS() is called, and I'm not even quite sure that > it's safe to assume that the local memory-context tree is in a > consistent state when CHECK_FOR_INTERRUPTS() is called. I don't know of existing discussion, but it seems rather fundamental to me - if either DSA or memory contexts could be inconsistent at a CFI(), how could it possibly be safe to interrupt at that point? After all, after an error you need to be able to reset the memory contexts / release memory in a dsa/dshash/whatnot? Memory context reset requires walking over the allocations made in the context, similar releasing a dsa? Greetings, Andres Freund
On Thu, Apr 10, 2025 at 4:05 PM Andres Freund <andres@anarazel.de> wrote: > I don't know of existing discussion, but it seems rather fundamental to me - > if either DSA or memory contexts could be inconsistent at a CFI(), how could > it possibly be safe to interrupt at that point? After all, after an error you > need to be able to reset the memory contexts / release memory in a > dsa/dshash/whatnot? Memory context reset requires walking over the allocations > made in the context, similar releasing a dsa? I think it would be a bit surprising if somebody put a CHECK_FOR_INTERRUPTS() inside aset.c or similar, but I don't see a reason why we couldn't end up with one reachable via the DSA code. DSA calls DSM which depending on dynamic_shared_memory_type might involve filesystem operations. That's a fairly large amount of code. I admit I have no particular theory about how CFI could be reachable from there today, but even if it definitely isn't, I don't see why someone would hesitate to add one in the future. -- Robert Haas EDB: http://www.enterprisedb.com
Hi, On 2025-04-14 10:03:28 -0400, Robert Haas wrote: > On Thu, Apr 10, 2025 at 4:05 PM Andres Freund <andres@anarazel.de> wrote: > > I don't know of existing discussion, but it seems rather fundamental to me - > > if either DSA or memory contexts could be inconsistent at a CFI(), how could > > it possibly be safe to interrupt at that point? After all, after an error you > > need to be able to reset the memory contexts / release memory in a > > dsa/dshash/whatnot? Memory context reset requires walking over the allocations > > made in the context, similar releasing a dsa? > > I think it would be a bit surprising if somebody put a > CHECK_FOR_INTERRUPTS() inside aset.c or similar, but I don't see a > reason why we couldn't end up with one reachable via the DSA code. DSA > calls DSM which depending on dynamic_shared_memory_type might involve > filesystem operations. That's a fairly large amount of code. I admit I > have no particular theory about how CFI could be reachable from there > today, but even if it definitely isn't, I don't see why someone would > hesitate to add one in the future. There very well could be a CFI - but it better be somewhere where the in-memory state is consistent. Otherwise an error inside raised in the CFI would lead the in-memory state inconsistent which then would cause problems when cleaning up the dsa during resowner release or process exit. What am I missing here? Greetings, Andres Freund