albertogpz commented on a change in pull request #7347:
URL: https://github.com/apache/geode/pull/7347#discussion_r836644349
##########
File path:
geode-assembly/src/acceptanceTest/java/org/apache/geode/cache/wan/SeveralGatewayReceiversWithSamePortAndHostnameForSendersTest.java
##########
@@ -215,6 +216,55 @@ public void
testTwoSendersWithSameIdShouldUseSameValueForEnforceThreadsConnectTo
}
+
+
+ /**
+ * The aim of this test 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.
+ */
+ @Test
+ public void
testPingsToReceiversWithSamePortAndHostnameForSendersReachTheRightReceiver()
Review comment:
The description of this test case is exactly the same as the one of a
test above and the name only changes by the final s.
Also the contents are the same with the exception of the last check.
I propose that the check of this test is moved to the first test case and
that the description of that test is updated with what is intended by the check
in this test.
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionImpl.java
##########
@@ -117,6 +118,47 @@ public ServerQueueStatus connect(EndpointManager
endpointManager, ServerLocation
return status;
}
+ public ServerQueueStatus connect(EndpointManager endpointManager,
Review comment:
There is a lot of code repeated in this method from the previous method.
I suggest it to be refactored in order to avoid it.
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionFactoryImpl.java
##########
@@ -146,6 +147,46 @@ public Connection
createClientToServerConnection(ServerLocation location, boolea
return connection;
}
+
+ @Override
+ public Connection createClientToServerConnection(
Review comment:
There is a lot of code repeated in this method from the previous method.
I suggest it to be refactored in order to avoid it.
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/pooling/ConnectionManager.java
##########
@@ -71,6 +72,29 @@ Connection borrowConnection(ServerLocation server, long
acquireTimeout,
boolean onlyUseExistingCnx)
throws AllConnectionsInUseException, NoAvailableServersException;
+ /**
+ * Borrow an existing idle connection or create a new one to a specific
server. Fails after one
+ * failed attempt to create a new connection. May cause pool to exceed
maxConnections by one, if
+ * no connection is available.
+ *
+ * @param serverLocationAndMemberId The server the connection needs to be to.
+ * @param acquireTimeout The amount of time to wait for a connection to
become available, if
+ * onlyUseExistingCnx is set to true.
+ * @param onlyUseExistingCnx if true, will not create a new connection if
none are available.
+ * @return A connection to use.
+ * @throws AllConnectionsInUseException If there is no available connection
on the desired server,
+ * and onlyUseExistingCnx is set.
+ * @throws NoAvailableServersException If we can't connect to any server
+ * @throws ServerConnectivityException If finding a connection and creating
a connection both fail
+ * to return a connection
+ *
+ */
+ Connection borrowConnection(ServerLocationAndMemberId
serverLocationAndMemberId,
+ long acquireTimeout,
+ boolean onlyUseExistingCnx)
+ throws AllConnectionsInUseException, NoAvailableServersException;
+
+
Review comment:
What does this method do differently compared to the one passing a
server? Do we really need it if it does the same thing?
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/ConnectionConnector.java
##########
@@ -88,6 +89,32 @@ public ConnectionImpl connectClientToServer(ServerLocation
location, boolean for
}
}
+
+ public ConnectionImpl connectClientToServer(ServerLocationAndMemberId
serverLocationAndMemberId,
Review comment:
There is a lot of code repeated in this method. I suggest that you
refactor it in order to avoid it.
##########
File path:
geode-core/src/main/java/org/apache/geode/cache/client/internal/pooling/ConnectionManagerImpl.java
##########
@@ -355,6 +385,54 @@ public Connection borrowConnection(ServerLocation server,
long acquireTimeout,
throw new AllConnectionsInUseException();
}
+
+ @Override
+ public Connection borrowConnection(ServerLocationAndMemberId
serverLocationAndMemberId,
Review comment:
There is a lot of code repeated in this method from the previous method.
I suggest it to be refactored in order to avoid it.
--
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-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
