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/06/01 22:52:24 UTC
[GitHub] [geode] kirklund opened a new pull request #6542: DRAFT: GEODE-9132: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
kirklund opened a new pull request #6542:
URL: https://github.com/apache/geode/pull/6542
Prevent ServerConnectivityExceptions when test encounters long GCs or
slow CPUs.
Add test-only system property to AbstractCacheServer for
maximumTimeBetweenPings.
Use system property to set maximumTimeBetweenPings to GeodeAwaitility
timeout.
Use GeodeAwaitility timeout for client pool readTimeout.
Remove unnecessary client pool settings within each test.
Inline getCause within Throwable assertions.
Thank you for submitting a contribution to Apache Geode.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646990925
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO:KIRK: fails
.untilAsserted(() -> assertThat(clientCounter.getCreates()).isEqualTo(ONE_HUNDRED)));
Review comment:
TODO: Unfortunately, this assertion is flaky and I don't know why
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund merged pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund merged pull request #6542:
URL: https://github.com/apache/geode/pull/6542
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646990925
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO:KIRK: fails
.untilAsserted(() -> assertThat(clientCounter.getCreates()).isEqualTo(ONE_HUNDRED)));
Review comment:
TODO: Unfortunately, the following assertion is flaky and I don't know why
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646988873
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
Review comment:
TODO: I don't know what causes GEODE-7812
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646988723
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2474,28 +2413,22 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
int serverPort1 = server1.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
-
int serverPort2 = server2.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
-
int serverPort3 = server3.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
client1.invoke(() -> new ClientBuilder()
- .concurrencyChecksEnabled()
- .prSingleHopEnabled(true)
.serverPorts(serverPort1, serverPort2, serverPort3)
.subscriptionAckInterval()
.subscriptionEnabled(true)
.subscriptionRedundancy()
- .readTimeout(100000)
+ .readTimeoutMillis(100000)
Review comment:
TODO: custom readTimeoutMillis should be removed from testEventIdOutOfOrderInPartitionRegionSingleHop
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2459,13 +2401,10 @@ public void testPartialKeyInPRSingleHopWithRedundancy() throws Exception {
assertThat(newSizeOnClient2).isEqualTo(newSizeOnServer1);
}
-
/**
* The purpose of this test is to validate that when two servers of three in a cluster configured
* with a client doing singlehop, that the client gets afterCreate messages for each entry in the
- * putall.
- * Further, we also check that the region size is correct on the remaining server.
- *
+ * putall. Further, we also check that the region size is correct on the remaining server.
*/
@Test
public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
Review comment:
Unfortunately, I haven't been able to fix the flakiness of testEventIdOutOfOrderInPartitionRegionSingleHop.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mhansonp commented on pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
mhansonp commented on pull request #6542:
URL: https://github.com/apache/geode/pull/6542#issuecomment-858382057
Well done Kirk!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646988723
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2474,28 +2413,22 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
int serverPort1 = server1.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
-
int serverPort2 = server2.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
-
int serverPort3 = server3.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
client1.invoke(() -> new ClientBuilder()
- .concurrencyChecksEnabled()
- .prSingleHopEnabled(true)
.serverPorts(serverPort1, serverPort2, serverPort3)
.subscriptionAckInterval()
.subscriptionEnabled(true)
.subscriptionRedundancy()
- .readTimeout(100000)
+ .readTimeoutMillis(100000)
Review comment:
TODO: custom readTimeoutMillis should be removed from testEventIdOutOfOrderInPartitionRegionSingleHop
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646990925
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO:KIRK: fails
.untilAsserted(() -> assertThat(clientCounter.getCreates()).isEqualTo(ONE_HUNDRED)));
Review comment:
TODO: Unfortunately, this assertion in testEventIdOutOfOrderInPartitionRegionSingleHop is flaky and I don't know why
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646988723
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2474,28 +2413,22 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
int serverPort1 = server1.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
-
int serverPort2 = server2.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
-
int serverPort3 = server3.invoke(() -> new ServerBuilder()
.regionShortcut(PARTITION)
.create());
client1.invoke(() -> new ClientBuilder()
- .concurrencyChecksEnabled()
- .prSingleHopEnabled(true)
.serverPorts(serverPort1, serverPort2, serverPort3)
.subscriptionAckInterval()
.subscriptionEnabled(true)
.subscriptionRedundancy()
- .readTimeout(100000)
+ .readTimeoutMillis(100000)
Review comment:
TODO: custom readTimeoutMillis should be removed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646990925
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO:KIRK: fails
.untilAsserted(() -> assertThat(clientCounter.getCreates()).isEqualTo(ONE_HUNDRED)));
Review comment:
TODO: Unfortunately, this assertion in testEventIdOutOfOrderInPartitionRegionSingleHop is flaky and I don't know why
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r647007193
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED);
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO: GEODE-9242 fails on this assertion even with await
.untilAsserted(() -> assertThat(clientCounter.getCreates()).isEqualTo(ONE_HUNDRED)));
Review comment:
TODO: Unfortunately, this assertion in testEventIdOutOfOrderInPartitionRegionSingleHop is still flaky
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646989316
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO:KIRK: fails
Review comment:
TODO: Unfortunately, the following assertion is flaky and I don't know why
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646988455
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2459,13 +2401,10 @@ public void testPartialKeyInPRSingleHopWithRedundancy() throws Exception {
assertThat(newSizeOnClient2).isEqualTo(newSizeOnServer1);
}
-
/**
* The purpose of this test is to validate that when two servers of three in a cluster configured
* with a client doing singlehop, that the client gets afterCreate messages for each entry in the
- * putall.
- * Further, we also check that the region size is correct on the remaining server.
- *
+ * putall. Further, we also check that the region size is correct on the remaining server.
*/
@Test
public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
Review comment:
Unfortunately, I haven't been able to fix the flakiness of testEventIdOutOfOrderInPartitionRegionSingleHop.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r647007812
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED);
});
- client1.invoke(() -> await()
+ client1.invoke(() -> await() // TODO: GEODE-9242 fails on this assertion even with await
.untilAsserted(() -> assertThat(clientCounter.getCreates()).isEqualTo(ONE_HUNDRED)));
Review comment:
TODO: Unfortunately, this assertion in testEventIdOutOfOrderInPartitionRegionSingleHop is still flaky (GEODE-9242)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6542: GEODE-9103: Fix ServerConnectivityExceptions in PutAllClientServerDistributedTest
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6542:
URL: https://github.com/apache/geode/pull/6542#discussion_r646988873
##########
File path: geode-cq/src/distributedTest/java/org/apache/geode/internal/cache/PutAllClientServerDistributedTest.java
##########
@@ -2541,10 +2474,10 @@ public void testEventIdOutOfOrderInPartitionRegionSingleHop() {
// client1 add listener and putAll
client1.invoke(() -> {
Region<String, TickerData> region = getClientCache().getRegion(regionName);
- doPutAll(region, keyPrefix, ONE_HUNDRED); // fails in GEODE-7812
+ doPutAll(region, keyPrefix, ONE_HUNDRED); // TODO:KIRK: fails in GEODE-7812
Review comment:
TODO: I don't know what causes GEODE-7812
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org