On 6/23/23 08:03, Alexander Pyhalov wrote:
> Tomas Vondra писал 2023-06-22 17:16:
>> On 6/22/23 13:46, Tomas Vondra wrote:
>>> ...
>>>
>>> I haven't tried the reproducer, but I think I see the issue - we store
>>> the bitmap as part of the event to be executed later, but the bitmap is
>>> in per
Tomas Vondra писал 2023-06-22 17:16:
On 6/22/23 13:46, Tomas Vondra wrote:
...
I haven't tried the reproducer, but I think I see the issue - we store
the bitmap as part of the event to be executed later, but the bitmap
is
in per-tuple context and gets reset. So I guess we need to copy it
into
On 6/22/23 13:46, Tomas Vondra wrote:
> ...
>
> I haven't tried the reproducer, but I think I see the issue - we store
> the bitmap as part of the event to be executed later, but the bitmap is
> in per-tuple context and gets reset. So I guess we need to copy it into
> the proper long-lived context
On 6/22/23 13:07, Alexander Pyhalov wrote:
> Tomas Vondra писал 2023-05-25 17:41:
>
>> The attached patch does this - I realized we actually have estate in
>> ExecGetAllUpdatedCols(), so we don't even need a variant with a
>> different signature. That makes the patch much simpler.
>>
>> The que
Tomas Vondra писал 2023-05-25 17:41:
The attached patch does this - I realized we actually have estate in
ExecGetAllUpdatedCols(), so we don't even need a variant with a
different signature. That makes the patch much simpler.
The question is whether we need the signature anyway. There might be
On 5/25/23 16:41, Tomas Vondra wrote:
> ...
>
> The attached patch does this - I realized we actually have estate in
> ExecGetAllUpdatedCols(), so we don't even need a variant with a
> different signature. That makes the patch much simpler.
>
> The question is whether we need the signature anyway.
On 5/24/23 22:19, Andres Freund wrote:
>
> ...
>
> Hm - stepping back a bit, why are we doing the work in ExecGetAllUpdatedCols()
> over and over? Unless I am missing something, the result doesn't change
> across rows. And it doesn't look that cheap to compute, leaving aside the
> allocation that
On 5/24/23 21:49, Tomas Vondra wrote:
>
>
> On 5/24/23 20:55, Andres Freund wrote:
>> Hi,
>>
>> On 2023-05-24 15:38:41 +0200, Tomas Vondra wrote:
>>> I looked at this again, and I think GetPerTupleMemoryContext(estate)
>>> might do the trick, see the 0002 part.
>>
>> Yea, that seems like the ri
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
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.
> >>
> >> W
Hi,
On 2023-05-24 21:49:13 +0200, Tomas Vondra wrote:
> >> and then have to pass updatedCols elsewhere. It's tricky to just switch
> >> to the context (e.g. in ExecASUpdateTriggers/ExecARUpdateTriggers), as
> >> AfterTriggerSaveEvent allocates other bits of memory too (in a longer
> >> lived conte
On 5/24/23 20:14, Andres Freund wrote:
> Hi,
>
> On 2023-05-23 23:26:42 +0200, Tomas Vondra wrote:
>> On 5/23/23 19:14, Andres Freund wrote:
>>> Hi,
>>>
>>> On 2023-05-23 18:23:00 +0200, Tomas Vondra wrote:
This means that for an UPDATE with triggers, we may end up calling this
for each
Hi,
On 2023-05-23 18:23:00 +0200, Tomas Vondra wrote:
> While looking for other places allocating stuff in ExecutorState (for
> the UPDATE case) and leaving it there, I found two more cases:
For a bit I thought there was a similar problem in ExecWithCheckOptions() -
but we error out immediately a
On 5/24/23 20:55, Andres Freund wrote:
> Hi,
>
> On 2023-05-24 15:38:41 +0200, Tomas Vondra wrote:
>> I looked at this again, and I think GetPerTupleMemoryContext(estate)
>> might do the trick, see the 0002 part.
>
> Yea, that seems like the right thing here.
>
>
>> Unfortunately it's not mu
Hi,
On 2023-05-24 15:38:41 +0200, Tomas Vondra wrote:
> I looked at this again, and I think GetPerTupleMemoryContext(estate)
> might do the trick, see the 0002 part.
Yea, that seems like the right thing here.
> Unfortunately it's not much
> smaller/simpler than just freeing the chunks, because
Hi,
On 2023-05-23 23:26:42 +0200, Tomas Vondra wrote:
> On 5/23/23 19:14, Andres Freund wrote:
> > Hi,
> >
> > On 2023-05-23 18:23:00 +0200, Tomas Vondra wrote:
> >> This means that for an UPDATE with triggers, we may end up calling this
> >> for each row, possibly multiple bitmaps. And those bit
On 5/24/23 17:37, Tom Lane wrote:
> Tomas Vondra writes:
>> While looking for other places allocating stuff in ExecutorState (for
>> the UPDATE case) and leaving it there, I found two more cases:
>
>> 1) copy_plpgsql_datums
>
>> 2) make_expanded_record_from_tupdesc
>>make_expanded_record_
Tomas Vondra writes:
> On 5/23/23 23:39, Tom Lane wrote:
>> FWIW, I've had some success localizing palloc memory leaks with valgrind's
>> leak detection mode. The trick is to ask it for a report before the
>> context gets destroyed. Beats writing your own infrastructure ...
> I haven't tried va
Tomas Vondra writes:
> While looking for other places allocating stuff in ExecutorState (for
> the UPDATE case) and leaving it there, I found two more cases:
> 1) copy_plpgsql_datums
> 2) make_expanded_record_from_tupdesc
>make_expanded_record_from_exprecord
> All of this is calls from plpg
On 5/23/23 22:57, Tomas Vondra wrote:
>
>
> On 5/23/23 18:39, Tom Lane wrote:
>> Tomas Vondra writes:
>>> it seems there's a fairly annoying memory leak in trigger code,
>>> introduced by
>>> ...
>>> Attached is a patch, restoring the pre-12 behavior for me.
>>
>>> While looking for other plac
On 5/24/23 10:19, Jakub Wartak wrote:
> Hi, just two cents:
>
> On Tue, May 23, 2023 at 8:01 PM Andres Freund wrote:
>>
>> Hi,
>>
>> On 2023-05-23 13:28:30 -0400, Tom Lane wrote:
>>> Andres Freund writes:
Could it help to have a mode where the executor shutdown hook checks how
much
On 5/23/23 23:39, Tom Lane wrote:
> Tomas Vondra writes:
>> 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 e
Hi, just two cents:
On Tue, May 23, 2023 at 8:01 PM Andres Freund wrote:
>
> Hi,
>
> On 2023-05-23 13:28:30 -0400, Tom Lane wrote:
> > Andres Freund writes:
> > > Could it help to have a mode where the executor shutdown hook checks how
> > > much
> > > memory is allocated in ExecutorState and w
Tomas Vondra writes:
> 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
On 5/23/23 19:14, Andres Freund wrote:
> Hi,
>
> On 2023-05-23 18:23:00 +0200, Tomas Vondra wrote:
>> This means that for an UPDATE with triggers, we may end up calling this
>> for each row, possibly multiple bitmaps. And those bitmaps are allocated
>> in ExecutorState, so won't be freed until
On 5/23/23 18:39, Tom Lane wrote:
> Tomas Vondra writes:
>> it seems there's a fairly annoying memory leak in trigger code,
>> introduced by
>> ...
>> Attached is a patch, restoring the pre-12 behavior for me.
>
>> While looking for other places allocating stuff in ExecutorState (for
>> the UP
Andres Freund writes:
> On 2023-05-23 13:28:30 -0400, Tom Lane wrote:
>> Why? Unlike Lists, those things are already a single palloc chunk.
> We do a fair amount of 8 byte allocations - they have quite a bit of overhead,
> even after c6e0fe1f2a0. Not needing allocations for the common case of
>
Hi,
On 2023-05-23 13:28:30 -0400, Tom Lane wrote:
> Andres Freund writes:
> > Could it help to have a mode where the executor shutdown hook checks how
> > much
> > memory is allocated in ExecutorState and warns if its too much?
>
> It'd be very hard to set a limit for what's "too much", since th
Andres Freund writes:
> I've wondered about some form of instrumentation to detect such issues
> before.
Yeah.
> Could it help to have a mode where the executor shutdown hook checks how much
> memory is allocated in ExecutorState and warns if its too much?
It'd be very hard to set a limit for w
Hi,
On 2023-05-23 18:23:00 +0200, Tomas Vondra wrote:
> This means that for an UPDATE with triggers, we may end up calling this
> for each row, possibly multiple bitmaps. And those bitmaps are allocated
> in ExecutorState, so won't be freed until the end of the query :-(
Ugh.
I've wondered abou
Tomas Vondra writes:
> it seems there's a fairly annoying memory leak in trigger code,
> introduced by
> ...
> Attached is a patch, restoring the pre-12 behavior for me.
> While looking for other places allocating stuff in ExecutorState (for
> the UPDATE case) and leaving it there, I found two mo
31 matches
Mail list logo
