On Thu, Jun 5, 2025 at 3:28 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, 5 Jun 2025 at 03:19, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > Thank you for updating the patch. I have some comments and questions:
> >
> > In ReorderBufferAbort():
> >
> > /*
> > * We might have decoded changes for this transaction that could load
> > * the cache as per the current transaction's view (consider DDL's
> > * happened in this transaction). We don't want the decoding of future
> > * transactions to use those cache entries so execute invalidations.
> > */
> > if (txn->ninvalidations > 0)
> > ReorderBufferImmediateInvalidation(rb, txn->ninvalidations,
> > txn->invalidations);
> >
> > I think that if the txn->invalidations_distributed is overflowed, we
> > would miss executing the txn->invalidations here. Probably the same is
> > true for ReorderBufferForget() and ReorderBufferInvalidate().
>
> I'm accumulating the invalidations in txn->invalidations irrespective
> of RBTXN_INVAL_OVERFLOWED txn.
Agreed with this change.
>
> > ---
> > I'd like to make it clear again which case we need to execute
> > txn->invalidations as well as txn->invalidations_distributed (like in
> > ReorderBufferProcessTXN()) and which case we need to execute only
> > txn->invalidations (like in ReorderBufferForget() and
> > ReorderBufferAbort()). I think it might be worth putting some comments
> > about overall strategy somewhere.
>
> I have added comments for this, feel free to reword it if some
> changes are required.
>
> The attached v11 version patch has the changes for the same.
I think the patch is getting into good shape. I've attached a patch
that includes changes I recommend. For example, it's better to rename
RBTXN_INVAL_OVERFLOWED to RBTXN_DISTR_INVAL_OVERFLOWED, and it
includes some comment updates. Please review them.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
