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 2020/10/26 14:25:38 UTC
[GitHub] [geode] albertogpz opened a new pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
albertogpz opened a new pull request #5670:
URL: https://github.com/apache/geode/pull/5670
…eceivers share the same hostname-for-senders
When serveral receivers have the same hostname-for-senders only one of them received
pings.
The reason was that the structure holding the endpoints was a Map whose key was
the server location. As all the endpoints for the remote gateway senders shared
the same server location, only one received the ping.
The structure holding endpoints in the endpoint manager has been updated to use
as key a ServerLocationAndMemberId class instead of ServerLocation.
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mkevo merged pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
mkevo merged pull request #5670:
URL: https://github.com/apache/geode/pull/5670
----------------------------------------------------------------
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] kohlmu-pivotal commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
kohlmu-pivotal commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-728880748
@albertogpz, I'm truly not the best SME to evaluate this PR. I have requested (and added) someone onto this PR, who will hopefully review this PR for correctness
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-730321103
> Nice work Alberto. I appreciate that you persevered and added a Dockerized test that exhibits your achievements.
>
> I wonder if you'd be willing to write something up to contribute to Geode documentation about this use case. Other people could benefit from knowing about this option.
>
> I do have a couple of minor changes that I'd like to see in the new acceptance test.
Thanks @bschuchardt . I agree it would be good to write something in the documentation about this use case. I assume you are talking about WAN deployments in which the topology of the Geode cluster (the servers) is hidden behind a single IP address. Am I correct?
If so, I will look for a place in the Geode documentation to add to it. I think it will be best to do it in a new PR.
----------------------------------------------------------------
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] Bill commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
Bill commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-720610506
@albertogpz if you want to use Docker in your tests, yeah, the easiest way to do that right now is to make your test an "acceptanceTest" which means putting it in an `/geode-assembly/src/acceptanceTest` hierarchy.
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-717039272
> Hi there @albertogpz.
> Thank you for this contribution, but would it be possible to create a DUnit test to test out the functionality that you are providing.
>
> DUnit tests, is our current way to spin up multiple Servers in different VM's in order to test out multi-server functionality.
Hi @kohlmu-pivotal .
I thought about creating a DUnit test for this change but the problem I ran into is that it would require to have a load balancer that balances connections between the servers that share the same hostname-for-senders value.
Any idea how to implement this with the current framework?
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-717506887
> @albertogpz, hostname-for-senders is a string value which represents the hostname that is given to the senders. This means, that if you start 2 (or more servers) on the same machine they can have the same value. Could you create two servers with a gateway receiver each, with the same hostname-for-server value and see if this solves the problem.
>
> If not, we have to seriously put our heads together and try and come up with a solution to the problem. As I'd prefer a test to be in place that reproduce the issue and then of course fix it. Might even mean that we might have to create a whole new framework to test these kind of issues.
@kohlmu-pivotal, the receivers part is easy, we can create two senders with the same hostname-for-senders value as you suggest.
The difficult part is the sending side. The gateway senders would use the value of hostname-for-senders to connect to the receivers but we should have "something" (the load balancer in the real world) that should intercept the connect call to the hostname-for-senders address so that the connection is finally made against the ip addresses to which the receivers binded.
I do not know if we can tweak in the test case the part of the creation of the connection in the gateway sender so that when a connection to an endpoint is created, the IP address to be used is changed to one of the IP addresses on which the receivers are listening.
I will try to check if this is possible.
Any ideas are welcome.
Thanks!
----------------------------------------------------------------
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] kohlmu-pivotal commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
kohlmu-pivotal commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-717471100
@albertogpz, hostname-for-senders is a string value which represents the hostname that is given to the senders. This means, that if you start 2 (or more servers) on the same machine they can have the same value. Could you create two servers with a gateway receiver each, with the same hostname-for-server value and see if this solves the problem.
If not, we have to seriously put our heads together and try and come up with a solution to the problem. As I'd prefer a test to be in place that reproduce the issue and then of course fix it. Might even mean that we might have to create a whole new framework to test these kind of issues.
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-721133690
> > @albertogpz if you want to use Docker in your tests, yeah, the easiest way to do that right now is to make your test an "acceptanceTest" which means putting it in an `/geode-assembly/src/acceptanceTest` hierarchy.
>
> Thanks, @Bill . How hard would it be to use docker-compose in a distributedTest? I am saying it because I used in my test the org.apache.geode.test.dunit.VM class. If I move it to be an acceptanceTest I would have to implement it in a different way. It looks to me that in acceptanceTest, Geode processes are started with gfsh. Any suggestion?
I finally created the test as acceptanceTest.
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-717537364
> @albertogpz some engineers pointed me at `DualServerSNIAcceptanceTest` to possibly be a template to help with testing this.
> I will admit I'm not across anything done in this test, maybe @Bill could provide some guidance if you have more questions
@kohlmu-pivotal Great, thanks! I will take a look at that test and if @Bill wants to comment, he is more than welcome.
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-728901172
> @albertogpz, I'm truly not the best SME to evaluate this PR. I have requested (and added) someone onto this PR, who will hopefully review this PR for correctness
Thanks @kohlmu-pivotal . If you could add @bschuchardt it would probably be best as he reviewed the previous tickets related to this issue.
----------------------------------------------------------------
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] albertogpz commented on a change in pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on a change in pull request #5670:
URL: https://github.com/apache/geode/pull/5670#discussion_r526639463
##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSenders.java
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.wan;
+
+import static com.palantir.docker.compose.execution.DockerComposeExecArgument.arguments;
+import static com.palantir.docker.compose.execution.DockerComposeExecOption.options;
+import static org.apache.geode.cache.Region.SEPARATOR;
+import static org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.REMOTE_LOCATORS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.StringTokenizer;
+
+import com.palantir.docker.compose.DockerComposeRule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.persistence.PartitionOfflineException;
+import org.apache.geode.client.sni.NotOnWindowsDockerRule;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.internal.cache.ForceReattemptException;
+import org.apache.geode.internal.cache.PoolStats;
+import org.apache.geode.internal.cache.wan.AbstractGatewaySender;
+import org.apache.geode.internal.cache.wan.InternalGatewaySenderFactory;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.categories.WanTest;
+
+
+/**
+ * These tests use two Geode sites:
+ *
+ * - One site (the remote one) consisting of a 2-server, 1-locator Geode cluster.
+ * The servers host a partition region (region-wan) and have gateway senders to receive events
+ * from the other site with the same value for hostname-for-senders and listening on the
+ * same port (2324).
+ * The servers and locator run each inside a Docker container and are not route-able
+ * from the host (where this JUnit test is running).
+ * Another Docker container is running the HAProxy image and it's set up as a TCP load balancer.
+ * The other site connects to the locator and to the gateway receivers via the
+ * TCP load balancer that forwards traffic directed to the 20334 port to the locator and
+ * traffic directed to the 2324 port to the receivers in a round robin fashion.
+ *
+ * - Another site consisting of a 1-server, 1-locator Geode cluster.
+ * The server hosts a partition region (region-wan) and has a parallel gateway receiver
+ * to send events to the remote site.
+ *
+ * The aim of the tests is verify that when several gateway receivers in a remote site
+ * share the same port and hostname-for-senders, the pings sent from the gateway senders
+ * reach the right gateway receiver and not just any of the receivers. Failure to do this
+ * may result in the closing of connections by a gateway receiver for not having
+ * received the ping in time.
+ */
+@Category({WanTest.class})
+public class SeveralGatewayReceiversWithSamePortAndHostnameForSenders {
+
+ private static final int NUM_SERVERS = 2;
+
+ private static Cache cache;
+
+ private static final URL DOCKER_COMPOSE_PATH =
+ SeveralGatewayReceiversWithSamePortAndHostnameForSenders.class
+ .getResource("docker-compose.yml");
+
+ // Docker compose does not work on windows in CI. Ignore this test on windows
+ // Using a RuleChain to make sure we ignore the test before the rule comes into play
+ @ClassRule
+ public static NotOnWindowsDockerRule docker =
+ new NotOnWindowsDockerRule(() -> DockerComposeRule.builder()
+ .file(DOCKER_COMPOSE_PATH.getPath()).build());
+
+ @Rule
+ public DistributedRule distributedRule =
+ DistributedRule.builder().withVMCount(NUM_SERVERS + 1).build();
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Start locator
+ docker.get().exec(options("-T"), "locator",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-locator.gfsh"));
+ // Start server1
+ docker.get().exec(options("-T"), "server1",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-server1.gfsh"));
+ // Start server2
+ docker.get().exec(options("-T"), "server2",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-server2.gfsh"));
+ // Create partition region and gateway receiver
+ docker.get().exec(options("-T"), "locator",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-create.gfsh"));
+ }
+
+ public SeveralGatewayReceiversWithSamePortAndHostnameForSenders() {
+ super();
+ }
+
+ @Test
+ public void testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceivers() {
+ String senderId = "ln";
+ String regionName = "region-wan";
+ final int remoteLocPort = 20334;
+
+ int locPort = createLocator(VM.getVM(0), 1, remoteLocPort);
+
+ VM vm1 = VM.getVM(1);
+ createCache(vm1, locPort);
+
+ // We must use more than one dispatcher thread. With just one dispatcher thread, only one
+ // connection will be created by the sender towards one of the receivers and it will be
+ // monitored by the one ping thread for that remote receiver.
+ // With more than one thread, several connections will be opened and there should be one ping
+ // thread per remote receiver.
+ createGatewaySender(vm1, senderId, 2, true, 5,
+ 5, GatewaySender.DEFAULT_ORDER_POLICY);
+
+ createPartitionedRegion(vm1, regionName, senderId, 0, 10);
+
+ int NUM_PUTS = 10;
+
+ putKeyValues(vm1, NUM_PUTS, regionName);
+
+ // Wait longer than the value set in the receivers for
+ // maximum-time-between-pings (60000) to verify that connections are not closed
+ // by the receivers because each has received the pings timely.
+ int maxTimeBetweenPingsInReceiver = 65000;
Review comment:
It was not possible before the fix for #5701 but as that is now in place, I have been able to reduce the max to 15 secs.
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-720564213
----------------------------------------------------------------
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] albertogpz commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
albertogpz commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-731064938
> > Nice work Alberto. I appreciate that you persevered and added a Dockerized test that exhibits your achievements.
> > I wonder if you'd be willing to write something up to contribute to Geode documentation about this use case. Other people could benefit from knowing about this option.
> > I do have a couple of minor changes that I'd like to see in the new acceptance test.
>
> Thanks @bschuchardt . I agree it would be good to write something in the documentation about this use case. I assume you are talking about WAN deployments in which the topology of the Geode cluster (the servers) is hidden behind a single IP address. Am I correct?
>
> If so, I will look for a place in the Geode documentation to add to it. I think it will be best to do it in a new PR.
@bschuchardt I have created a new ticket GEODE-8738 and a PR #5766 with the information you suggested.
----------------------------------------------------------------
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] kohlmu-pivotal commented on pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
kohlmu-pivotal commented on pull request #5670:
URL: https://github.com/apache/geode/pull/5670#issuecomment-717534797
@albertogpz some engineers pointed me at `DualServerSNIAcceptanceTest` to possibly be a template to help with testing this.
I will admit I'm not across anything done in this test, maybe @Bill could provide some guidance if you have more questions
----------------------------------------------------------------
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] bschuchardt commented on a change in pull request #5670: GEODE-8656: Fix ping only sent to one gateway receiver when several r…
Posted by GitBox <gi...@apache.org>.
bschuchardt commented on a change in pull request #5670:
URL: https://github.com/apache/geode/pull/5670#discussion_r525594024
##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSenders.java
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.wan;
+
+import static com.palantir.docker.compose.execution.DockerComposeExecArgument.arguments;
+import static com.palantir.docker.compose.execution.DockerComposeExecOption.options;
+import static org.apache.geode.cache.Region.SEPARATOR;
+import static org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.REMOTE_LOCATORS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.StringTokenizer;
+
+import com.palantir.docker.compose.DockerComposeRule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.persistence.PartitionOfflineException;
+import org.apache.geode.client.sni.NotOnWindowsDockerRule;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.internal.cache.ForceReattemptException;
+import org.apache.geode.internal.cache.PoolStats;
+import org.apache.geode.internal.cache.wan.AbstractGatewaySender;
+import org.apache.geode.internal.cache.wan.InternalGatewaySenderFactory;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.categories.WanTest;
+
+
+/**
+ * These tests use two Geode sites:
+ *
+ * - One site (the remote one) consisting of a 2-server, 1-locator Geode cluster.
+ * The servers host a partition region (region-wan) and have gateway senders to receive events
+ * from the other site with the same value for hostname-for-senders and listening on the
+ * same port (2324).
+ * The servers and locator run each inside a Docker container and are not route-able
+ * from the host (where this JUnit test is running).
+ * Another Docker container is running the HAProxy image and it's set up as a TCP load balancer.
+ * The other site connects to the locator and to the gateway receivers via the
+ * TCP load balancer that forwards traffic directed to the 20334 port to the locator and
+ * traffic directed to the 2324 port to the receivers in a round robin fashion.
+ *
+ * - Another site consisting of a 1-server, 1-locator Geode cluster.
+ * The server hosts a partition region (region-wan) and has a parallel gateway receiver
+ * to send events to the remote site.
+ *
+ * The aim of the tests is verify that when several gateway receivers in a remote site
+ * share the same port and hostname-for-senders, the pings sent from the gateway senders
+ * reach the right gateway receiver and not just any of the receivers. Failure to do this
+ * may result in the closing of connections by a gateway receiver for not having
+ * received the ping in time.
+ */
+@Category({WanTest.class})
+public class SeveralGatewayReceiversWithSamePortAndHostnameForSenders {
+
+ private static final int NUM_SERVERS = 2;
+
+ private static Cache cache;
+
+ private static final URL DOCKER_COMPOSE_PATH =
+ SeveralGatewayReceiversWithSamePortAndHostnameForSenders.class
+ .getResource("docker-compose.yml");
+
+ // Docker compose does not work on windows in CI. Ignore this test on windows
+ // Using a RuleChain to make sure we ignore the test before the rule comes into play
+ @ClassRule
+ public static NotOnWindowsDockerRule docker =
+ new NotOnWindowsDockerRule(() -> DockerComposeRule.builder()
+ .file(DOCKER_COMPOSE_PATH.getPath()).build());
+
+ @Rule
+ public DistributedRule distributedRule =
+ DistributedRule.builder().withVMCount(NUM_SERVERS + 1).build();
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Start locator
+ docker.get().exec(options("-T"), "locator",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-locator.gfsh"));
+ // Start server1
+ docker.get().exec(options("-T"), "server1",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-server1.gfsh"));
+ // Start server2
+ docker.get().exec(options("-T"), "server2",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-server2.gfsh"));
+ // Create partition region and gateway receiver
+ docker.get().exec(options("-T"), "locator",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-create.gfsh"));
+ }
+
+ public SeveralGatewayReceiversWithSamePortAndHostnameForSenders() {
+ super();
+ }
+
+ @Test
+ public void testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceivers() {
+ String senderId = "ln";
+ String regionName = "region-wan";
+ final int remoteLocPort = 20334;
+
+ int locPort = createLocator(VM.getVM(0), 1, remoteLocPort);
+
+ VM vm1 = VM.getVM(1);
+ createCache(vm1, locPort);
+
+ // We must use more than one dispatcher thread. With just one dispatcher thread, only one
+ // connection will be created by the sender towards one of the receivers and it will be
+ // monitored by the one ping thread for that remote receiver.
+ // With more than one thread, several connections will be opened and there should be one ping
+ // thread per remote receiver.
+ createGatewaySender(vm1, senderId, 2, true, 5,
+ 5, GatewaySender.DEFAULT_ORDER_POLICY);
+
+ createPartitionedRegion(vm1, regionName, senderId, 0, 10);
+
+ int NUM_PUTS = 10;
+
+ putKeyValues(vm1, NUM_PUTS, regionName);
+
+ // Wait longer than the value set in the receivers for
+ // maximum-time-between-pings (60000) to verify that connections are not closed
+ // by the receivers because each has received the pings timely.
+ int maxTimeBetweenPingsInReceiver = 65000;
+ try {
+ Thread.sleep(maxTimeBetweenPingsInReceiver);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
Review comment:
While Intellij Idea likes to generate this kind of code it isn't useful when there's a failure.
Get rid of the try/catch and add a "throws" to the test method instead.
##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSenders.java
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.wan;
+
+import static com.palantir.docker.compose.execution.DockerComposeExecArgument.arguments;
+import static com.palantir.docker.compose.execution.DockerComposeExecOption.options;
+import static org.apache.geode.cache.Region.SEPARATOR;
+import static org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.REMOTE_LOCATORS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.StringTokenizer;
+
+import com.palantir.docker.compose.DockerComposeRule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.persistence.PartitionOfflineException;
+import org.apache.geode.client.sni.NotOnWindowsDockerRule;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.internal.cache.ForceReattemptException;
+import org.apache.geode.internal.cache.PoolStats;
+import org.apache.geode.internal.cache.wan.AbstractGatewaySender;
+import org.apache.geode.internal.cache.wan.InternalGatewaySenderFactory;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.categories.WanTest;
+
+
+/**
+ * These tests use two Geode sites:
+ *
+ * - One site (the remote one) consisting of a 2-server, 1-locator Geode cluster.
+ * The servers host a partition region (region-wan) and have gateway senders to receive events
+ * from the other site with the same value for hostname-for-senders and listening on the
+ * same port (2324).
+ * The servers and locator run each inside a Docker container and are not route-able
+ * from the host (where this JUnit test is running).
+ * Another Docker container is running the HAProxy image and it's set up as a TCP load balancer.
+ * The other site connects to the locator and to the gateway receivers via the
+ * TCP load balancer that forwards traffic directed to the 20334 port to the locator and
+ * traffic directed to the 2324 port to the receivers in a round robin fashion.
+ *
+ * - Another site consisting of a 1-server, 1-locator Geode cluster.
+ * The server hosts a partition region (region-wan) and has a parallel gateway receiver
+ * to send events to the remote site.
+ *
+ * The aim of the tests is verify that when several gateway receivers in a remote site
+ * share the same port and hostname-for-senders, the pings sent from the gateway senders
+ * reach the right gateway receiver and not just any of the receivers. Failure to do this
+ * may result in the closing of connections by a gateway receiver for not having
+ * received the ping in time.
+ */
+@Category({WanTest.class})
+public class SeveralGatewayReceiversWithSamePortAndHostnameForSenders {
Review comment:
I don't know if it's a rule or not but the names of literally all of the other unit test classes in the Geode repository end with "Test", so maybe this could be SeveralGatewayReceiversWithSamePortAndHostnameForSenders**Test**.
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/internal/LiveServerPinger.java
##########
@@ -87,6 +87,7 @@ private void cancelFuture(Endpoint endpoint) {
public void run2() {
if (endpoint.timeToPing(pingIntervalNanos)) {
try {
+ endpoint.updateLastExecute();
Review comment:
(just a comment) I see that you've moved the updateLastExecute() for PingOp to LiveServerPinger.
##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSenders.java
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.cache.wan;
+
+import static com.palantir.docker.compose.execution.DockerComposeExecArgument.arguments;
+import static com.palantir.docker.compose.execution.DockerComposeExecOption.options;
+import static org.apache.geode.cache.Region.SEPARATOR;
+import static org.apache.geode.distributed.ConfigurationProperties.DISTRIBUTED_SYSTEM_ID;
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.REMOTE_LOCATORS;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.io.File;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.StringTokenizer;
+
+import com.palantir.docker.compose.DockerComposeRule;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.persistence.PartitionOfflineException;
+import org.apache.geode.client.sni.NotOnWindowsDockerRule;
+import org.apache.geode.distributed.Locator;
+import org.apache.geode.internal.cache.ForceReattemptException;
+import org.apache.geode.internal.cache.PoolStats;
+import org.apache.geode.internal.cache.wan.AbstractGatewaySender;
+import org.apache.geode.internal.cache.wan.InternalGatewaySenderFactory;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.categories.WanTest;
+
+
+/**
+ * These tests use two Geode sites:
+ *
+ * - One site (the remote one) consisting of a 2-server, 1-locator Geode cluster.
+ * The servers host a partition region (region-wan) and have gateway senders to receive events
+ * from the other site with the same value for hostname-for-senders and listening on the
+ * same port (2324).
+ * The servers and locator run each inside a Docker container and are not route-able
+ * from the host (where this JUnit test is running).
+ * Another Docker container is running the HAProxy image and it's set up as a TCP load balancer.
+ * The other site connects to the locator and to the gateway receivers via the
+ * TCP load balancer that forwards traffic directed to the 20334 port to the locator and
+ * traffic directed to the 2324 port to the receivers in a round robin fashion.
+ *
+ * - Another site consisting of a 1-server, 1-locator Geode cluster.
+ * The server hosts a partition region (region-wan) and has a parallel gateway receiver
+ * to send events to the remote site.
+ *
+ * The aim of the tests is verify that when several gateway receivers in a remote site
+ * share the same port and hostname-for-senders, the pings sent from the gateway senders
+ * reach the right gateway receiver and not just any of the receivers. Failure to do this
+ * may result in the closing of connections by a gateway receiver for not having
+ * received the ping in time.
+ */
+@Category({WanTest.class})
+public class SeveralGatewayReceiversWithSamePortAndHostnameForSenders {
+
+ private static final int NUM_SERVERS = 2;
+
+ private static Cache cache;
+
+ private static final URL DOCKER_COMPOSE_PATH =
+ SeveralGatewayReceiversWithSamePortAndHostnameForSenders.class
+ .getResource("docker-compose.yml");
+
+ // Docker compose does not work on windows in CI. Ignore this test on windows
+ // Using a RuleChain to make sure we ignore the test before the rule comes into play
+ @ClassRule
+ public static NotOnWindowsDockerRule docker =
+ new NotOnWindowsDockerRule(() -> DockerComposeRule.builder()
+ .file(DOCKER_COMPOSE_PATH.getPath()).build());
+
+ @Rule
+ public DistributedRule distributedRule =
+ DistributedRule.builder().withVMCount(NUM_SERVERS + 1).build();
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Start locator
+ docker.get().exec(options("-T"), "locator",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-locator.gfsh"));
+ // Start server1
+ docker.get().exec(options("-T"), "server1",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-server1.gfsh"));
+ // Start server2
+ docker.get().exec(options("-T"), "server2",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-server2.gfsh"));
+ // Create partition region and gateway receiver
+ docker.get().exec(options("-T"), "locator",
+ arguments("gfsh", "run", "--file=/geode/scripts/geode-starter-create.gfsh"));
+ }
+
+ public SeveralGatewayReceiversWithSamePortAndHostnameForSenders() {
+ super();
+ }
+
+ @Test
+ public void testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceivers() {
+ String senderId = "ln";
+ String regionName = "region-wan";
+ final int remoteLocPort = 20334;
+
+ int locPort = createLocator(VM.getVM(0), 1, remoteLocPort);
+
+ VM vm1 = VM.getVM(1);
+ createCache(vm1, locPort);
+
+ // We must use more than one dispatcher thread. With just one dispatcher thread, only one
+ // connection will be created by the sender towards one of the receivers and it will be
+ // monitored by the one ping thread for that remote receiver.
+ // With more than one thread, several connections will be opened and there should be one ping
+ // thread per remote receiver.
+ createGatewaySender(vm1, senderId, 2, true, 5,
+ 5, GatewaySender.DEFAULT_ORDER_POLICY);
+
+ createPartitionedRegion(vm1, regionName, senderId, 0, 10);
+
+ int NUM_PUTS = 10;
+
+ putKeyValues(vm1, NUM_PUTS, regionName);
+
+ // Wait longer than the value set in the receivers for
+ // maximum-time-between-pings (60000) to verify that connections are not closed
+ // by the receivers because each has received the pings timely.
+ int maxTimeBetweenPingsInReceiver = 65000;
Review comment:
can the max be shortened so that this test doesn't take so long to run?
----------------------------------------------------------------
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