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 2022/04/29 03:47:01 UTC
[GitHub] [geode] mivanac opened a new pull request, #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
mivanac opened a new pull request, #7633:
URL: https://github.com/apache/geode/pull/7633
…SendersTest
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
-->
### For all changes:
- [*] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [*] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [*] Is your initial contribution a single, squashed commit?
- [*] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
<!-- Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
-->
--
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] mivanac merged pull request #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
Posted by GitBox <gi...@apache.org>.
mivanac merged PR #7633:
URL: https://github.com/apache/geode/pull/7633
--
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] mivanac commented on a diff in pull request #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
Posted by GitBox <gi...@apache.org>.
mivanac commented on code in PR #7633:
URL: https://github.com/apache/geode/pull/7633#discussion_r861970465
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -258,7 +258,8 @@ public void testPingsToReceiversWithSamePortAndHostnameForSendersUseOnlyOneMoreC
await().untilAsserted(() -> assertThat(getSenderPoolDisconnects(vm1, senderId)).isEqualTo(0));
- await().untilAsserted(() -> assertThat(getSenderPoolConnects(vm1, senderId)).isEqualTo(4));
+ await().untilAsserted(
+ () -> assertThat(getSenderPoolConnects(vm1, senderId)).isLessThanOrEqualTo(4));
Review Comment:
Because it depends on the moment we are checking number of connections, and it can be 3 or 4.
--
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] DonalEvans commented on a diff in pull request #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on code in PR #7633:
URL: https://github.com/apache/geode/pull/7633#discussion_r861977574
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -258,7 +258,8 @@ public void testPingsToReceiversWithSamePortAndHostnameForSendersUseOnlyOneMoreC
await().untilAsserted(() -> assertThat(getSenderPoolDisconnects(vm1, senderId)).isEqualTo(0));
- await().untilAsserted(() -> assertThat(getSenderPoolConnects(vm1, senderId)).isEqualTo(4));
+ await().untilAsserted(
+ () -> assertThat(getSenderPoolConnects(vm1, senderId)).isLessThanOrEqualTo(4));
Review Comment:
But the assertion is in an `await()` so it gets checked repeatedly (every 50 ms by default) until the await times out (which takes 5 minutes). So it seems like the time we're checking the stat can't be the reason.
--
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] pivotal-jbarrett commented on a diff in pull request #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on code in PR #7633:
URL: https://github.com/apache/geode/pull/7633#discussion_r862099954
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -258,7 +258,8 @@ public void testPingsToReceiversWithSamePortAndHostnameForSendersUseOnlyOneMoreC
await().untilAsserted(() -> assertThat(getSenderPoolDisconnects(vm1, senderId)).isEqualTo(0));
- await().untilAsserted(() -> assertThat(getSenderPoolConnects(vm1, senderId)).isEqualTo(4));
+ await().untilAsserted(
+ () -> assertThat(getSenderPoolConnects(vm1, senderId)).isLessThanOrEqualTo(4));
Review Comment:
If it can be 3 or 4 then let's do:
`assertThat(getSenderPoolConnects(vm1, senderId)).isIn(3,4)`
--
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] mivanac commented on a diff in pull request #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
Posted by GitBox <gi...@apache.org>.
mivanac commented on code in PR #7633:
URL: https://github.com/apache/geode/pull/7633#discussion_r861988382
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -258,7 +258,8 @@ public void testPingsToReceiversWithSamePortAndHostnameForSendersUseOnlyOneMoreC
await().untilAsserted(() -> assertThat(getSenderPoolDisconnects(vm1, senderId)).isEqualTo(0));
- await().untilAsserted(() -> assertThat(getSenderPoolConnects(vm1, senderId)).isEqualTo(4));
+ await().untilAsserted(
+ () -> assertThat(getSenderPoolConnects(vm1, senderId)).isLessThanOrEqualTo(4));
Review Comment:
It depends on time, when we trigger pinging for each server, which in this solution is delayed by new parameter. But it is depending on timing, so in some test iteration, result can vary.
--
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] DonalEvans commented on a diff in pull request #7633: GEODE-10266: update SeveralGatewayReceiversWithSamePortAndHostnameFor…
Posted by GitBox <gi...@apache.org>.
DonalEvans commented on code in PR #7633:
URL: https://github.com/apache/geode/pull/7633#discussion_r861967352
##########
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java:
##########
@@ -258,7 +258,8 @@ public void testPingsToReceiversWithSamePortAndHostnameForSendersUseOnlyOneMoreC
await().untilAsserted(() -> assertThat(getSenderPoolDisconnects(vm1, senderId)).isEqualTo(0));
- await().untilAsserted(() -> assertThat(getSenderPoolConnects(vm1, senderId)).isEqualTo(4));
+ await().untilAsserted(
+ () -> assertThat(getSenderPoolConnects(vm1, senderId)).isLessThanOrEqualTo(4));
Review Comment:
Why is the number of pool connects not deterministic in this test? Is the fact that it can vary a bug, or expected behaviour?
--
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