Date: Thu, 11 Jul 2019 23:08:13 -0700
From: Shawn Debnath <sdn@amazon.com>
To: Thomas Munro <thomas.munro@gmail.com>
CC: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, pgsql-hackers
	<pgsql-hackers@postgresql.org>
Subject: Re: Introduce timeout capability for ConditionVariableSleep
Message-ID: <20190712060813.GA99189@f01898859afd.ant.amazon.com>
References: <20190313005342.GA8301@f01898859afd.ant.amazon.com>
 <20190313.172415.156658364.horiguchi.kyotaro@lab.ntt.co.jp>
 <20190315002611.GA1119@f01898859afd.ant.amazon.com>
 <20190315.141517.229371570.horiguchi.kyotaro@lab.ntt.co.jp>
 <20190316222717.GA2139@f01898859afd.ant.amazon.com>
 <20190321182049.GA95415@f01898859afd.ant.amazon.com>
 <CA+hUKGKFdqVdSeKnsgsHTQLppRm3+uTOiEOfy+ZgBwshJAZZVg@mail.gmail.com>
 <CA+hUKGLQ_RW+Xs8znDn36e-+mq2--zrPemBqTQ8eKT-VO1OF4Q@mail.gmail.com>
 <CA+hUKGKWS7w4pUFjBPnkU964yBoSK-L-zsHQ2J_-8V0cT-UMgw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: 
 <CA+hUKGKWS7w4pUFjBPnkU964yBoSK-L-zsHQ2J_-8V0cT-UMgw@mail.gmail.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Precedence: Bulk
Precedence: bulk

On Tue, Jul 09, 2019 at 11:03:18PM +1200, Thomas Munro wrote:
> On Sun, Jul 7, 2019 at 3:09 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> > +               /* Timed out */
> > +               if (rc == 0)
> > +                       return true;
> 
> Here's a version without that bit, because I don't think we need it.

This works. Agree that letting it fall through covers the first gap.

> > That still leaves the danger that the CV can be signalled some time
> > after ConditionVariableTimedSleep() returns.  So now I'm wondering if
> > ConditionVariableCancelSleep() should signal the CV if it discovers
> > that this process is not in the proclist, on the basis that that must
> > indicate that we've been signalled even though we're not interested in
> > the message anymore, and yet some other process else might be
> > interested, and that might have been the only signal that is ever
> > going to be delivered by ConditionVariableSignal().
> 
> And a separate patch to do that.  Thoughts?

I like it. This covers the gap all the way till cancel is invoked and it 
manipulates the list to remove itself or realizes that it needs to 
forward the signal to some other process.

Thanks Thomas!

-- 
Shawn Debnath
Amazon Web Services (AWS)