From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Andreas Seltenreich <seltenreich@gmx.de>
Cc: pgsql-hackers@postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael.paquier@gmail.com>
Subject: Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM
Date: Mon, 02 Oct 2017 17:54:20 -0400
Message-ID: <1409.1506981260@sss.pgh.pa.us> (raw)
In-Reply-To: <665.1506980082@sss.pgh.pa.us>
References: <87h8vhtgj2.fsf@ansel.ydns.eu>
<20171002210805.x4lxxmarluen6rfg@alap3.anarazel.de>
<459.1506979825@sss.pgh.pa.us>
<20171002213215.4e3iwt7oh5wdjqm2@alap3.anarazel.de>
<665.1506980082@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
I wrote:
> If this is the only problem then I'd agree we should stick to a spinlock
> (I assume the strings in question can't be very long). I was thinking
> more about what to do if we find other violations that are harder to fix.
I took a quick look through walreceiver.c, and there are a couple of
obvious problems of the same ilk in WalReceiverMain, which is doing this:
walrcv->lastMsgSendTime = walrcv->lastMsgReceiptTime = walrcv->latestWalEndTime = GetCurrentTimestamp();
(ie, a potential kernel call) inside a spinlock. But there seems no
real problem with just collecting the timestamp before we enter that
critical section.
I also don't especially like the fact that just above there it reaches
elog(PANIC) with the lock still held, though at least that's a case that
should never happen.
Further down, it's doing a pfree() inside the spinlock, apparently
for no other reason than to save one "if (tmp_conninfo)".
I don't especially like the Asserts inside spinlocks, either. Personally,
I think if those conditions are worth testing then they're worth testing
for real (in production). Variables that are manipulated by multiple
processes are way more likely to assume unexpected states than local
variables.
I'm also rather befuddled by the fact that this code sets and clears
walrcv->latch outside the critical sections. If that field is used
by any other process, surely that's completely unsafe. If it isn't,
why is it being kept in shared memory?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, seltenreich@gmx.de, alvherre@2ndquadrant.com, michael.paquier@gmail.com
Subject: Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM
In-Reply-To: <1409.1506981260@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox