At Tue, 25 Apr 2017 10:11:06 +0900, Masahiko Sawada <sawada.m...@gmail.com> wrote in <cad21aobaxrmbbah5-myxxkspatis0xt_-yvvb2y+jg2m-ew...@mail.gmail.com> > On Mon, Apr 24, 2017 at 7:57 PM, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > Hello, > > > > At Mon, 24 Apr 2017 11:18:32 +0900, Masahiko Sawada <sawada.m...@gmail.com> > > wrote in > > <cad21aobu8mzdgx-stj3u+qkaej5rpnouotw4kfexc4xddnf...@mail.gmail.com> > >> >> BEGIN; > >> >> ALTER SUBSCRIPTION hoge_sub ENABLE; > >> >> PREPARE TRANSACTION 'g'; > >> >> BEGIN; > >> >> SELECT 1; > >> >> COMMIT; -- wake up the launcher at this point. > >> >> COMMIT PREPARED 'g'; > >> >> > >> >> Even if we wake up the launcher even when a ROLLBACK PREPARED, it's > >> >> not a big deal to the launcher process actually. Compared with the > >> >> complexly of changing the logic so that on_commit_launcher_wakeup > >> >> corresponds to prepared transaction, we might want to accept this > >> >> behavior. > >> > > >> > on_commit_launcher_wakeup needs to be recoreded in 2PC state > >> > file to handle this issue properly. But I agree with you, that would > >> > be overkill for small gain. So I'm thinking to add the following > >> > comment into your patch and push it. Thought? > >> > > >> > ------------------------- > >> > Note that on_commit_launcher_wakeup flag doesn't work as expected in 2PC > >> > case. > >> > For example, COMMIT PREPARED on the transaction enabling the flag > >> > doesn't wake up > >> > the logical replication launcher if ROLLBACK on another transaction > >> > runs before it. > >> > To handle this case properly, the flag needs to be recorded in 2PC > >> > state file so that > >> > COMMIT PREPARED and ROLLBACK PREPARED can determine whether to wake up > >> > the launcher. However this is overkill for small gain and false wakeup > >> > of the launcher > >> > is not so harmful (probably we can live with that), so we do nothing > >> > here for this issue. > >> > ------------------------- > >> > > >> > >> Agreed. > >> > >> I added this comment to the patch and attached it. > > > > The following "However" may need a follwoing comma. > > > >> However this is overkill for small gain and false wakeup of the > >> launcher is not so harmful (probably we can live with that), so > >> we do nothing here for this issue. > > > > I agree this as a whole. But I think that the main issue here is > > not false wakeups, but 'possible delay of launching new workers > > by 3 minutes at most' > > In what case does launching of the apply worker delay 3 minutes at most?
In the "while (!got_SIGTERM)" loop in ApplyLauncherMain, if we tried to run enabled subscriptions but no subscription is enabled, it waits for 3 minutes. If we turn on a subscription but the trigger is consumed by the false wakeup, the loop sees no enabled subscription and will sleep for 3 minutes unless any other trigger comes. # I haven't tried that, though. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
