Horton

Thread

Commits

  1. Fix race condition with unprotected use of a latch pointer variable.

  2. Fix coding rules violations in walreceiver.c

  3. Use latch instead of select() in walreceiver

  1. [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Andreas Seltenreich <seltenreich@gmx.de> — 2017-10-02T20:56:49Z

    Hi,

low-memory testing with REL_10_STABLE at 1f19550a87 produced the
following PANIC:

    stuck spinlock detected at pg_stat_get_wal_receiver, walreceiver.c:1397

I was about to wrap the pstrdup()s with a PG_TRY block, but I can't find
a spinlock being released in a PG_CATCH block anywhere, so maybe that's
a bad idea?

regards,
Andreas

  2. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Thomas Munro <thomas.munro@enterprisedb.com> — 2017-10-02T21:05:16Z

    On Tue, Oct 3, 2017 at 9:56 AM, Andreas Seltenreich <seltenreich@gmx.de> wrote:
> low-memory testing with REL_10_STABLE at 1f19550a87 produced the
> following PANIC:
>
>     stuck spinlock detected at pg_stat_get_wal_receiver, walreceiver.c:1397
>
> I was about to wrap the pstrdup()s with a PG_TRY block, but I can't find
> a spinlock being released in a PG_CATCH block anywhere, so maybe that's
> a bad idea?

No comment on what might be holding the spinlock there, but perhaps
the spinlock-protected code should strncpy into stack-local buffers
instead of calling pstrdup()?  The buffers could be statically sized
with NAMEDATALEN and MAXCONNINFO.

-- 
Thomas Munro
http://www.enterprisedb.com

  3. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Andres Freund <andres@anarazel.de> — 2017-10-02T21:08:05Z

    On 2017-10-02 22:56:49 +0200, Andreas Seltenreich wrote:
> Hi,
> 
> low-memory testing with REL_10_STABLE at 1f19550a87 produced the
> following PANIC:
> 
>     stuck spinlock detected at pg_stat_get_wal_receiver, walreceiver.c:1397

Ugh.

> I was about to wrap the pstrdup()s with a PG_TRY block, but I can't find
> a spinlock being released in a PG_CATCH block anywhere, so maybe that's
> a bad idea?

Yes, that'd be a bad idea. It's not great to have memcpys in a critical
section, but it's way better than pallocs. So we need to use some local
buffers that this get copied to.

This seems to have been introduced as part of b1a9bad9e74 and then
9ed551e0a4f.  Authors CCed.

Greetings,

Andres Freund

  4. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-02T21:30:25Z

    Andres Freund <andres@anarazel.de> writes:
> On 2017-10-02 22:56:49 +0200, Andreas Seltenreich wrote:
>> low-memory testing with REL_10_STABLE at 1f19550a87 produced the
>> following PANIC:
>> stuck spinlock detected at pg_stat_get_wal_receiver, walreceiver.c:1397

> Ugh.

Egad.

> Yes, that'd be a bad idea. It's not great to have memcpys in a critical
> section, but it's way better than pallocs. So we need to use some local
> buffers that this get copied to.

Or replace the spinlock with an LWLock?  In any case, I think it would be
a good idea to look at every other critical section touching that lock
to see if there are any other blatant coding-rule violations.

			regards, tom lane

  5. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Andres Freund <andres@anarazel.de> — 2017-10-02T21:32:15Z

    On 2017-10-02 17:30:25 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Yes, that'd be a bad idea. It's not great to have memcpys in a critical
> > section, but it's way better than pallocs. So we need to use some local
> > buffers that this get copied to.
> 
> Or replace the spinlock with an LWLock?

That'd probably be a good idea, but I'm loathe to do so in the back
branches. Not at this callsite, but some others, there's some potential
for contention.

Greetings,

Andres Freund

  6. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-02T21:34:42Z

    Andres Freund <andres@anarazel.de> writes:
> On 2017-10-02 17:30:25 -0400, Tom Lane wrote:
>> Or replace the spinlock with an LWLock?

> That'd probably be a good idea, but I'm loathe to do so in the back
> branches. Not at this callsite, but some others, there's some potential
> for contention.

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.

			regards, tom lane

  7. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-02T21:54:20Z

    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

  8. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Michael Paquier <michael.paquier@gmail.com> — 2017-10-03T00:54:36Z

    On Tue, Oct 3, 2017 at 6:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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 don't think that there is any need to switch to a LWLock. Any issues
in need to be dealt with here don't require it, if we are fine with
the memcpy method of course.

> 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.

No problems seen either from here.

> 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.

This part has been around since the beginning in 1bb2558. I agree that
the lock should be released before doing the logging.

> Further down, it's doing a pfree() inside the spinlock, apparently
> for no other reason than to save one "if (tmp_conninfo)".

Check.

> 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.

Those could be replaced by similar checks using some
PG_USED_FOR_ASSERTS_ONLY out of the spin lock sections, though I am
not sure if those are worth worrying. What do others think?

> 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?

Do you mean the introduction of WalRcvForceReply by 314cbfc? This is
more recent, and has been discussed during the review of the
remote_apply patch here to avoid sending SIGUSR1 too much from the
startup process to the WAL receiver:
https://www.postgresql.org/message-id/CA+TgmobPsROS-gFk=_KJdW5scQjcKtpiLhsH9Cw=UWH1htFKaw@mail.gmail.com

I am attaching a patch that addresses the bugs for the spin lock sections.
-- 
Michael

  9. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Andreas Seltenreich <seltenreich@gmx.de> — 2017-10-03T11:04:04Z

    Michael Paquier writes:

> I am attaching a patch that addresses the bugs for the spin lock sections.
> [2. text/x-diff; walreceiver-spin-calls.patch]

I haven't seen a spinlock PANIC since testing with the patch applied.
They occured five times with the same amount of testing done earlier.

regards,
Andreas

  10. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-10-03T11:24:16Z

    Andreas Seltenreich wrote:
> Michael Paquier writes:
> 
> > I am attaching a patch that addresses the bugs for the spin lock sections.
> > [2. text/x-diff; walreceiver-spin-calls.patch]
> 
> I haven't seen a spinlock PANIC since testing with the patch applied.
> They occured five times with the same amount of testing done earlier.

Thanks for testing.  I'm about to push something.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-10-03T14:07:14Z

    Fixed the pstrdup problem by replacing with strlcpy() to stack-allocated
variables (rather than palloc + memcpy as proposed in Michael's patch).

About the other problems:

Tom Lane wrote:

> 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.

Done that way.

> 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.

Fixed by releasing spinlock just before elog.

> Further down, it's doing a pfree() inside the spinlock, apparently
> for no other reason than to save one "if (tmp_conninfo)".

Fixed.

> 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 didn't change these.  It doesn't look to me that these asserts are
worth very much as production code.

> 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?

I think the latch is only used locally.  Seems that it was only put in
shmem to avoid a separate variable ...

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  12. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-03T14:31:42Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Fixed the pstrdup problem by replacing with strlcpy() to stack-allocated
> variables (rather than palloc + memcpy as proposed in Michael's patch).

+1

> Tom Lane wrote:
>> I don't especially like the Asserts inside spinlocks, either.

> I didn't change these.  It doesn't look to me that these asserts are
> worth very much as production code.

OK.  If we ever see these hit in the buildfarm I might argue for
reconsidering, but without some evidence of that sort it's not
worth much concern.

>> 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?

> I think the latch is only used locally.  Seems that it was only put in
> shmem to avoid a separate variable ...

Hm, I'm strongly tempted to move it to a separate static variable then.
That's not a bug fix, so maybe it only belongs in HEAD, but is there
value in keeping the branches in sync in this code?  It sounded from
your commit message like they were pretty different already :-(

			regards, tom lane

  13. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2017-10-03T14:50:23Z

    Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

> > Tom Lane wrote:
> >> I don't especially like the Asserts inside spinlocks, either.
> 
> > I didn't change these.  It doesn't look to me that these asserts are
> > worth very much as production code.
> 
> OK.  If we ever see these hit in the buildfarm I might argue for
> reconsidering, but without some evidence of that sort it's not
> worth much concern.

Sure.  I would be very surprised if buildfarm ever exercises this code.

> > I think the latch is only used locally.  Seems that it was only put in
> > shmem to avoid a separate variable ...
> 
> Hm, I'm strongly tempted to move it to a separate static variable then.
> That's not a bug fix, so maybe it only belongs in HEAD, but is there
> value in keeping the branches in sync in this code?  It sounded from
> your commit message like they were pretty different already :-(

Well, there were conflicts in almost every branch, but they were pretty
minor.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  14. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-03T16:30:02Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Tom Lane wrote:
>> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>>> I think the latch is only used locally.  Seems that it was only put in
>>> shmem to avoid a separate variable ...

>> Hm, I'm strongly tempted to move it to a separate static variable then.

Oh, wait, look at

/*
 * Wake up the walreceiver main loop.
 *
 * This is called by the startup process whenever interesting xlog records
 * are applied, so that walreceiver can check if it needs to send an apply
 * notification back to the master which may be waiting in a COMMIT with
 * synchronous_commit = remote_apply.
 */
void
WalRcvForceReply(void)
{
	WalRcv->force_reply = true;
	if (WalRcv->latch)
		SetLatch(WalRcv->latch);
}

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.

I'm inclined to think that it'd be a good idea to move the set and
clear of the latch field into the nearby spinlock critical sections,
and then change WalRcvForceReply to look like

void
WalRcvForceReply(void)
{
	Latch	   *latch;

	WalRcv->force_reply = true;
	/* fetching the latch pointer might not be atomic, so use spinlock */
	SpinLockAcquire(&WalRcv->mutex);
	latch = WalRcv->latch;
	SpinLockRelease(&WalRcv->mutex);
	if (latch)
		SetLatch(latch);
}

This still leaves us with a hazard of applying SetLatch to the latch
of a PGPROC that is no longer the walreceiver's, but I think that's
okay (and we have the same problem elsewhere).  At worst it leads to
an extra wakeup of the next process using that PGPROC.

I'm also thinking that the force_reply flag needs to be sig_atomic_t,
not bool, because bool store/fetch isn't necessarily atomic on all
platforms.

			regards, tom lane

  15. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-03T16:58:23Z

    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.
>
> I'm inclined to think that it'd be a good idea to move the set and
> clear of the latch field into the nearby spinlock critical sections,
> and then change WalRcvForceReply to look like ...

Concretely, as per the attached.

I reordered the WalRcvData fields to show that the "latch" field is now
treated as protected by the spinlock.  In the back branches, we shouldn't
do that, just in case some external code is touching the mutex field.

			regards, tom lane

  16. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-10-03T17:44:12Z

    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.

			regards, tom lane

  17. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2017-10-03T18:32:22Z

    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

  18. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Thomas Munro <thomas.munro@enterprisedb.com> — 2017-10-03T20:01:38Z

    On Wed, Oct 4, 2017 at 7:32 AM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
> 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).

I wonder if the new ConditionVariable mechanism would be worth
considering in future cases like this, where the signalling process
doesn't know the identity of the process to be woken up (or even how
many waiting processes there are), but instead any interested waiters
block on a particular ConditionVariable that models a specific
interesting condition.  In the end it's just latches anyway, but it
may be a better abstraction.  On the other hand I'm not sure how waits
on a ConditionVariable would be multiplexed with IO (a distinct wait
event, or leaky abstraction where the caller relies on the fact that
it's built on MyProc->latch, something else?).

-- 
Thomas Munro
http://www.enterprisedb.com

  19. Re: [sqlsmith] stuck spinlock in pg_stat_get_wal_receiver after OOM

    Petr Jelinek <petr.jelinek@2ndquadrant.com> — 2017-10-03T21:40:28Z

    On 03/10/17 22:01, Thomas Munro wrote:
> On Wed, Oct 4, 2017 at 7:32 AM, Petr Jelinek
> <petr.jelinek@2ndquadrant.com> wrote:
>> 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).
> 
> I wonder if the new ConditionVariable mechanism would be worth
> considering in future cases like this, where the signalling process
> doesn't know the identity of the process to be woken up (or even how
> many waiting processes there are), but instead any interested waiters
> block on a particular ConditionVariable that models a specific
> interesting condition.  In the end it's just latches anyway, but it
> may be a better abstraction.  On the other hand I'm not sure how waits
> on a ConditionVariable would be multiplexed with IO (a distinct wait
> event, or leaky abstraction where the caller relies on the fact that
> it's built on MyProc->latch, something else?).
> 

In this specific case the problem is that what we are waiting for is
indeed the Latch because we want the libpqwalreceiver calls to be
interruptible when waiting for I/O.

Otherwise, yes ConditionVariable is useful for generic signaling, we use
it in slot and origin for "lock" waits (they don't have normal catalog
so normal locking does not work).

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