You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by nabarun nag <nn...@pivotal.io> on 2016/11/02 20:14:03 UTC
Review Request 53401: GEODE-1932: Protected use of global variables
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53401/
-----------------------------------------------------------
Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian zhou.
Repository: geode
Description
-------
* As per the stacktrace in the ticket we could see that the failure was happening because the test was getting the wrong result for the function execution.
* In the TestFunction executeFunctionReexecuteExceptionOnServer we could see that it was using global counter static variable retryCount.
* This global variable retryCount was also used by executeFunctionReexecuteException.
* So if these functions are invoked parallely we have a chance that these counter global variable may have corrupted values.
* Hence we gave both the test functions their own retry count global static variable, they dont share retry count anymore.
* Also we made these test functions synchronized so that reexecution by different threads do no corrupt the global static variables.
* The test function executeFunctionReexecuteExceptionOnServer stops re-execution when the retryCount >= 5 but in the test we validate that retryCount == 5. This was made uniform by making the test validate that retryCount >= 5
* Added more information in the assertEquals, which now output the received and expected values when there is an assertion failure.
Diffs
-----
geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java d217792
geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java f9f05ab
Diff: https://reviews.apache.org/r/53401/diff/
Testing
-------
precheckin
IntelliJ multiple executions.
Thanks,
nabarun nag
Re: Review Request 53401: GEODE-1932: Protected use of global
variables
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53401/#review154627
-----------------------------------------------------------
Fix it, then Ship it!
geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java (line 666)
<https://reviews.apache.org/r/53401/#comment224260>
Couldn't this just be assertEquals?
- Dan Smith
On Nov. 2, 2016, 8:14 p.m., nabarun nag wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53401/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2016, 8:14 p.m.)
>
>
> Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * As per the stacktrace in the ticket we could see that the failure was happening because the test was getting the wrong result for the function execution.
> * In the TestFunction executeFunctionReexecuteExceptionOnServer we could see that it was using global counter static variable retryCount.
> * This global variable retryCount was also used by executeFunctionReexecuteException.
> * So if these functions are invoked parallely we have a chance that these counter global variable may have corrupted values.
> * Hence we gave both the test functions their own retry count global static variable, they dont share retry count anymore.
> * Also we made these test functions synchronized so that reexecution by different threads do no corrupt the global static variables.
> * The test function executeFunctionReexecuteExceptionOnServer stops re-execution when the retryCount >= 5 but in the test we validate that retryCount == 5. This was made uniform by making the test validate that retryCount >= 5
> * Added more information in the assertEquals, which now output the received and expected values when there is an assertion failure.
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java d217792
> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java f9f05ab
>
> Diff: https://reviews.apache.org/r/53401/diff/
>
>
> Testing
> -------
>
> precheckin
> IntelliJ multiple executions.
>
>
> Thanks,
>
> nabarun nag
>
>
Re: Review Request 53401: GEODE-1932: Protected use of global
variables
Posted by Jason Huynh <jh...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53401/#review154645
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java (line 668)
<https://reviews.apache.org/r/53401/#comment224277>
the message is doing a list.get(0), but it should be doing a list.get(1)
- Jason Huynh
On Nov. 2, 2016, 8:14 p.m., nabarun nag wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53401/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2016, 8:14 p.m.)
>
>
> Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * As per the stacktrace in the ticket we could see that the failure was happening because the test was getting the wrong result for the function execution.
> * In the TestFunction executeFunctionReexecuteExceptionOnServer we could see that it was using global counter static variable retryCount.
> * This global variable retryCount was also used by executeFunctionReexecuteException.
> * So if these functions are invoked parallely we have a chance that these counter global variable may have corrupted values.
> * Hence we gave both the test functions their own retry count global static variable, they dont share retry count anymore.
> * Also we made these test functions synchronized so that reexecution by different threads do no corrupt the global static variables.
> * The test function executeFunctionReexecuteExceptionOnServer stops re-execution when the retryCount >= 5 but in the test we validate that retryCount == 5. This was made uniform by making the test validate that retryCount >= 5
> * Added more information in the assertEquals, which now output the received and expected values when there is an assertion failure.
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java d217792
> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java f9f05ab
>
> Diff: https://reviews.apache.org/r/53401/diff/
>
>
> Testing
> -------
>
> precheckin
> IntelliJ multiple executions.
>
>
> Thanks,
>
> nabarun nag
>
>
Re: Review Request 53401: GEODE-1932: Protected use of global
variables
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53401/#review154643
-----------------------------------------------------------
Ship it!
Ship It!
- xiaojian zhou
On Nov. 2, 2016, 8:14 p.m., nabarun nag wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53401/
> -----------------------------------------------------------
>
> (Updated Nov. 2, 2016, 8:14 p.m.)
>
>
> Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> * As per the stacktrace in the ticket we could see that the failure was happening because the test was getting the wrong result for the function execution.
> * In the TestFunction executeFunctionReexecuteExceptionOnServer we could see that it was using global counter static variable retryCount.
> * This global variable retryCount was also used by executeFunctionReexecuteException.
> * So if these functions are invoked parallely we have a chance that these counter global variable may have corrupted values.
> * Hence we gave both the test functions their own retry count global static variable, they dont share retry count anymore.
> * Also we made these test functions synchronized so that reexecution by different threads do no corrupt the global static variables.
> * The test function executeFunctionReexecuteExceptionOnServer stops re-execution when the retryCount >= 5 but in the test we validate that retryCount == 5. This was made uniform by making the test validate that retryCount >= 5
> * Added more information in the assertEquals, which now output the received and expected values when there is an assertion failure.
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java d217792
> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java f9f05ab
>
> Diff: https://reviews.apache.org/r/53401/diff/
>
>
> Testing
> -------
>
> precheckin
> IntelliJ multiple executions.
>
>
> Thanks,
>
> nabarun nag
>
>