On Sat, Feb 27, 2021 at 7:31 AM Peter Smith <smithpb2...@gmail.com> wrote: > > On Fri, Feb 26, 2021 at 9:58 PM Amit Kapila <amit.kapil...@gmail.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.
