gemmellr commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1182744430
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java:
##########
@@ -664,33 +669,94 @@ private void failoverOrReconnect(final Object
connectionID,
sessionsToFailover = new HashSet<>(sessions);
}
+ // Notify sessions before failover.
for (ClientSessionInternal session : sessionsToFailover) {
session.preHandleFailover(connection);
}
- boolean allSessionReconnected = false;
- int failedReconnectSessionsCounter = 0;
- do {
- allSessionReconnected =
reconnectSessions(sessionsToFailover, oldConnection, reconnectAttempts, me);
- if (oldConnection != null) {
- oldConnection.destroy();
+
+ // Try to reconnect to the current connector pair.
+ // Before ARTEMIS-4251 ClientSessionFactoryImpl only tries to
reconnect to the current connector pair.
+ int reconnectRetries = 0;
+ boolean sessionsReconnected = false;
+ BiPredicate<Boolean, Integer> reconnectRetryPredicate =
+ (reconnected, retries) -> clientProtocolManager.isAlive() &&
+ !reconnected && (reconnectAttempts == -1 || retries <
reconnectAttempts);
+ while (reconnectRetryPredicate.test(sessionsReconnected,
reconnectRetries)) {
+
+ int remainingReconnectRetries = reconnectAttempts == -1 ? -1
: reconnectAttempts - reconnectRetries;
+ reconnectRetries +=
getConnectionWithRetry(remainingReconnectRetries, oldConnection);
+
+ if (connection != null) {
+ sessionsReconnected =
reconnectSessions(sessionsToFailover, oldConnection, me);
+
+ if (!sessionsReconnected) {
+ if (oldConnection != null) {
+ oldConnection.destroy();
+ }
+
+ oldConnection = connection;
+ connection = null;
+ }
+ }
+
+ reconnectRetries++;
+ if (reconnectRetryPredicate.test(sessionsReconnected,
reconnectRetries)) {
+ waitForRetry(retryInterval);
}
+ }
- if (!allSessionReconnected) {
- failedReconnectSessionsCounter++;
- oldConnection = connection;
- connection = null;
- // Wait for retry when the connection is established but
not all session are reconnected.
- if ((reconnectAttempts == -1 ||
failedReconnectSessionsCounter < reconnectAttempts) && oldConnection != null) {
+ // Try to connect to other connector pairs.
+ // After ARTEMIS-4251 ClientSessionFactoryImpl tries to connect
to
+ // other connector pairs when reconnection o the current
connector pair fails.
+ int failoverReties = 0;
Review Comment:
Reties = Retries
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java:
##########
@@ -664,33 +669,94 @@ private void failoverOrReconnect(final Object
connectionID,
sessionsToFailover = new HashSet<>(sessions);
}
+ // Notify sessions before failover.
for (ClientSessionInternal session : sessionsToFailover) {
session.preHandleFailover(connection);
}
- boolean allSessionReconnected = false;
- int failedReconnectSessionsCounter = 0;
- do {
- allSessionReconnected =
reconnectSessions(sessionsToFailover, oldConnection, reconnectAttempts, me);
- if (oldConnection != null) {
- oldConnection.destroy();
+
+ // Try to reconnect to the current connector pair.
+ // Before ARTEMIS-4251 ClientSessionFactoryImpl only tries to
reconnect to the current connector pair.
+ int reconnectRetries = 0;
+ boolean sessionsReconnected = false;
+ BiPredicate<Boolean, Integer> reconnectRetryPredicate =
+ (reconnected, retries) -> clientProtocolManager.isAlive() &&
+ !reconnected && (reconnectAttempts == -1 || retries <
reconnectAttempts);
+ while (reconnectRetryPredicate.test(sessionsReconnected,
reconnectRetries)) {
+
+ int remainingReconnectRetries = reconnectAttempts == -1 ? -1
: reconnectAttempts - reconnectRetries;
+ reconnectRetries +=
getConnectionWithRetry(remainingReconnectRetries, oldConnection);
+
+ if (connection != null) {
+ sessionsReconnected =
reconnectSessions(sessionsToFailover, oldConnection, me);
+
+ if (!sessionsReconnected) {
+ if (oldConnection != null) {
+ oldConnection.destroy();
+ }
+
+ oldConnection = connection;
+ connection = null;
+ }
+ }
+
+ reconnectRetries++;
+ if (reconnectRetryPredicate.test(sessionsReconnected,
reconnectRetries)) {
+ waitForRetry(retryInterval);
}
+ }
- if (!allSessionReconnected) {
- failedReconnectSessionsCounter++;
- oldConnection = connection;
- connection = null;
- // Wait for retry when the connection is established but
not all session are reconnected.
- if ((reconnectAttempts == -1 ||
failedReconnectSessionsCounter < reconnectAttempts) && oldConnection != null) {
+ // Try to connect to other connector pairs.
+ // After ARTEMIS-4251 ClientSessionFactoryImpl tries to connect
to
+ // other connector pairs when reconnection o the current
connector pair fails.
+ int failoverReties = 0;
+ int connectorsCount = 0;
+ Pair<TransportConfiguration, TransportConfiguration>
connectorPair;
+ BiPredicate<Boolean, Integer> failoverRetryPredicate =
+ (reconnected, retries) -> clientProtocolManager.isAlive() &&
+ !reconnected && (failoverAttempts == -1 || retries <
failoverAttempts);
+ while (failoverRetryPredicate.test(sessionsReconnected,
failoverReties)) {
+
+ connectorsCount++;
+ connectorPair = serverLocator.selectNextConnectorPair();
+
+ if (connectorPair != null) {
+ connectorConfig = connectorPair.getA();
+ currentConnectorConfig = connectorPair.getA();
+ if (connectorPair.getB() != null) {
+ backupConnectorConfig = connectorPair.getB();
+ }
+
+ getConnection();
+ }
+
+ if (connection != null) {
+ sessionsReconnected =
reconnectSessions(sessionsToFailover, oldConnection, me);
+
+ if (!sessionsReconnected) {
+ if (oldConnection != null) {
+ oldConnection.destroy();
+ }
+
+ oldConnection = connection;
+ connection = null;
+ }
+ }
+
+ if (connectorsCount >= serverLocator.getConnectorsSize()) {
+ connectorsCount = 0;
+ failoverReties++;
+ if (failoverRetryPredicate.test(false, failoverReties)) {
Review Comment:
Since this overall 'failover connect block' doesnt use
*getConnectionWithRetry* like the earlier 'reconnect connect block' does, that
looks to mean it also doesnt use the retryInterval scaling/max within
*getConnectionWithRetry*, instead only using the base retryInterval config
value during its waits (on the line below this). I didnt see anything obvious
skimming the doc...should probably either document it doesnt use the
scaling/max, or make it do so.
--
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]
