Re: Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-24 Thread anilkumar gingade

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


Ship it!




Ship It!

- anilkumar gingade


On April 24, 2017, 10 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58628/
> ---
> 
> (Updated April 24, 2017, 10 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, 
> and Lynn Gallinat.
> 
> 
> Bugs: GEODE-2785
> https://issues.apache.org/jira/browse/GEODE-2785
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Verify that all buckets lost primary get the afterSecondary callbacks.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
>  7fd470f 
> 
> 
> Diff: https://reviews.apache.org/r/58628/diff/4/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Eric Shu
> 
>



Re: Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-24 Thread Eric Shu

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

(Updated April 24, 2017, 10 p.m.)


Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, and 
Lynn Gallinat.


Changes
---

fix a typo.


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


Repository: geode


Description
---

Verify that all buckets lost primary get the afterSecondary callbacks.


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
 7fd470f 


Diff: https://reviews.apache.org/r/58628/diff/4/

Changes: https://reviews.apache.org/r/58628/diff/3-4/


Testing
---

precheckin


Thanks,

Eric Shu



Re: Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-24 Thread Eric Shu


> On April 24, 2017, 9:43 p.m., Jason Huynh wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
> > Lines 149 (patched)
> > 
> >
> > If afterSecondary is invoked more than once, do we want the test to 
> > fail?

I would rather test to fail if multiple afterSecondary callbacks are invoked. 
At least it will be a performance issue, why a bucket on a node lost primary 
twice? If this happens, I think we may need to investage further.


- Eric


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


On April 21, 2017, 9:14 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58628/
> ---
> 
> (Updated April 21, 2017, 9:14 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, 
> and Lynn Gallinat.
> 
> 
> Bugs: GEODE-2785
> https://issues.apache.org/jira/browse/GEODE-2785
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Verify that all buckets lost primary get the afterSecondary callbacks.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
>  7fd470f 
> 
> 
> Diff: https://reviews.apache.org/r/58628/diff/3/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Eric Shu
> 
>



Re: Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-24 Thread Jason Huynh

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




geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
Lines 146 (patched)


typo: bucket



geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
Lines 149 (patched)


If afterSecondary is invoked more than once, do we want the test to fail?



geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
Lines 157 (patched)


should this be fore VM2 and not VM1?

Same as before, do we want the test to fail when it gets into that 
situation or do we want to just ignore it and adjust the expected number of 
times?


- Jason Huynh


On April 21, 2017, 9:14 p.m., Eric Shu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58628/
> ---
> 
> (Updated April 21, 2017, 9:14 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, 
> and Lynn Gallinat.
> 
> 
> Bugs: GEODE-2785
> https://issues.apache.org/jira/browse/GEODE-2785
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Verify that all buckets lost primary get the afterSecondary callbacks.
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
>  7fd470f 
> 
> 
> Diff: https://reviews.apache.org/r/58628/diff/3/
> 
> 
> Testing
> ---
> 
> precheckin
> 
> 
> Thanks,
> 
> Eric Shu
> 
>



Re: Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-21 Thread Eric Shu

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

(Updated April 21, 2017, 9:14 p.m.)


Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, and 
Lynn Gallinat.


Changes
---

Actual changes.


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


Repository: geode


Description
---

Verify that all buckets lost primary get the afterSecondary callbacks.


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
 7fd470f 


Diff: https://reviews.apache.org/r/58628/diff/3/

Changes: https://reviews.apache.org/r/58628/diff/2-3/


Testing
---

precheckin


Thanks,

Eric Shu



Re: Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-21 Thread Eric Shu

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

(Updated April 21, 2017, 9:12 p.m.)


Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, and 
Lynn Gallinat.


Changes
---

Fix a copy paste error.


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


Repository: geode


Description
---

Verify that all buckets lost primary get the afterSecondary callbacks.


Diffs (updated)
-

  
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
 9c38948 


Diff: https://reviews.apache.org/r/58628/diff/2/

Changes: https://reviews.apache.org/r/58628/diff/1-2/


Testing
---

precheckin


Thanks,

Eric Shu



Review Request 58628: GEODE-2785: Fix a test issue with additional afterSecondary invocations

2017-04-21 Thread Eric Shu

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

Review request for geode, anilkumar gingade, Darrel Schneider, Jason Huynh, and 
Lynn Gallinat.


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


Repository: geode


Description
---

Verify that all buckets lost primary get the afterSecondary callbacks.


Diffs
-

  
geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java
 9c38948 


Diff: https://reviews.apache.org/r/58628/diff/1/


Testing
---

precheckin


Thanks,

Eric Shu