Re: Skip collecting decoded changes of already-aborted transactions

On Sun, Jan 19, 2025 at 7:53 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Jan 17, 2025 at 11:19 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > On Wed, Jan 15, 2025 at 4:43 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > > My thoughts are that any consistency improvement is a step in the
> > > right direction so even "don't increase the consistency much" is still
> > > better than nothing.
> >
> > I agree that doing something is better than nothing. The proposed
> > idea, having RBTXN_IS_PREPARED prefix for all related flags, improves
> > the consistency in terms of names, but I'm not sure this is the right
> > direction. For example, RBTXN_IS_PREPARED_SKIPPED is quite confusing
> > to me. I think this name implies "this is a prepared transaction but
> > is skipped", but I don't think it conveys the meaning well. In
> > addition to that, if we add RBTXN_IS_PREPARED flag also for skipped
> > prepared transactions, we would end up with doing like:
> >
> > txn->txn_flags |= (RBTXN_IS_PREPARED | RBTXN_IS_PREPARED_SKIPPED);
> >
> > Which seems quite redundant. It makes more sense to me to do like:
> >
> > txn->txn_flags |= (RBTXN_IS_PREPARED | RBTXN_SKIPPED_PREPARE);
> >
> > I'd like to avoid a situation like where we rename these names just
> > for better consistency in terms of names and later rename them to
> > better names for other reasons again and again.
> >
>
> Sounds reasonable. We agree with just changing RBTXN_PREPARE to
> RBTXN_IS_PREPARED and its corresponding macro. The next step is to
> update the patch to reflect the same.

Right. I've attached the updated patches.

Regards,

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