On Fri, May 28, 2021 at 6:01 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > > On 5/28/21 2:17 PM, Dilip Kumar wrote: > > On Fri, May 28, 2021 at 5:16 PM Tomas Vondra > > <tomas.von...@enterprisedb.com> wrote: > >> On 5/27/21 6:36 AM, Dilip Kumar wrote: > >>> On Thu, May 27, 2021 at 9:47 AM Amit Kapila <amit.kapil...@gmail.com> > >>> wrote: > >>>> > >>>> On Thu, May 27, 2021 at 9:40 AM Dilip Kumar <dilipbal...@gmail.com> > >>>> wrote: > >>>> > >>>> True, but if you do this clean-up in ReorderBufferCleanupTXN then you > >>>> don't need to take care at separate places. Also, toast_hash is stored > >>>> in txn so it appears natural to clean it up in while releasing TXN. > >>> > >>> Make sense, basically, IMHO we will have to do in TruncateTXN and > >>> ReturnTXN as attached? > >>> > >> > >> Yeah, I've been working on a fix over the last couple days (because of a > >> customer issue), and I ended up with the reset in ReorderBufferReturnTXN > >> too - it does solve the issue in the case I've been investigating. > >> > >> I'm not sure the reset in ReorderBufferTruncateTXN is correct, though. > >> Isn't it possible that we'll need the TOAST data after streaming part of > >> the transaction? After all, we're not resetting invalidations, tuplecids > >> and snapshot either > > > > Actually, as per the current design, we don't need the toast data > > after the streaming. Because currently, we don't allow to stream the > > transaction if we need to keep the toast across stream e.g. if we only > > have toast insert without the main insert we assure this as partial > > changes, another case is if we have multi-insert with toast then we > > keep the transaction as mark partial until we get the last insert of > > the multi-insert. So with the current design we don't have any case > > where we need to keep toast data across streams. > > > > ... And we'll eventually clean it after the streamed > >> transaction gets commited (ReorderBufferStreamCommit ends up calling > >> ReorderBufferReturnTXN too). > > > > Right, but generally after streaming we assert that txn->size should > > be 0. That could be changed if we change the above design but this is > > what it is today. > > > > Can we add some assert to enforce this? >
There is already an Assert for this. See ReorderBufferCheckMemoryLimit. -- With Regards, Amit Kapila.