On Thu, Aug 8, 2024 at 9:43 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, Aug 7, 2024 at 3:17 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, Aug 7, 2024 at 7:42 AM Masahiko Sawada <sawada.m...@gmail.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? -- With Regards, Amit Kapila.
