On Tue, Aug 6, 2024 at 1:12 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Sat, Aug 3, 2024 at 1:21 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > I found a bug in the memory counter update in reorderbuffer. It was > > introduced by commit 5bec1d6bc5e, so pg17 and master are affected. > > > > In ReorderBufferCleanupTXN() we zero the transaction size and then > > free the transaction entry as follows: > > > > /* Update the memory counter */ > > ReorderBufferChangeMemoryUpdate(rb, NULL, txn, false, txn->size); > > > > /* deallocate */ > > ReorderBufferReturnTXN(rb, txn); > > > > Why do we need to zero the transaction size explicitly? Shouldn't it > automatically become zero after freeing all the changes?
It will become zero after freeing all the changes. However, since updating the max-heap when freeing each change could bring some overhead, we freed the changes without updating the memory counter, and then zerod it. > > > However, if the transaction entry has toast changes we free them in > > ReorderBufferToastReset() called from ReorderBufferToastReset(), > > > > Here, you mean ReorderBufferToastReset() called from > ReorderBufferReturnTXN(), right? Right. Thank you for pointing it out. > 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. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
