From 6ca12036c0604f2705aaaa5d9dc21456eaeb58f4 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Mon, 2 Mar 2020 14:22:42 -0500 Subject: [PATCH] GH-3199: TCP FailoverClientCF fail back default Resolves https://github.com/spring-projects/spring-integration/issues/3199 - change the default behavior to not fail back until the current connection fails - reduce method complexity (Sonar) - 5.3 only --- .../FailoverClientConnectionFactory.java | 40 +++++++++++-------- src/reference/asciidoc/ip.adoc | 12 +++--- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java index 2e3ca816f64..9dabd3578e1 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/FailoverClientConnectionFactory.java @@ -42,7 +42,7 @@ */ public class FailoverClientConnectionFactory extends AbstractClientConnectionFactory { - private static final long DEFAULT_REFRESH_SHARED_INTERVAL = 0L; + private static final long DEFAULT_REFRESH_SHARED_INTERVAL = Long.MAX_VALUE; private final List factories; @@ -50,7 +50,9 @@ public class FailoverClientConnectionFactory extends AbstractClientConnectionFac private long refreshSharedInterval = DEFAULT_REFRESH_SHARED_INTERVAL; - private boolean closeOnRefresh; + private boolean closeOnRefresh = true; + + private boolean failBack; private volatile long creationTime; @@ -69,10 +71,9 @@ public FailoverClientConnectionFactory(List fac /** * When using a shared connection {@link #setSingleUse(boolean) singleUse} is false, * specify how long to wait before trying to fail back to start from the beginning of - * the factory list. Default is 0 for backwards compatibility to always try to get a - * connection to the primary server. If you don't want to fail back until the current - * connection is closed, set this to {@link Long#MAX_VALUE}. - * Cannot be changed when using {@link CachingClientConnectionFactory} delegates. + * the factory list. Default is {@link Long#MAX_VALUE} - meaning only fail back when + * the current connection fails. Cannot be changed when using + * {@link CachingClientConnectionFactory} delegates. * @param refreshSharedInterval the interval in milliseconds. * @since 4.3.22 * @see #setSingleUse(boolean) @@ -82,6 +83,7 @@ public void setRefreshSharedInterval(long refreshSharedInterval) { Assert.isTrue(!this.cachingDelegates, "'refreshSharedInterval' cannot be changed when using 'CachingClientConnectionFactory` delegates"); this.refreshSharedInterval = refreshSharedInterval; + this.failBack = refreshSharedInterval != Long.MAX_VALUE; } /** @@ -148,7 +150,7 @@ public void registerSender(TcpSender sender) { protected TcpConnectionSupport obtainConnection() throws InterruptedException { FailoverTcpConnection sharedConnection = (FailoverTcpConnection) getTheConnection(); boolean shared = !isSingleUse() && !this.cachingDelegates; - boolean refreshShared = shared + boolean refreshShared = this.failBack && shared && sharedConnection != null && System.currentTimeMillis() > this.creationTime + this.refreshSharedInterval; if (sharedConnection != null && sharedConnection.isOpen() && !refreshShared) { @@ -161,20 +163,26 @@ protected TcpConnectionSupport obtainConnection() throws InterruptedException { } failoverTcpConnection.incrementEpoch(); if (shared) { - this.creationTime = System.currentTimeMillis(); - /* - * We may have simply wrapped the same connection in a new wrapper; don't close. - */ - if (refreshShared && this.closeOnRefresh - && !sharedConnection.delegate.equals(failoverTcpConnection.delegate) - && sharedConnection.isOpen()) { - sharedConnection.close(); - } + closeRefreshedIfNecessary(sharedConnection, refreshShared, failoverTcpConnection); setTheConnection(failoverTcpConnection); } return failoverTcpConnection; } + private void closeRefreshedIfNecessary(FailoverTcpConnection sharedConnection, boolean refreshShared, + FailoverTcpConnection failoverTcpConnection) { + + this.creationTime = System.currentTimeMillis(); + /* + * We may have simply wrapped the same connection in a new wrapper; don't close. + */ + if (refreshShared && this.closeOnRefresh + && !sharedConnection.delegate.equals(failoverTcpConnection.delegate) + && sharedConnection.isOpen()) { + sharedConnection.close(); + } + } + @Override public void start() { for (AbstractClientConnectionFactory factory : this.factories) { diff --git a/src/reference/asciidoc/ip.adoc b/src/reference/asciidoc/ip.adoc index 7a902ec8bc1..b2d1a7758f4 100644 --- a/src/reference/asciidoc/ip.adoc +++ b/src/reference/asciidoc/ip.adoc @@ -536,23 +536,25 @@ The following example shows how to configure a failover client connection factor NOTE: When using the failover connection factory, the `singleUse` property must be consistent between the factory itself and the list of factories it is configured to use. -The connection factory has two properties when used with a shared connection (`singleUse=false`): +The connection factory has two properties related to failing back, when used with a shared connection (`singleUse=false`): * `refreshSharedInterval` * `closeOnRefresh` -These are `0` and `false` to retain the same behavior that existed before the properties were added. - Consider the following scenario based on the above configuration: Let's say `clientFactory1` cannot establish a connection but `clientFactory2` can. -Each time the `failCF` `getConnection()` method is called, we will again attempt to connect using `clientFactory1`; if successful, the "old" connection will remain open and may be reused in future if the first factory fails once more. +When the `failCF` `getConnection()` method is called after the `refreshSharedInterval` has passed, we will again attempt to connect using `clientFactory1`; if successful, the connection to `clientFactory2` will be closed. +If `closeOnRefresh` is `false`, the "old" connection will remain open and may be reused in future if the first factory fails once more. -Set `refreshSharedInterval` to only attempt to reconnect with the first factory after that time has expired; set it to `Long.MAX_VALUE` if you only want to fail back to the first factory when the current connection fails. +Set `refreshSharedInterval` to only attempt to reconnect with the first factory after that time has expired; setting it to `Long.MAX_VALUE` (default) if you only want to fail back to the first factory when the current connection fails. Set `closeOnRefresh` to close the "old" connection after a refresh actually creates a new connection. IMPORTANT: These properties do not apply if any of the delegate factories is a `CachingClientConnectionFactory` because the connection caching is handled there; in that case the list of connection factories will always be consulted to get a connection. +Starting with version 5.3, these default to `Long.MAX_VALUE` and `true` so the factory only attempts to fail back when the current connection fails. +To revert to the default behavior of previous versions, set them to `0` and `false`. + [[tcp-affinity-cf]] ==== TCP Thread Affinity Connection Factory