Re: wake up logical workers after ALTER SUBSCRIPTION

On Tue, Jan 3, 2023 at 11:51 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Tue, Jan 03, 2023 at 11:43:59AM +0530, Amit Kapila wrote:
> > On Wed, Dec 7, 2022 at 11:42 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> >> After sleeping on this, I think we can do better.  IIUC we can simply check
> >> for AllTablesyncsReady() at the end of process_syncing_tables_for_apply()
> >> and wake up the logical replication workers (which should just consiѕt of
> >> setting the current process's latch) if we are ready for two_phase mode.
> >
> > How just waking up will help with two_phase mode? For that, we need to
> > restart the apply worker as we are doing at the beginning of
> > process_syncing_tables_for_apply().
>
> Right.  IIRC waking up causes the apply worker to immediately call
> process_syncing_tables_for_apply() again, which will then proc_exit(0) as
> appropriate.
>

But we are already in apply worker and performing
process_syncing_tables_for_apply(). This means the apply worker is not
waiting/sleeping, so what exactly are we trying to wake up?

>  It might be possible to move the restart logic to the end of
> process_syncing_tables_for_apply() to avoid this extra wakeup.  WDYT?
>

I am not sure if I understand the problem you are trying to solve with
this part of the patch. Are you worried that after we mark some of the
relation's state as READY, all the table syncs are in the READY state
but we will not immediately try to check the two_pahse stuff and
probably the apply worker may sleep before the next time it invokes
process_syncing_tables_for_apply()? If so, we probably also need to
ensure that table_states_valid is marked false probably via
invalidations so that we can get the latest state and then perform
this check. I guess if we can do that then we can directly move the
restart logic to the end.

-- 
With Regards,
Amit Kapila.