From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Fix memory counter update in reorderbuffer |
Date: | 2024-08-11 22:22:53 |
Message-ID: | CAD21AoDHC4Sob=NEYTxgu5wd4rzCpSLY_hWapMUqf4WKrAxmyw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Aug 10, 2024 at 5:48 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Fri, Aug 9, 2024 at 3:30 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Aug 8, 2024 at 9:43 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Aug 7, 2024 at 3:17 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > >
> > > > On Wed, Aug 7, 2024 at 7:42 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > >
> > > > >
> > > > > > BTW, commit 5bec1d6bc5e also introduced
> > > > > > ReorderBufferChangeMemoryUpdate() in ReorderBufferTruncateTXN() which
> > > > > > is also worth considering while fixing the reported problem. It may
> > > > > > not have the same problem as you have reported but we can consider
> > > > > > whether setting txn size as zero explicitly is required or not.
> > > > >
> > > > > The reason why it introduced ReorderBufferChangeMemoryUpdate() is the
> > > > > same as I mentioned above. And yes, as you mentioned, it doesn't have
> > > > > the same problem that I reported here.
> > > > >
> > > >
> > > > I checked again and found that ReorderBufferResetTXN() first calls
> > > > ReorderBufferTruncateTXN() and then ReorderBufferToastReset(). After
> > > > that, it also tries to free spec_insert change which should have the
> > > > same problem. So, what saves this path from the same problem?
> > >
> > > Good catch. I've not created a test case for that but it seems to be
> > > possible to happen.
> > >
> > > I think that subtracting txn->size to reduce the memory counter to
> > > zero seems to be a wrong idea in the first place. If we want to save
> > > updating memory counter and max-heap, we should use the exact memory
> > > size that we freed. In other words, just change the memory usage
> > > update to a batch operation.
> > >
> >
> > Sounds reasonable but how would you find the size for a batch update
> > operation? Are you planning to track it while freeing the individual
> > changes?
>
> Yes, one idea is to make ReorderBufferReturnChange() return the memory
> size that it just freed. Then the caller who wants to update the
> memory counter in a batch sums up the memory size.
I've drafted the patch. I didn't change ReorderBufferReturnChange()
but called ReorderBufferChangeSize() for individual change instead, as
it's simpler.gi
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachment | Content-Type | Size |
---|---|---|
fix_memory_counter_update_in_reorderbuffer_v2.patch | application/octet-stream | 4.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2024-08-12 00:32:05 | Re: Recent 027_streaming_regress.pl hangs |
Previous Message | Tom Lane | 2024-08-11 21:54:25 | Re: Returning from a rule with extended query protocol |