On Tue, Jan 3, 2023 at 11:51 PM Nathan Bossart <nathandboss...@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 <nathandboss...@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.
