Re: [HACKERS] some review comments on logical rep code
От | Kyotaro HORIGUCHI |
---|---|
Тема | Re: [HACKERS] some review comments on logical rep code |
Дата | |
Msg-id | 20170424.195758.90320244.horiguchi.kyotaro@lab.ntt.co.jp обсуждение исходный текст |
Ответ на | Re: [HACKERS] some review comments on logical rep code (Masahiko Sawada <sawada.mshk@gmail.com>) |
Ответы |
Re: [HACKERS] some review comments on logical rep code
(Masahiko Sawada <sawada.mshk@gmail.com>)
Re: [HACKERS] some review comments on logical rep code (Fujii Masao <masao.fujii@gmail.com>) |
Список | pgsql-hackers |
Hello, At Mon, 24 Apr 2017 11:18:32 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in <CAD21AoBU8mZdGx-sTJ3u+qkaej5RPnOuotW4kfeXc4XdDNF8cw@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' (this is centainly a kind of false wakeups, though). We can live with this failure when using two-paase commmit, but I think it shouldn't happen silently. How about providing AtPrepare_ApplyLauncher(void) like the follows and calling it in PrepareTransaction? void AtPrepare_ApplyLauncher(void) { if (on_commit_launcher_wakeup) ereport(WARNING, (errmsg ("logical replication may not start immediately"), errhint("PREPARE TRANSACTION can hinder immediate lanching of logical replication worker."))); } regards, -- Kyotaro Horiguchi NTT Open Source Software Center
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Robert HaasДата:
Сообщение: Re: [HACKERS] Adding support for Default partition in partitioning
Следующее
От: Konstantin KnizhnikДата:
Сообщение: Re: [HACKERS] Cached plans and statement generalization