You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anilkumar gingade <ag...@pivotal.io> on 2016/05/19 21:47:40 UTC

Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

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

Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.


Repository: geode


Description
-------

GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.


Diffs
-----

  geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 

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


Testing
-------

Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...


Thanks,

anilkumar gingade

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

Posted by Dan Smith <ds...@pivotal.io>.

> On May 20, 2016, 5:42 p.m., Jason Huynh wrote:
> > I forgot to ask but do we remove the FlakyTest category from this test now?

If the test is fixed, you should remove the category.


- Dan


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


On May 19, 2016, 10:54 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 10:54 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -----
> 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> -------
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47615/#review134169
-----------------------------------------------------------



I forgot to ask but do we remove the FlakyTest category from this test now?

- Jason Huynh


On May 19, 2016, 10:54 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 10:54 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -----
> 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> -------
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47615/
-----------------------------------------------------------

(Updated May 19, 2016, 10:54 p.m.)


Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.


Changes
-------

Based on review removed waits for events that were not needed...


Repository: geode


Description
-------

GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.


Diffs (updated)
-----

  geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 

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


Testing
-------

Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...


Thanks,

anilkumar gingade

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

Posted by anilkumar gingade <ag...@pivotal.io>.

> On May 19, 2016, 10 p.m., Jason Huynh wrote:
> > Long term this test could probably be broken up into smaller chunks as it's testing multiple scenarios

Agree...


> On May 19, 2016, 10 p.m., Jason Huynh wrote:
> > geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java, line 432
> > <https://reviews.apache.org/r/47615/diff/1/?file=1388453#file1388453line432>
> >
> >     I think having the waitForUpdate (which happens a few lines below) will be enough for cq_1?  He won't get the create and update out of order correct?

Yes...waiting for update is good enough...It doesn't recieve events out of order in this case/test. I added there, for debugging purpose, to identify which event got missed...If the test is working as expected, it should not add any overhead...


> On May 19, 2016, 10 p.m., Jason Huynh wrote:
> > geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java, line 433
> > <https://reviews.apache.org/r/47615/diff/1/?file=1388453#file1388453line433>
> >
> >     I don't think we need this one here (for cq_3), we have a wait for updated later and do not do any validation until after the update.

Its not needed...I will remove this validation...And send a new review request...


- anilkumar


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


On May 19, 2016, 9:47 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 9:47 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -----
> 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> -------
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47615/#review134056
-----------------------------------------------------------



Long term this test could probably be broken up into smaller chunks as it's testing multiple scenarios


geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java (line 432)
<https://reviews.apache.org/r/47615/#comment198694>

    I think having the waitForUpdate (which happens a few lines below) will be enough for cq_1?  He won't get the create and update out of order correct?



geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java (line 433)
<https://reviews.apache.org/r/47615/#comment198693>

    I don't think we need this one here (for cq_3), we have a wait for updated later and do not do any validation until after the update.


- Jason Huynh


On May 19, 2016, 9:47 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 9:47 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -----
> 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> -------
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 47615: GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop.

Posted by Jason Huynh <hu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/47615/#review134060
-----------------------------------------------------------


Ship it!




- Jason Huynh


On May 19, 2016, 9:47 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47615/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 9:47 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Darrel Schneider, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> GEODE-988: Added wait for events to arrive on stopped CQ before it is stopped. The test stops the CQ and verifies no events are recieved in stopped state; it was doing that by checking operations/events that happend before stop and not counting the changes happened during stop. In a slower environment, it could so happen that the CQ may not have recieved all the events before it is stopped, thus causing the validation to fail.
> 
> 
> Diffs
> -----
> 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqPerfUsingPoolDUnitTest.java 130f924643ad350539f02a59f34b6ba18cef1c10 
> 
> Diff: https://reviews.apache.org/r/47615/diff/
> 
> 
> Testing
> -------
> 
> Reproduced the issue by slowing down the CQ listener processing. Verified the test passes with the changes...
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>