On Wed, Apr 26, 2017 at 3:47 PM, Kyotaro HORIGUCHI <horiguchi.kyot...@lab.ntt.co.jp> wrote: > At Wed, 26 Apr 2017 14:31:12 +0900, Masahiko Sawada <sawada.m...@gmail.com> > wrote in > <CAD21AoDMy8a6UwMrRh8pigQbDC+JAOQ4m_tXT41VRP4SYp23=w...@mail.gmail.com> >> On Wed, Apr 26, 2017 at 12:35 PM, Petr Jelinek >> <petr.jeli...@2ndquadrant.com> wrote: >> > On 26/04/17 01:01, Fujii Masao wrote: >> >>>> 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? >> >> >> >> Or we should apply the attached patch and handle the 2PC case properly? >> >> I was thinking that it's overkill more than necessary, but that seems not >> >> true >> >> as far as I implement that. >> >> >> > Looks like it does not even increase size of the 2pc file, +1 for this. >> >> In my honest opinion, I didn't have a big will that we should handle >> even two-phase commit case, because this case is very rare (I could >> not image such case) and doesn't mean to lead a harmful result such as >> crash of server and returning inconsistent result. it just delays the >> launching worker for at most 3 minutes. We also can deal with this for >> example by making maximum nap time of apply launcher user-configurable >> and document it. >> But if we can deal with it by minimum changes like attached your patch I >> agree. > > This change looks reasonable to me, +1 from me to this. > > The patch reads on_commit_launcher_wakeup directly then updates > it via ApplyALuncherWakeupAtCommit() but it's too much to add a > function for the sake of this.
OK, so what about the attached patch? I replaced all the calls to ApplyLauncherWakeupAtCommit() with the code "on_commit_launcher_wakeup = true". Regards, -- Fujii Masao
002_Reset_on_commit_launcher_wakeup_v6.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
