On Wed, Jun 2, 2021 at 11:52 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jun 2, 2021 at 11:38 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Wed, Jun 2, 2021 at 11:25 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > I think the same relation case might not create a problem because it > > > won't find the entry for it in the toast_hash, so it will return from > > > there but the other two problems will be there. > > > > Right > > > > So, one idea could be > > > to just avoid those two cases (by simply adding return for those > > > cases) and still we can rely on toast clean up on the next > > > insert/update/delete. However, your approach looks more natural to me > > > as that will allow us to clean up memory in all cases instead of > > > waiting till the transaction end. So, I still vote for going with your > > > patch's idea of cleaning at spec_abort but I am fine if you and others > > > decide not to process spec_abort message. What do you think? Tomas, do > > > you have any opinion on this matter? > > > > I agree that processing with spec abort looks more natural and ideally > > the current code expects it to be getting cleaned after the change, > > that's the reason we have those assertions and errors. > >
Okay, so, let's go with that approach. I have thought about whether it creates any problem in back-branches but couldn't think of any primarily because we are not going to send anything additional to plugin/subscriber. Do you see any problems with back branches if we go with this approach? > > OTOH I agree > > that we can just return from those conditions because now we know that > > with the current code those situations are possible. My vote is with > > handling the spec abort option (Option1) because that looks more > > natural way of handling these issues and we also don't have to clean > > up the hash in "ReorderBufferReturnTXN" if no followup change after > > spec abort. > > > > Even, if we decide to go with spec_abort approach, it might be better > to still keep the toastreset call in ReorderBufferReturnTXN so that it > can be freed in case of error. > Please take care of this as well. -- With Regards, Amit Kapila.
