At Fri, 12 May 2017 11:44:19 +0900, Masahiko Sawada <sawada.m...@gmail.com> wrote in <CAD21AoA15xsu1gbOH=l1xu7g7zukk1uactoz+-mqowp1_xb...@mail.gmail.com> > On Thu, May 11, 2017 at 10:48 AM, Michael Paquier > <michael.paqu...@gmail.com> wrote: > > Hi all, > > > > I had my eyes on the WAL sender code this morning, and I have noticed > > that walsender.c is not completely consistent with the PID lookups it > > does in walsender.c. In two code paths, the PID value is checked > > without holding the WAL sender spin lock (WalSndRqstFileReload and > > pg_stat_get_wal_senders), which looks like a very bad idea contrary to > > what the new WalSndWaitStopping() does and what InitWalSenderSlot() is > > doing for ages. > > > > Any thoughts about the patch attached to make things more consistent? > > It seems to me that having some safeguards would be nice for > > robustness. > > +1. I think this is a sensible change.
It intends to avoid exccesive locking during looking up stats values. But we don't have so much vacant WanSnd slots in a reasonable setup. Thus it seems reasonable to read the pid value within the lock section since it adds practically no additional cost. pg_stat_get_wal_receiver seems to need the same amendment since the code is a parallel to that of wal receiver. Or, if we were too sensitive to such locks for nothing, we could use double-checked locking but I don't think we are so here. In short, +1 too and walreceiver needs the same amendment. 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
