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/03/23 19:03:05 UTC

Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

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

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


Repository: geode


Description
-------

Test issue. The test is not waiting for events to arrive for all the CqListeners; it waits for one of the CqListener (thae last one registered) and validates for all CqListener; based on the order at which CQlistners are invoked (and thread scheduling), when check for expected events are made, the events may not have arrived at all CqListeners.

Added wait logic in the Listner code, so that we wait for all the expected event to arrive.


Diffs
-----

  geode-core/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryTestListener.java e4026715fe7c742edac7cea6105d9bda357b4043 
  geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java be916651f80c04d03e72eceb06e01ef6b13691b8 

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


Testing
-------

Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.


Thanks,

anilkumar gingade

Re: Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

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

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

(Updated March 25, 2016, 7:10 p.m.)

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

Repository: geode

Description
-------

Test issue. The test is not waiting for events to arrive for all the CQs; it waits for one of the CQ (registered last) and validates events on other CQ. Based on the order at which CQs (listners) are invoked (and thread scheduling), the CQ events may not have been arrived for the CQ where check is called.

Added wait logic in the validate code...By doing so, the CQ (listner) will wait for all the expected event to arrive. And in future the caller of this code doesn't have to add any extra wait logic.

Also commented the debug log codes that were not needed.

Diffs
-----

geode-core/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryTestListener.java e4026715fe7c742edac7cea6105d9bda357b4043
geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java be916651f80c04d03e72eceb06e01ef6b13691b8

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

Testing (updated)
-------

Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.

Run tests:
CqPerfUsingPoolDUnitTest
CqQueryDUnitTest (10 times)
CqQueryUsingPoolDUnitTest (10 times)

Thanks,

anilkumar gingade

Re: Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

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

(Updated March 23, 2016, 8:35 p.m.)

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

Repository: geode

Description (updated)
-------

Added wait logic in the validate code...By doing so, the CQ (listner) will wait for all the expected event to arrive. And in future the caller of this code doesn't have to add any extra wait logic.

Also commented the debug log codes that were not needed.

Diffs
-----

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

Testing
-------

Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.

Thanks,

anilkumar gingade

Re: Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

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


> On March 23, 2016, 7:35 p.m., Barry Oglesby wrote:
> > geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java, line 1009
> > <https://reviews.apache.org/r/45227/diff/1/?file=1312273#file1312273line1009>
> >
> >     I'm not sure how this is validating all the listeners. It still looks like it is validating just the first one. Is that what you want?

You are right, it should have been read as CQ not CqListner...I will update the comments...

The method ValidateCQ() is called to see all the expected events are recieved by the CqListener associated with that CQ (in this case there is only one CqListner per CQ.

By moving Wait logic to ValidateCQ, we don't have to have separate wait logic before calling this method. This will help in the future...

Here is what test was doing:

    cqDUnitTest.waitForUpdated(client, "testMatchingCqs_3", CqQueryUsingPoolDUnitTest.KEY+size); // On a different CQ.
    
    cqDUnitTest.validateCQ(client, "testMatchingCqs_1",...) // Checks for events on other CQ.


- anilkumar


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


On March 23, 2016, 8:35 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45227/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 8:35 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Test issue. The test is not waiting for events to arrive for all the CQs; it waits for one of the CQ (registered last) and validates events on other CQ. Based on the order at which CQs (listners) are invoked (and thread scheduling), the CQ events may not have been arrived for the CQ where check is called.
> 
> Added wait logic in the validate code...By doing so, the CQ (listner) will wait for all the expected event to arrive. And in future the caller of this code doesn't have to add any extra wait logic.
> 
> Also commented the debug log codes that were not needed.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryTestListener.java e4026715fe7c742edac7cea6105d9bda357b4043 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java be916651f80c04d03e72eceb06e01ef6b13691b8 
> 
> Diff: https://reviews.apache.org/r/45227/diff/
> 
> 
> Testing
> -------
> 
> Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

Posted by Barry Oglesby <bo...@pivotal.io>.

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




geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java (line 1009)
<https://reviews.apache.org/r/45227/#comment187848>

    I'm not sure how this is validating all the listeners. It still looks like it is validating just the first one. Is that what you want?


- Barry Oglesby


On March 23, 2016, 6:03 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45227/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 6:03 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Test issue. The test is not waiting for events to arrive for all the CqListeners; it waits for one of the CqListener (thae last one registered) and validates for all CqListener; based on the order at which CQlistners are invoked (and thread scheduling), when check for expected events are made, the events may not have arrived at all CqListeners.
> 
> Added wait logic in the Listner code, so that we wait for all the expected event to arrive.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryTestListener.java e4026715fe7c742edac7cea6105d9bda357b4043 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java be916651f80c04d03e72eceb06e01ef6b13691b8 
> 
> Diff: https://reviews.apache.org/r/45227/diff/
> 
> 
> Testing
> -------
> 
> Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

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


> On March 23, 2016, 7:37 p.m., Barry Oglesby wrote:
> > geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java, line 1020
> > <https://reviews.apache.org/r/45227/diff/1/?file=1312273#file1312273line1020>
> >
> >     I assume you meant to comment out all the logging? If so, you might as well just delete the calls.

I just commented out, thinking it could be handy in future (for debugging)...


- anilkumar


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


On March 23, 2016, 8:35 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45227/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 8:35 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Test issue. The test is not waiting for events to arrive for all the CQs; it waits for one of the CQ (registered last) and validates events on other CQ. Based on the order at which CQs (listners) are invoked (and thread scheduling), the CQ events may not have been arrived for the CQ where check is called.
> 
> Added wait logic in the validate code...By doing so, the CQ (listner) will wait for all the expected event to arrive. And in future the caller of this code doesn't have to add any extra wait logic.
> 
> Also commented the debug log codes that were not needed.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryTestListener.java e4026715fe7c742edac7cea6105d9bda357b4043 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java be916651f80c04d03e72eceb06e01ef6b13691b8 
> 
> Diff: https://reviews.apache.org/r/45227/diff/
> 
> 
> Testing
> -------
> 
> Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 45227: GEODE-988: CI Failure: CqPerfUsingPoolDUnitTest.testMatchingCqs failed with AssertionFailedError

Posted by Barry Oglesby <bo...@pivotal.io>.

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




geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java (line 1020)
<https://reviews.apache.org/r/45227/#comment187849>

    I assume you meant to comment out all the logging? If so, you might as well just delete the calls.


- Barry Oglesby


On March 23, 2016, 6:03 p.m., anilkumar gingade wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45227/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 6:03 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, nabarun nag, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Test issue. The test is not waiting for events to arrive for all the CqListeners; it waits for one of the CqListener (thae last one registered) and validates for all CqListener; based on the order at which CQlistners are invoked (and thread scheduling), when check for expected events are made, the events may not have arrived at all CqListeners.
> 
> Added wait logic in the Listner code, so that we wait for all the expected event to arrive.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryTestListener.java e4026715fe7c742edac7cea6105d9bda357b4043 
>   geode-cq/src/test/java/com/gemstone/gemfire/cache/query/cq/dunit/CqQueryUsingPoolDUnitTest.java be916651f80c04d03e72eceb06e01ef6b13691b8 
> 
> Diff: https://reviews.apache.org/r/45227/diff/
> 
> 
> Testing
> -------
> 
> Was unable to reproduce the issue even after 300 runs. After going through test code made changes to address the wait issue.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>