Re: Skip collecting decoded changes of already-aborted transactions

On Tue, Dec 10, 2024 at 10:59 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, Nov 26, 2024 at 3:02 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> >
> > I've attached a new version patch that incorporates all comments I got so far.
> >
> > I think the patch is in good shape but I'm considering whether we
> > might want to call ReorderBufferToastReset() after truncating all
> > changes, in ReorderBufferTruncateTXNIfAborted() just in case. Will
> > investigate further.
> >
>
> There’s something that seems a bit odd to me. Consider the case where
> the largest transaction(s) are aborted. If
> ReorderBufferCanStartStreaming() returns true, the changes from this
> transaction will only be discarded if it's a streamable transaction.
> However, if ReorderBufferCanStartStreaming() is false, the changes
> will be discarded regardless.
>
> What seems strange to me in this patch is truncating the changes of a
> large aborted transaction depending on whether we need to stream or
> spill but actually that should be completely independent IMHO. My
> concern is that if the largest transaction is aborted but isn’t yet
> streamable, we might end up picking the next transaction, which could
> be much smaller. This smaller transaction might not help us stay
> within the memory limit, and we could repeat this process for a few
> more transactions. In contrast, it might be more efficient to simply
> discard the large aborted transaction, even if it’s not streamable, to
> avoid this issue.
>

If the largest transaction is non-streamable, won't the transaction
returned by ReorderBufferLargestTXN() in the other case already
suffice the need?

-- 
With Regards,
Amit Kapila.