You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/07/28 16:47:14 UTC
[GitHub] [geode-native] mmartell opened a new pull request #835: GEODE-9472: Relax the ratio of before and after threads.
mmartell opened a new pull request #835:
URL: https://github.com/apache/geode-native/pull/835
The heuristic was a bit too strict. With 25 iterations, there was no reason to have the range be so small.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #835: GEODE-9472: Relax the ratio of before and after threads.
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #835:
URL: https://github.com/apache/geode-native/pull/835#discussion_r678561976
##########
File path: clicache/integration-test2/GarbageCollectCache.cs
##########
@@ -83,10 +83,9 @@ public void VerifyNoLeakedThreads()
// environment startup.
if (i > 5)
{
- //Assert.True(.8 < ratio && ratio < 1.3);
string error = "ncThreadsBefore = " + ncThreadsBefore.ToString() +
", ncThreadsAfter = " + ncThreadsAfter.ToString();
- Assert.False(!(.8 < ratio && ratio < 1.3), error);
+ Assert.False(!(.5 < ratio && ratio < 2.0), error);
Review comment:
I put that in for debugging purposes. Assert.True wasn't giving me the output. Agree with your comment. Will change it back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey merged pull request #835: GEODE-9472: Relax the ratio of before and after threads.
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey merged pull request #835:
URL: https://github.com/apache/geode-native/pull/835
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey commented on a change in pull request #835: GEODE-9472: Relax the ratio of before and after threads.
Posted by GitBox <gi...@apache.org>.
pdxcodemonkey commented on a change in pull request #835:
URL: https://github.com/apache/geode-native/pull/835#discussion_r678505200
##########
File path: clicache/integration-test2/GarbageCollectCache.cs
##########
@@ -83,10 +83,9 @@ public void VerifyNoLeakedThreads()
// environment startup.
if (i > 5)
{
- //Assert.True(.8 < ratio && ratio < 1.3);
string error = "ncThreadsBefore = " + ncThreadsBefore.ToString() +
", ncThreadsAfter = " + ncThreadsAfter.ToString();
- Assert.False(!(.8 < ratio && ratio < 1.3), error);
+ Assert.False(!(.5 < ratio && ratio < 2.0), error);
Review comment:
Can we please make this a positive assert, rather than a negative? It's very hard to read. The commented-out line that you removed is MUCH clearer, while stating the same thing.
```
Assert.True(.5 < ratio < 2.0);
```
Even changing it to two different assertions is easier to read, and potentially more helpful for debugging/diagnosis:
```
Assert.False(ratio < .5, error);
Assert.False(ratio > 2.0, error);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode-native] mmartell commented on a change in pull request #835: GEODE-9472: Relax the ratio of before and after threads.
Posted by GitBox <gi...@apache.org>.
mmartell commented on a change in pull request #835:
URL: https://github.com/apache/geode-native/pull/835#discussion_r678561976
##########
File path: clicache/integration-test2/GarbageCollectCache.cs
##########
@@ -83,10 +83,9 @@ public void VerifyNoLeakedThreads()
// environment startup.
if (i > 5)
{
- //Assert.True(.8 < ratio && ratio < 1.3);
string error = "ncThreadsBefore = " + ncThreadsBefore.ToString() +
", ncThreadsAfter = " + ncThreadsAfter.ToString();
- Assert.False(!(.8 < ratio && ratio < 1.3), error);
+ Assert.False(!(.5 < ratio && ratio < 2.0), error);
Review comment:
I put that in for debugging purposes. Assert.True wasn't giving me the output. Agree with your comment. Will change it back since we don't need to debug anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org