On Wed, May 10, 2017 at 9:48 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > 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.
Well, it's probably worth changing for consistency, but I'm not sure that it rises to the level of "a very bad idea". It actually seems almost entirely harmless. Spuriously setting the needreload flag on a just-deceased WAL sender will just result in some future WAL sender doing a bit of unnecessary work, but I don't think it breaks anything and the probability is vanishingly low. The other change could result a bogus 0 PID in pg_stat_get_wal_senders output, but I bet you couldn't reproduce that more than once in a blue moon even with a test rig designed to provoke it, and if it does happen it isn't really anything more than a trivial annoyance. So I'm in favor of committing this and maybe even back-patching it, but I also don't think it's a big deal. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers