Subject: Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver
 after OOM
To: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Andres Freund <andres@anarazel.de>,
 Andreas Seltenreich <seltenreich@gmx.de>, pgsql-hackers@postgresql.org,
 Michael Paquier <michael.paquier@gmail.com>,
 Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
 Petr Jelinek <petr@2ndquadrant.com>
References: <20171003145023.nc2ffduj3qvy4b5b@alvherre.pgsql>
 <22735.1507048202@sss.pgh.pa.us> <6526.1507049903@sss.pgh.pa.us>
 <23454.1507052652@sss.pgh.pa.us>
From: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Message-ID: <4689c67f-5ffb-7359-a3e9-4abf3908e603@2ndquadrant.com>
Date: Tue, 3 Oct 2017 20:32:22 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <23454.1507052652@sss.pgh.pa.us>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 03/10/17 19:44, Tom Lane wrote:
> I wrote:
>>> So that's trouble waiting to happen, for sure.  At the very least we
>>> need to do a single fetch of WalRcv->latch, not two.  I wonder whether
>>> even that is sufficient, though: this coding requires an atomic fetch of
>>> a pointer, which is something we generally don't assume to be safe.
> 
> BTW, I had supposed that this bug was of long standing, but actually it's
> new in v10, dating to 597a87ccc9a6fa8af7f3cf280b1e24e41807d555.  Before
> that walreceiver start/stop just changed the owner of a long-lived shared
> latch, and there was no question of stale pointers.
> 
> I considered reverting that decision, but the reason for it seems to have
> been to let libpqwalreceiver.c manipulate MyProc->procLatch rather than
> having to know about a custom latch.  That's probably a sufficient reason
> to justify some squishiness in the wakeup logic.  Still, we might want to
> revisit it if we find any other problems here.
That's correct, and because the other users of that library don't have
special latch it seemed feasible to standardize on the procLatch. If we
indeed wanted to change the decision about this I think we can simply
give latch as a parameter to libpqrcv_connect and store it in the
WalReceiverConn struct. The connection does not need to live past the
latch (although it currently does, but that's just a matter of
reordering the code in WalRcvDie() a little AFAICS).

-- 
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers