Re: memory leak in trigger handling (since PG12)
От | Tomas Vondra |
---|---|
Тема | Re: memory leak in trigger handling (since PG12) |
Дата | |
Msg-id | 5479b188-17b1-ad6b-bf7d-fcefaf9d382d@enterprisedb.com обсуждение исходный текст |
Ответ на | Re: memory leak in trigger handling (since PG12) (Andres Freund <andres@anarazel.de>) |
Список | pgsql-hackers |
On 5/24/23 22:22, Andres Freund wrote: > Hi, > > On 2023-05-24 21:56:22 +0200, Tomas Vondra wrote: >>>> The really hard thing was determining what causes the memory leak - the >>>> simple instrumentation doesn't help with that at all. It tells you there >>>> might be a leak, but you don't know where did the allocations came from. >>>> >>>> What I ended up doing is a simple gdb script that sets breakpoints on >>>> all palloc/pfree variants, and prints info (including the backtrace) for >>>> each call on ExecutorState. And then a script that aggregate those to >>>> identify which backtraces allocated most chunks that were not freed. >>> >>> FWIW, for things like this I found "heaptrack" to be extremely helpful. >>> >>> E.g. for a reproducer of the problem here, it gave me the attach "flame graph" >>> of the peak memory usage - after attaching to a running backend and running an >>> UPDATE triggering the leak.. >>> >>> Because the view I screenshotted shows the stacks contributing to peak memory >>> usage, it works nicely to find "temporary leaks", even if memory is actually >>> freed after all etc. >>> >> >> That's a nice visualization, but isn't that useful only once you >> determine there's a memory leak? Which I think is the hard problem. > > So is your gdb approach, unless I am misunderstanding? The view I > screenshotted shows the "peak" allocated memory, if you have a potential leak, > you can see where most of the allocated memory was allocated. Which at least > provides you with a good idea of where to look for a problem in more detail. > Right, it wasn't my ambition to detect memory leaks but to source of the leak if there's one. I got a bit distracted by the discussion detecting leaks etc. > >>>>> Hm. Somehow this doesn't seem quite right. Shouldn't we try to use a shorter >>>>> lived memory context instead? Otherwise we'll just end up with the same >>>>> problem in a few years. >>>>> >>>> >>>> I agree using a shorter lived memory context would be more elegant, and >>>> more in line with how we do things. But it's not clear to me why we'd >>>> end up with the same problem in a few years with what the patch does. >>> >>> Because it sets up the pattern of manual memory management and continues to >>> run the relevant code within a query-lifetime context. >>> >> >> Oh, you mean someone might add new allocations to this code (or into one >> of the functions executed from it), and that'd leak again? Yeah, true. > > Yes. It's certainly not obvious this far down that we are called in a > semi-long-lived memory context. > That's true, but I don't see how adding a ExecGetAllUpdatedCols() variant that allocates stuff in a short-lived context improves this. That'll only cover memory allocated in ExecGetAllUpdatedCols() and nothing else. regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления:
Предыдущее
От: vignesh CДата:
Сообщение: Implement generalized sub routine find_in_log for tap test