You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "brusdev (via GitHub)" <gi...@apache.org> on 2023/04/24 07:56:17 UTC

[GitHub] [activemq-artemis] brusdev opened a new pull request, #4447: ARTEMIS-4251 Support CORE client failover to other live servers

brusdev opened a new pull request, #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447

   Improve the CORE client failover connecting to other live servers when all reconnect attempts fails, i.e. in a cluster composed of 2 live servers, when the server to which the CORE client is connected goes down the CORE client should reconnect its sessions to the other liver broker.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#issuecomment-1562596505

   @clebertsuconic branch rabased


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] gtully commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "gtully (via GitHub)" <gi...@apache.org>.
gtully commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1176456963


##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java:
##########
@@ -653,6 +653,21 @@ ClientSessionFactory createSessionFactory(TransportConfiguration transportConfig
     */
    int getInitialConnectAttempts();
 
+   /**
+    * Sets the maximum number of attempts to establish a connection after a failed reconnection.

Review Comment:
   should that be "failed connection" rather than "failed reconnection"



##########
docs/user-manual/en/client-reconnection.md:
##########
@@ -89,9 +89,20 @@ Client reconnection is configured using the following parameters:
   ridiculously large values. By setting this parameter you can set an upper limit
   on that value. The default value is `2000` milliseconds.
 
+- `ha`. This optional parameter determines weather the client will try to
+  reconnect to the backup node when the live node is not reachable.
+  The default value is `false`.
+  For more information on HA, please see [High Availability and Failover](ha.md).
+
 - `reconnectAttempts`. This optional parameter determines the total number of
-  reconnect attempts to make before giving up and shutting down. A value of
-  `-1` signifies an unlimited number of attempts. The default value is `0`.
+  reconnect attempts to make to the current live/backup pair before giving up.

Review Comment:
   I am confused :-)
   
   I thought failover was between live and backup
   
   reconnect should first try to failover, then fall back to any connection to eventually reconnect. these seems the opposite.



##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java:
##########
@@ -653,6 +653,21 @@ ClientSessionFactory createSessionFactory(TransportConfiguration transportConfig
     */
    int getInitialConnectAttempts();
 
+   /**
+    * Sets the maximum number of attempts to establish a connection after a failed reconnection.

Review Comment:
   maximum number of attempts to reconnect after a failure. But I may have this wrong!



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "AntonRoskvist (via GitHub)" <gi...@apache.org>.
AntonRoskvist commented on PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#issuecomment-1520145971

   Really nice addition.
   
   Haven't really looked into how it can be implemented, but there is a feature in "ActiveMQ Classic" that I think would be a great addition on top of this change. It's called "priorityBackup" ( https://activemq.apache.org/failover-transport-reference.html#priority-backup )
   
   It has the client perform this type of failover to carry on its work, but then afterwards tries to continuously connect back to the original broker, releasing the new connection if/when it succeeds. This is good for handling client distribution across a broker cluster that sort of "persists" through maintenance, crashes, network outages etc on the broker side.
   
   Either way, this is a really nice addition as is!


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1178713896


##########
docs/user-manual/en/client-reconnection.md:
##########
@@ -89,9 +89,20 @@ Client reconnection is configured using the following parameters:
   ridiculously large values. By setting this parameter you can set an upper limit
   on that value. The default value is `2000` milliseconds.
 
+- `ha`. This optional parameter determines weather the client will try to
+  reconnect to the backup node when the live node is not reachable.
+  The default value is `false`.
+  For more information on HA, please see [High Availability and Failover](ha.md).
+
 - `reconnectAttempts`. This optional parameter determines the total number of
-  reconnect attempts to make before giving up and shutting down. A value of
-  `-1` signifies an unlimited number of attempts. The default value is `0`.
+  reconnect attempts to make to the current live/backup pair before giving up.

Review Comment:
   @gtully @gemmellr thanks for your feedback, I'll review the doc and I'll move the pause after trying all available connectors.



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1183650110


##########
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:
   Use the exponential back-off algorithm also for `failover connect block` sounds good to me



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1182305228


##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java:
##########
@@ -653,6 +653,21 @@ ClientSessionFactory createSessionFactory(TransportConfiguration transportConfig
     */
    int getInitialConnectAttempts();
 
+   /**
+    * Sets the maximum number of attempts to establish a connection after a failed reconnection.

Review Comment:
   done, thanks for the suggestion



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] gemmellr commented on pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#issuecomment-1533154934

   Seems good to me, would be good to get a review from folks more familiar with the core client.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#issuecomment-1561283178

   it looks good to me... I will do a final through review once you rebase... and we should run the whole test suite also after rebasing.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] gemmellr commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1176860418


##########
docs/user-manual/en/client-reconnection.md:
##########
@@ -89,9 +89,20 @@ Client reconnection is configured using the following parameters:
   ridiculously large values. By setting this parameter you can set an upper limit
   on that value. The default value is `2000` milliseconds.
 
+- `ha`. This optional parameter determines weather the client will try to
+  reconnect to the backup node when the live node is not reachable.
+  The default value is `false`.
+  For more information on HA, please see [High Availability and Failover](ha.md).
+
 - `reconnectAttempts`. This optional parameter determines the total number of
-  reconnect attempts to make before giving up and shutting down. A value of
-  `-1` signifies an unlimited number of attempts. The default value is `0`.
+  reconnect attempts to make to the current live/backup pair before giving up.

Review Comment:
   Yeah, could probably do with some docs to clarify what specifically is a 'reconnect attempt' vs a 'failover attempt' and thus help show when you would use one/other/both, for those that dont already know the previous/existing behaviour and thus also wont be able tell later which one of these is the new option/behaviour (/me raises hand).
   
   (E.g, in other clients I know, there is only one notion of 'attempts', where I believe generally they retry the server originally connected to again and then try all the other servers currently known about until succeeding, then repeat [after delay] if still not connected...with that one-shot-at-each-server process collectively being '1 attempt')



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#issuecomment-1533179495

   @gemmellr thanks for your review and suggestion


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#issuecomment-1561281973

   @brusdev can you squash / rebase it? (there are a few small conflicts)


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic merged pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1178713896


##########
docs/user-manual/en/client-reconnection.md:
##########
@@ -89,9 +89,20 @@ Client reconnection is configured using the following parameters:
   ridiculously large values. By setting this parameter you can set an upper limit
   on that value. The default value is `2000` milliseconds.
 
+- `ha`. This optional parameter determines weather the client will try to
+  reconnect to the backup node when the live node is not reachable.
+  The default value is `false`.
+  For more information on HA, please see [High Availability and Failover](ha.md).
+
 - `reconnectAttempts`. This optional parameter determines the total number of
-  reconnect attempts to make before giving up and shutting down. A value of
-  `-1` signifies an unlimited number of attempts. The default value is `0`.
+  reconnect attempts to make to the current live/backup pair before giving up.

Review Comment:
   @gtully @gemmellr thanks for your feedback, I'll review the doc and I'll move the pause between 2 consecutive failover retries after trying all available connectors.



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1188845326


##########
docs/user-manual/en/client-reconnection-failover.md:
##########
@@ -1,10 +1,32 @@
-# Client Reconnection and Session Reattachment
+# Client Reconnection and Failover

Review Comment:
   Can we simplify the doc here?
   
   
   I still haven't figured out how you would configure the client to failover to another server... 
   
   I thought about just having two topics would make it simpler.
   
   
   
   
   - Failover to a backup Server
   
   
   (don't worry too much about mentioning Apache ActiveMQ bblablablabla all the time)
   
   just say, if you want to reconnect to a backup server, configure this, and that.
   
   
   
   
   
   - Failover to non backup servers
   
   Mention what you have to do if you want to failover to non backup servers...
   
   
   
   
   
   There are so many options here that's getting confusing to understand.
   
   
   that's my only concern here.



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1182305601


##########
docs/user-manual/en/client-reconnection.md:
##########
@@ -89,9 +89,20 @@ Client reconnection is configured using the following parameters:
   ridiculously large values. By setting this parameter you can set an upper limit
   on that value. The default value is `2000` milliseconds.
 
+- `ha`. This optional parameter determines weather the client will try to
+  reconnect to the backup node when the live node is not reachable.
+  The default value is `false`.
+  For more information on HA, please see [High Availability and Failover](ha.md).
+
 - `reconnectAttempts`. This optional parameter determines the total number of
-  reconnect attempts to make before giving up and shutting down. A value of
-  `-1` signifies an unlimited number of attempts. The default value is `0`.
+  reconnect attempts to make to the current live/backup pair before giving up.

Review Comment:
   done



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1198733716


##########
docs/user-manual/en/client-reconnection-failover.md:
##########
@@ -1,10 +1,32 @@
-# Client Reconnection and Session Reattachment
+# Client Reconnection and Failover

Review Comment:
   @clebertsuconic I partially rewrote the doc following your suggestions, could you take a look?



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] brusdev commented on a diff in pull request #4447: ARTEMIS-4251 Support CORE client failover to other live servers

Posted by "brusdev (via GitHub)" <gi...@apache.org>.
brusdev commented on code in PR #4447:
URL: https://github.com/apache/activemq-artemis/pull/4447#discussion_r1193313840


##########
docs/user-manual/en/client-reconnection-failover.md:
##########
@@ -1,10 +1,32 @@
-# Client Reconnection and Session Reattachment
+# Client Reconnection and Failover

Review Comment:
   @clebertsuconic thanks for your feedback, I'll rewrite the doc



-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org