You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2016/10/24 23:19:34 UTC

Review Request 53154: GEODE-2026: fix flaky test

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

Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.


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


Repository: geode


Description
-------

The test was initializing a value to a String,
then starting an async thread that would keep
changing it to an int,
it slept for 100ms and then assummed that the async
thread had changed the value to an Integer.
In rare cases the thread never was able to to its first
put so the value was still a String.

The test now waits for the async thread to change the value
to an Integer instead of doing a 100ms sleep.


Diffs
-----

  geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 

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


Testing
-------

precheckin


Thanks,

Darrel Schneider


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53154/#review153812
-----------------------------------------------------------


Ship it!




Ship It!

- Ken Howe


On Oct. 25, 2016, 5:35 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53154/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2016, 5:35 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.
> 
> 
> Bugs: GEODE-2026
>     https://issues.apache.org/jira/browse/GEODE-2026
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The test was initializing a value to a String,
> then starting an async thread that would keep
> changing it to an int,
> it slept for 100ms and then assummed that the async
> thread had changed the value to an Integer.
> In rare cases the thread never was able to to its first
> put so the value was still a String.
> 
> The test now waits for the async thread to change the value
> to an Integer instead of doing a 100ms sleep.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 
> 
> Diff: https://reviews.apache.org/r/53154/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53154/
-----------------------------------------------------------

(Updated Oct. 25, 2016, 10:35 a.m.)


Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.


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


Repository: geode


Description
-------

The test was initializing a value to a String,
then starting an async thread that would keep
changing it to an int,
it slept for 100ms and then assummed that the async
thread had changed the value to an Integer.
In rare cases the thread never was able to to its first
put so the value was still a String.

The test now waits for the async thread to change the value
to an Integer instead of doing a 100ms sleep.


Diffs (updated)
-----

  geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 

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


Testing
-------

precheckin


Thanks,

Darrel Schneider


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53154/#review153810
-----------------------------------------------------------




geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java (line 1643)
<https://reviews.apache.org/r/53154/#comment223273>

    Need to add: vm1.invoke(waitForIntValue);


- Darrel Schneider


On Oct. 24, 2016, 4:19 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53154/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2016, 4:19 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.
> 
> 
> Bugs: GEODE-2026
>     https://issues.apache.org/jira/browse/GEODE-2026
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The test was initializing a value to a String,
> then starting an async thread that would keep
> changing it to an int,
> it slept for 100ms and then assummed that the async
> thread had changed the value to an Integer.
> In rare cases the thread never was able to to its first
> put so the value was still a String.
> 
> The test now waits for the async thread to change the value
> to an Integer instead of doing a 100ms sleep.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 
> 
> Diff: https://reviews.apache.org/r/53154/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Ken Howe <kh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53154/#review153793
-----------------------------------------------------------



So what did you decide with regard to the await timeout? You noted a change to 60 but your commit left it at 30.

- Ken Howe


On Oct. 24, 2016, 11:19 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53154/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2016, 11:19 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.
> 
> 
> Bugs: GEODE-2026
>     https://issues.apache.org/jira/browse/GEODE-2026
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The test was initializing a value to a String,
> then starting an async thread that would keep
> changing it to an int,
> it slept for 100ms and then assummed that the async
> thread had changed the value to an Integer.
> In rare cases the thread never was able to to its first
> put so the value was still a String.
> 
> The test now waits for the async thread to change the value
> to an Integer instead of doing a 100ms sleep.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 
> 
> Diff: https://reviews.apache.org/r/53154/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Darrel Schneider <ds...@pivotal.io>.

> On Oct. 24, 2016, 4:24 p.m., Scott Jewell wrote:
> > Code looks good.
> > 
> > Seems like 30 seconds is one of those things that should normally happen very fast,
> > but occasionally might be hung up for some reason (i.e. GC), so why not wait 200 or something.
> > A longer wait max would not slow the test down, but might miss an occasional GC pause or something.

I changed it to 60 which is used in lots of our tests.


- Darrel


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


On Oct. 24, 2016, 4:19 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53154/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2016, 4:19 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.
> 
> 
> Bugs: GEODE-2026
>     https://issues.apache.org/jira/browse/GEODE-2026
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The test was initializing a value to a String,
> then starting an async thread that would keep
> changing it to an int,
> it slept for 100ms and then assummed that the async
> thread had changed the value to an Integer.
> In rare cases the thread never was able to to its first
> put so the value was still a String.
> 
> The test now waits for the async thread to change the value
> to an Integer instead of doing a 100ms sleep.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 
> 
> Diff: https://reviews.apache.org/r/53154/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Scott Jewell <sj...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53154/#review153756
-----------------------------------------------------------


Ship it!




Code looks good.

Seems like 30 seconds is one of those things that should normally happen very fast,
but occasionally might be hung up for some reason (i.e. GC), so why not wait 200 or something.
A longer wait max would not slow the test down, but might miss an occasional GC pause or something.

- Scott Jewell


On Oct. 24, 2016, 11:19 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53154/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2016, 11:19 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.
> 
> 
> Bugs: GEODE-2026
>     https://issues.apache.org/jira/browse/GEODE-2026
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The test was initializing a value to a String,
> then starting an async thread that would keep
> changing it to an int,
> it slept for 100ms and then assummed that the async
> thread had changed the value to an Integer.
> In rare cases the thread never was able to to its first
> put so the value was still a String.
> 
> The test now waits for the async thread to change the value
> to an Integer instead of doing a 100ms sleep.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 
> 
> Diff: https://reviews.apache.org/r/53154/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>


Re: Review Request 53154: GEODE-2026: fix flaky test

Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53154/#review153760
-----------------------------------------------------------




geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java (line 1632)
<https://reviews.apache.org/r/53154/#comment223167>

    no need to "throw InterruptedException"


- Darrel Schneider


On Oct. 24, 2016, 4:19 p.m., Darrel Schneider wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53154/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2016, 4:19 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Eric Shu, Scott Jewell, and Ken Howe.
> 
> 
> Bugs: GEODE-2026
>     https://issues.apache.org/jira/browse/GEODE-2026
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> The test was initializing a value to a String,
> then starting an async thread that would keep
> changing it to an int,
> it slept for 100ms and then assummed that the async
> thread had changed the value to an Integer.
> In rare cases the thread never was able to to its first
> put so the value was still a String.
> 
> The test now waits for the async thread to change the value
> to an Integer instead of doing a 100ms sleep.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionDUnitTest.java 5ca3d014b4bac01e239561301666fae5fb0d61ea 
> 
> Diff: https://reviews.apache.org/r/53154/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> 
> Thanks,
> 
> Darrel Schneider
> 
>