dschneider-pivotal commented on a change in pull request #7214:
URL: https://github.com/apache/geode/pull/7214#discussion_r798856752
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/UpdatePropagationDUnitTest.java
##########
@@ -91,30 +100,63 @@ public final void postSetUp() throws Exception {
// Server2 VM
server2 = host.getVM(1);
+ // Server3 VM
+ server3 = host.getVM(2);
+
// Client 1 VM
- client1 = host.getVM(2);
+ client1 = host.getVM(3);
// client 2 VM
- client2 = host.getVM(3);
+ client2 = host.getVM(4);
- PORT1 = server1.invoke(this::createServerCache);
- PORT2 = server2.invoke(this::createServerCache);
+ PORT1 = server1.invoke(() -> createServerCache());
+ PORT2 = server2.invoke(() -> createServerCache());
+ PORT3 = server3.invoke(() -> createServerCache());
- client1.invoke(
- () ->
createClientCache(NetworkUtils.getServerHostName(server1.getHost()), PORT1,
PORT2));
- client2.invoke(
- () ->
createClientCache(NetworkUtils.getServerHostName(server1.getHost()), PORT1,
PORT2));
+ hostnameServer1 = NetworkUtils.getServerHostName(server1.getHost());
+ hostnameServer3 = NetworkUtils.getServerHostName(server3.getHost());
IgnoredException.addIgnoredException("java.net.SocketException");
IgnoredException.addIgnoredException("Unexpected IOException");
}
+
+
+ @Test
+ public void updatesArePropagatedToAllMembersWhenOneKilled() throws Exception
{
+ client1.invoke(
+ () -> createClientCache(hostnameServer1, PORT1));
+ client2.invoke(
+ () -> createClientCache(hostnameServer3, PORT3));
+ int entries = 20;
+ AsyncInvocation invocation = client1.invokeAsync(() -> doPuts(entries));
+
+ // Wait for some entries to be put
+ Thread.sleep(5000);
Review comment:
Instead of sleeping (we have a goal of not sleeping in tests), run a
task on server1 that returns the number of entries stored on it. Then the test
could wait until this task returns at least some expected number of entries.
The best practice is to use GeodeAwaitility instead of sleep and you can
find lots of tests that use it for examples.
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/UpdatePropagationDUnitTest.java
##########
@@ -91,30 +100,63 @@ public final void postSetUp() throws Exception {
// Server2 VM
server2 = host.getVM(1);
+ // Server3 VM
+ server3 = host.getVM(2);
+
// Client 1 VM
- client1 = host.getVM(2);
+ client1 = host.getVM(3);
// client 2 VM
- client2 = host.getVM(3);
+ client2 = host.getVM(4);
- PORT1 = server1.invoke(this::createServerCache);
- PORT2 = server2.invoke(this::createServerCache);
+ PORT1 = server1.invoke(() -> createServerCache());
+ PORT2 = server2.invoke(() -> createServerCache());
+ PORT3 = server3.invoke(() -> createServerCache());
- client1.invoke(
- () ->
createClientCache(NetworkUtils.getServerHostName(server1.getHost()), PORT1,
PORT2));
- client2.invoke(
- () ->
createClientCache(NetworkUtils.getServerHostName(server1.getHost()), PORT1,
PORT2));
+ hostnameServer1 = NetworkUtils.getServerHostName(server1.getHost());
+ hostnameServer3 = NetworkUtils.getServerHostName(server3.getHost());
IgnoredException.addIgnoredException("java.net.SocketException");
IgnoredException.addIgnoredException("Unexpected IOException");
}
+
+
+ @Test
+ public void updatesArePropagatedToAllMembersWhenOneKilled() throws Exception
{
Review comment:
The way this test is written, it gives the impression that it is
validating the size on the clients. If you ran a task directly on server1 and
server3 instead client1 and client2 I think it would be more clear. It seems
like you could just call Region.size() on each server.
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/UpdatePropagationDUnitTest.java
##########
@@ -305,6 +360,32 @@ private void verifyUpdates() {
});
}
+ private void doPuts(int entries) throws Exception {
+ Region r1 = getCache().getRegion(REGION_NAME);
+ assertNotNull(r1);
+ for (int i = 0; i < entries; i++) {
+ try {
+ r1.put("" + i, "" + i);
+ } catch (Exception e) {
+ // ignore
Review comment:
It doesn't seem like you should see any exceptions from put since your
client put is being sent to server1 which never goes down. So it seems like you
should remove this try/catch and just throw any unexpected exception to the
caller of doPuts. Do you see exceptions here?
##########
File path:
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/tier/sockets/UpdatePropagationDUnitTest.java
##########
@@ -305,6 +360,32 @@ private void verifyUpdates() {
});
}
+ private void doPuts(int entries) throws Exception {
+ Region r1 = getCache().getRegion(REGION_NAME);
+ assertNotNull(r1);
+ for (int i = 0; i < entries; i++) {
+ try {
+ r1.put("" + i, "" + i);
+ } catch (Exception e) {
+ // ignore
+ }
+ Thread.sleep(1000);
Review comment:
don't sleep in tests.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
