pivotal-jbarrett commented on code in PR #7381:
URL: https://github.com/apache/geode/pull/7381#discussion_r860247826
##########
geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java:
##########
@@ -909,6 +906,104 @@ public Connection getConnection(InternalDistributedMember
memberAddress,
}
}
+
+ /**
+ * Return a connection to the given member. This method performs quick scan
for connection.
+ * Only one attempt to create a connection to the given member .
+ *
+ * @param memberAddress the IDS associated with the remoteId
+ * @param preserveOrder whether this is an ordered or unordered connection
+ * @param startTime the time this operation started
+ * @param ackTimeout the ack-wait-threshold * 1000 for the operation to be
transmitted (or zero)
+ * @param ackSATimeout the ack-severe-alert-threshold * 1000 for the
operation to be transmitted
+ * (or zero)
+ *
+ * @return the connection
+ */
+ public Connection getFirstScanForConnection(InternalDistributedMember
memberAddress,
+ final boolean preserveOrder, long startTime, long ackTimeout,
+ long ackSATimeout) throws IOException,
DistributedSystemDisconnectedException {
+ if (stopped) {
+ throw new DistributedSystemDisconnectedException("The conduit is
stopped");
+ }
+
+ Connection connection = null;
+ stopper.checkCancelInProgress(null);
+ boolean interrupted = Thread.interrupted();
+ try {
+ // If this is the second time through this loop, we had problems.
Review Comment:
What loop?
##########
geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java:
##########
@@ -909,6 +906,104 @@ public Connection getConnection(InternalDistributedMember
memberAddress,
}
}
+
+ /**
+ * Return a connection to the given member. This method performs quick scan
for connection.
+ * Only one attempt to create a connection to the given member .
+ *
+ * @param memberAddress the IDS associated with the remoteId
+ * @param preserveOrder whether this is an ordered or unordered connection
+ * @param startTime the time this operation started
+ * @param ackTimeout the ack-wait-threshold * 1000 for the operation to be
transmitted (or zero)
+ * @param ackSATimeout the ack-severe-alert-threshold * 1000 for the
operation to be transmitted
+ * (or zero)
+ *
+ * @return the connection
+ */
+ public Connection getFirstScanForConnection(InternalDistributedMember
memberAddress,
+ final boolean preserveOrder, long startTime, long ackTimeout,
+ long ackSATimeout) throws IOException,
DistributedSystemDisconnectedException {
+ if (stopped) {
+ throw new DistributedSystemDisconnectedException("The conduit is
stopped");
+ }
+
+ Connection connection = null;
+ stopper.checkCancelInProgress(null);
+ boolean interrupted = Thread.interrupted();
+ try {
+ // If this is the second time through this loop, we had problems.
+ // Tear down the connection so that it gets rebuilt.
+
+ Exception problem = null;
+ try {
+ // Get (or regenerate) the connection
+ // this could generate a ConnectionException, so it must be caught and
retried
+ boolean retryForOldConnection;
+ boolean debugRetry = false;
+ do {
+ retryForOldConnection = false;
+ connection = getConTable().get(memberAddress, preserveOrder,
startTime, ackTimeout,
+ ackSATimeout, true);
+ if (connection == null) {
+ // conduit may be closed - otherwise an ioexception would be thrown
+ problem = new IOException(
+ String.format("Unable to reconnect to server; possible
shutdown: %s",
+ memberAddress));
+ } else if (connection.isClosing()
+ || !connection.getRemoteAddress().equals(memberAddress)) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Got an old connection for {}: {}@{}",
memberAddress, connection,
+ connection.hashCode());
+ }
+ connection.closeOldConnection("closing old connection");
+ connection = null;
+ retryForOldConnection = true;
Review Comment:
I am rarely a fan of do/while loops and this one tracking this exit value
was initially hard to follow. I think a infinite loop with throw exceptions and
early breaks shortens it and clears up the purpose.
```java
final debugEnabled = logger.isDebugEnabled();
while(true) {
connection = getConTable()...;
if (connection == null) {
throw new IOException(...);
}
if (!(connection.isClosing() || ..)) {
break;
}
if (debugEnabled) {
logger.debug(...);
}
connection.closeOldConnection(...);
}
```
Throwing the exception gets handled below the same as having set the
`problem` variable.
You don't need to reset the `connection` to `null` after it is closed since
the next statement to execute sets it anyway.
I would even go one step further and move this whole loop logic into a
method.
```java
final Connection = getConnectionThatIsNotClosed(...);
```
```java
@NotNull Connection getConnectionThatIsNotClosed(...) {
final debugEnabled = logger.isDebugEnabled();
while(true) {
final Connection connection = getConTable()...;
if (connection == null) {
throw new IOException(...);
}
if (!(connection.isClosing() || ..)) {
return connection;
}
if (debugEnabled) {
logger.debug(...);
}
connection.closeOldConnection(...);
}
}
```
##########
geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java:
##########
@@ -909,6 +906,104 @@ public Connection getConnection(InternalDistributedMember
memberAddress,
}
}
+
+ /**
+ * Return a connection to the given member. This method performs quick scan
for connection.
+ * Only one attempt to create a connection to the given member .
+ *
+ * @param memberAddress the IDS associated with the remoteId
+ * @param preserveOrder whether this is an ordered or unordered connection
+ * @param startTime the time this operation started
+ * @param ackTimeout the ack-wait-threshold * 1000 for the operation to be
transmitted (or zero)
+ * @param ackSATimeout the ack-severe-alert-threshold * 1000 for the
operation to be transmitted
+ * (or zero)
+ *
+ * @return the connection
+ */
+ public Connection getFirstScanForConnection(InternalDistributedMember
memberAddress,
+ final boolean preserveOrder, long startTime, long ackTimeout,
+ long ackSATimeout) throws IOException,
DistributedSystemDisconnectedException {
+ if (stopped) {
+ throw new DistributedSystemDisconnectedException("The conduit is
stopped");
+ }
+
+ Connection connection = null;
+ stopper.checkCancelInProgress(null);
+ boolean interrupted = Thread.interrupted();
+ try {
+ // If this is the second time through this loop, we had problems.
+ // Tear down the connection so that it gets rebuilt.
+
+ Exception problem = null;
+ try {
+ // Get (or regenerate) the connection
+ // this could generate a ConnectionException, so it must be caught and
retried
+ boolean retryForOldConnection;
+ boolean debugRetry = false;
+ do {
+ retryForOldConnection = false;
+ connection = getConTable().get(memberAddress, preserveOrder,
startTime, ackTimeout,
+ ackSATimeout, true);
+ if (connection == null) {
+ // conduit may be closed - otherwise an ioexception would be thrown
+ problem = new IOException(
+ String.format("Unable to reconnect to server; possible
shutdown: %s",
+ memberAddress));
+ } else if (connection.isClosing()
+ || !connection.getRemoteAddress().equals(memberAddress)) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Got an old connection for {}: {}@{}",
memberAddress, connection,
+ connection.hashCode());
+ }
+ connection.closeOldConnection("closing old connection");
+ connection = null;
+ retryForOldConnection = true;
+ debugRetry = true;
+ }
+ } while (retryForOldConnection);
+ if (debugRetry && logger.isDebugEnabled()) {
Review Comment:
Does this log message do anything for us?
--
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]
