Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.apache.org
Content-Type: multipart/alternative;
 boundary="===============6500295238303818362=="
MIME-Version: 1.0
Subject: Re: Review Request 41915: Fixup TaskObserverTest to respect thread
 memory models.
From: "John Sirois" <john.sirois@gmail.com>
To: "Bill Farner" <wfarner@apache.org>, "Maxim Khutornenko" <maxim@apache.org>
Cc: "John Sirois" <john.sirois@gmail.com>,
 "Aurora" <reviews@aurora.apache.org>
Date: Tue, 05 Jan 2016 03:56:46 -0000
Message-ID: <20160105035646.26044.42341@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "John Sirois" <noreply@reviews.apache.org>
References: <20160105030513.26044.3118@reviews.apache.org>
In-Reply-To: <20160105030513.26044.3118@reviews.apache.org>
Reply-To: "John Sirois" <john.sirois@gmail.com>

--===============6500295238303818362==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Jan. 4, 2016, 8:05 p.m., John Sirois wrote:
> > Another answer could be to delete this test altogether.  It looks like it only really tests the proper converson from Time Amounts to fractional second waits.
> 
> Bill Farner wrote:
>     I'm tempted to take this route as well.  It's not clear to me what this test accomplishes, and it has the downside of high coupling to the implementation being tested.

Maxim - your test originally so your call.  For a refresh - see here: https://reviews.apache.org/r/35527/


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41915/#review112748
-----------------------------------------------------------


On Jan. 4, 2016, 8:02 p.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41915/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2016, 8:02 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Bugs: AURORA-1570
>     https://issues.apache.org/jira/browse/AURORA-1570
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Previously, a mock threading.Event was waited on in one thread
> and the count of waits was read in another thread.  Most thread
> memory models do not guaranty reads are fresh in this scenario
> unless there is a memory barrier of some sort forcing per-cpu
> caches to be flushed.
> 
> This change uses the underlying threading.Event as the memory
> barrier instead of mocking it and just wraps the event to record
> calls manually.
> 
>  src/main/python/apache/thermos/observer/task_observer.py      |  5 +++--
>  src/test/python/apache/thermos/observer/test_task_observer.py | 36 ++++++++++++++++++++++++------------
>  2 files changed, 27 insertions(+), 14 deletions(-)
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/thermos/observer/task_observer.py 1485de8faef52716f11b82a3556064de26c67427 
>   src/test/python/apache/thermos/observer/test_task_observer.py ace15c5305e75fac3a82971f4d71b92bcb37bafc 
> 
> Diff: https://reviews.apache.org/r/41915/diff/
> 
> 
> Testing
> -------
> 
> Before this change I got a failure between 1/5 and 1/10th of the
> time via:
> ```
> while true
> do
>   ./pants test src/test/python/apache/thermos/observer/ -- -kTaskObserverTest
> done
> ```
> 
> After the change I cannot trigger the failure.
> 
> 
> Thanks,
> 
> John Sirois
> 
>


--===============6500295238303818362==--