Re: Review Request 37321: GEODE-138: remove race condition from testEventDelivery

2015-08-10 Thread Kirk Lund 

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

Ship it!


Ship It!

- Kirk Lund


On Aug. 10, 2015, 10:02 p.m., Darrel Schneider wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37321/
> ---
> 
> (Updated Aug. 10, 2015, 10:02 p.m.)
> 
> 
> Review request for geode and Kirk Lund.
> 
> 
> Bugs: GEODE-138
> https://issues.apache.org/jira/browse/GEODE-138
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The race due to the test expecting local event delivery to be synchronous
> but the implementation of off-heap resource manager events being async.
> The test has been changed to always use a wait criteria when waiting
> for a memory event to arrive.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java
>  0b704a6955e641e249c00fc0942d4bca214ebbaf 
> 
> Diff: https://reviews.apache.org/r/37321/diff/
> 
> 
> Testing
> ---
> 
> Ran MemoryThresholdsOffHeapDUnitTest (it was the only thing modified by this 
> fix).
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 37321: GEODE-138: remove race condition from testEventDelivery

2015-08-10 Thread Darrel Schneider 

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

(Updated Aug. 10, 2015, 3:02 p.m.)


Review request for geode and Kirk Lund.


Changes
---

Based on review feedback change to callers to pass "true" as the 
useWaitCriterion


Bugs: GEODE-138
https://issues.apache.org/jira/browse/GEODE-138


Repository: geode


Description
---

The race due to the test expecting local event delivery to be synchronous
but the implementation of off-heap resource manager events being async.
The test has been changed to always use a wait criteria when waiting
for a memory event to arrive.


Diffs (updated)
-

  
gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java
 0b704a6955e641e249c00fc0942d4bca214ebbaf 

Diff: https://reviews.apache.org/r/37321/diff/


Testing
---

Ran MemoryThresholdsOffHeapDUnitTest (it was the only thing modified by this 
fix).


Thanks,

Darrel Schneider

Re: Review Request 37321: GEODE-138: remove race condition from testEventDelivery

2015-08-10 Thread Darrel Schneider 


> On Aug. 10, 2015, 2:36 p.m., Kirk Lund wrote:
> > gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java,
> >  line 1587
> > 
> >
> > Did you want to hardcode to true or add a line to assert the argument 
> > is true?
> > 
> > Or maybe change the parameter to non-final and have the 1st line of 
> > this method change it's value to true?

Originally I thought I would just change the callers to pass true in. In the 
future their might be a valid case for the event being sync in which case this 
method could then be used with a parameter of false. Since the parameter is 
accessed from a wait criteria it has to be a final. And it looked pretty wierd 
to make it no final; and then have a local final var that takes the param and 
just makes it true.
I'll go ahead and remove my changes to the param and just change all the 
current callers to set the param to true.


- Darrel


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


On Aug. 10, 2015, 2:28 p.m., Darrel Schneider wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37321/
> ---
> 
> (Updated Aug. 10, 2015, 2:28 p.m.)
> 
> 
> Review request for geode and Kirk Lund.
> 
> 
> Bugs: GEODE-138
> https://issues.apache.org/jira/browse/GEODE-138
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The race due to the test expecting local event delivery to be synchronous
> but the implementation of off-heap resource manager events being async.
> The test has been changed to always use a wait criteria when waiting
> for a memory event to arrive.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java
>  0b704a6955e641e249c00fc0942d4bca214ebbaf 
> 
> Diff: https://reviews.apache.org/r/37321/diff/
> 
> 
> Testing
> ---
> 
> Ran MemoryThresholdsOffHeapDUnitTest (it was the only thing modified by this 
> fix).
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Re: Review Request 37321: GEODE-138: remove race condition from testEventDelivery

2015-08-10 Thread Kirk Lund 

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



gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java
 (line 1587)


Did you want to hardcode to true or add a line to assert the argument is 
true?

Or maybe change the parameter to non-final and have the 1st line of this 
method change it's value to true?


- Kirk Lund


On Aug. 10, 2015, 9:28 p.m., Darrel Schneider wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37321/
> ---
> 
> (Updated Aug. 10, 2015, 9:28 p.m.)
> 
> 
> Review request for geode and Kirk Lund.
> 
> 
> Bugs: GEODE-138
> https://issues.apache.org/jira/browse/GEODE-138
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> The race due to the test expecting local event delivery to be synchronous
> but the implementation of off-heap resource manager events being async.
> The test has been changed to always use a wait criteria when waiting
> for a memory event to arrive.
> 
> 
> Diffs
> -
> 
>   
> gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java
>  0b704a6955e641e249c00fc0942d4bca214ebbaf 
> 
> Diff: https://reviews.apache.org/r/37321/diff/
> 
> 
> Testing
> ---
> 
> Ran MemoryThresholdsOffHeapDUnitTest (it was the only thing modified by this 
> fix).
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>

Review Request 37321: GEODE-138: remove race condition from testEventDelivery

2015-08-10 Thread Darrel Schneider 

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

Review request for geode and Kirk Lund.


Bugs: GEODE-138
https://issues.apache.org/jira/browse/GEODE-138


Repository: geode


Description
---

The race due to the test expecting local event delivery to be synchronous
but the implementation of off-heap resource manager events being async.
The test has been changed to always use a wait criteria when waiting
for a memory event to arrive.


Diffs
-

  
gemfire-core/src/test/java/com/gemstone/gemfire/cache/management/MemoryThresholdsOffHeapDUnitTest.java
 0b704a6955e641e249c00fc0942d4bca214ebbaf 

Diff: https://reviews.apache.org/r/37321/diff/


Testing
---

Ran MemoryThresholdsOffHeapDUnitTest (it was the only thing modified by this 
fix).


Thanks,

Darrel Schneider