On Wed, Jan 15, 2025 at 4:43 PM Peter Smith <smithpb2...@gmail.com> wrote: > > On Wed, Jan 15, 2025 at 5:49 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Wed, Jan 15, 2025 at 3:11 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > It seems we agreed on RBTXN_IS_PREPARED and rbtxn_is_prepared(). > > > Adding 'IS' seems to clarify the transaction having this flag *is* a > > > prepared transaction. Both other two constants RBTXN_SENT_PREAPRE and > > > RBTXN_SKIPPED_PREPARE seem not bad to me. > > > > > > > Agreed. > > > > > I find that the proposed > > > names don't increase the consistency much. Thoughts? > > > > > > > I also think so. > > > > 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. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
