Re: Skip collecting decoded changes of already-aborted transactions

On Sun, Nov 24, 2024 at 8:50 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi, Here are my review comments for patch v9-0001.
>
> These are only trivial nits for some code comments. Everything else
> looked good to me.
>
> ======
> .../replication/logical/reorderbuffer.c
>
> ReorderBufferTruncateTXN:
>
> 1.
> + * The given transaction is marked as streamed if appropriate and the caller
> + * asked it by passing 'mark_txn_streaming' being true.
>
> /asked it/requested it/
>
> /being true/as true/
>
> ~~~
>
> ReorderBufferPrepare:
>
> 2.
> + /*
> + * Remember if the transaction is already aborted to check if we detect
> + * that the transaction is concurrently aborted during the replay.
> + */
>
> SUGGESTION:
> Remember if the transaction is already aborted so we can detect when
> the transaction is concurrently aborted during the replay.
>

Thank you for the suggestions.

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.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com