Re: [HACKERS] logical decoding of two-phase transactions

On Sat, Feb 27, 2021 at 7:31 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Fri, Feb 26, 2021 at 9:58 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > 6.
> > + * XXX - Is there a potential timing problem here - e.g. if signal arrives
> > + * while executing this then maybe we will set table_states_valid without
> > + * refetching them?
> > + */
> > +static void
> > +FetchTableStates(bool *started_tx)
> > ..
> >
> > Can you explain which race condition you are worried about here which
> > is not possible earlier but can happen after this patch?
> >
>
> Yes, my question (in that XXX comment) was not about anything new for
> the current patch, because this FetchTableStates function has exactly
> the same logic as the HEAD code.
>
> I was only wondering if there is any possibility that one of the
> function calls (inside the if block) can end up calling
> CHECK_INTERRUPTS. If that could happen, then perhaps the
> table_states_valid flag could be assigned false (by the
> invalidate_syncing_table_states signal handler) only to be
> immediately/wrongly overwritten as table_states_valid = true in this
> FetchTableStates code.
>

This is not related to CHECK_FOR_INTERRUPTS. The
invalidate_syncing_table_states() can be called only when we process
invalidation messages which we do while locking the relation via
GetSubscriptionRelationstable_open->relation_open->LockRelationOid.
After that, it won't be done in that part of the code. So, I think we
don't need this comment.

--
With Regards,
Amit Kapila.