[
https://issues.apache.org/jira/browse/CAMEL-12850?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16636509#comment-16636509
]
ASF GitHub Bot commented on CAMEL-12850:
----------------------------------------
davsclaus closed pull request #2545: CAMEL-12850: camel-ftp tries reconnects
twice as much as maximumRecon…
URL: https://github.com/apache/camel/pull/2545
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
index f4e5c349565..8c18751d84d 100644
---
a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
+++
b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileConsumer.java
@@ -55,12 +55,7 @@ protected boolean prePollCheck() throws Exception {
log.trace("prePollCheck on {}",
getEndpoint().getConfiguration().remoteServerInformation());
}
try {
- if (getEndpoint().getMaximumReconnectAttempts() > 0) {
- // only use recoverable if we are allowed any re-connect
attempts
- recoverableConnectIfNecessary();
- } else {
- connectIfNecessary();
- }
+ connectIfNecessary();
} catch (Exception e) {
loggedIn = false;
@@ -183,37 +178,6 @@ protected void forceDisconnect() {
}
}
- protected void recoverableConnectIfNecessary() throws Exception {
- try {
- connectIfNecessary();
- } catch (Exception e) {
- if (log.isDebugEnabled()) {
- log.debug("Could not connect to: " + getEndpoint() + ". Will
try to recover.", e);
- }
- loggedIn = false;
- }
-
- // recover by re-creating operations which should most likely be able
to recover
- if (!loggedIn) {
- log.debug("Trying to recover connection to: {} with a fresh
client.", getEndpoint());
- // we want to preserve last FTP activity listener when we set a
new operations
- if (operations instanceof FtpOperations) {
- FtpOperations ftpOperations = (FtpOperations) operations;
- FtpClientActivityListener listener =
ftpOperations.getClientActivityListener();
- setOperations(getEndpoint().createRemoteFileOperations());
- getOperations().setEndpoint(getEndpoint());
- if (listener != null) {
- ftpOperations = (FtpOperations) getOperations();
- ftpOperations.setClientActivityListener(listener);
- }
- } else {
- setOperations(getEndpoint().createRemoteFileOperations());
- getOperations().setEndpoint(getEndpoint());
- }
- connectIfNecessary();
- }
- }
-
protected void connectIfNecessary() throws IOException {
// We need to send a noop first to check if the connection is still
open
boolean isConnected = false;
diff --git
a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileProducer.java
b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileProducer.java
index 2f4c5b6582e..38ccdd1177e 100644
---
a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileProducer.java
+++
b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/RemoteFileProducer.java
@@ -20,7 +20,6 @@
import org.apache.camel.ServicePoolAware;
import org.apache.camel.component.file.GenericFileOperationFailedException;
import org.apache.camel.component.file.GenericFileProducer;
-import org.apache.camel.util.ObjectHelper;
import org.apache.camel.util.URISupport;
/**
@@ -128,12 +127,7 @@ public void preWriteCheck() throws Exception {
// if not alive then reconnect
if (!noop) {
try {
- if (getEndpoint().getMaximumReconnectAttempts() > 0) {
- // only use recoverable if we are allowed any re-connect
attempts
- recoverableConnectIfNecessary();
- } else {
- connectIfNecessary();
- }
+ connectIfNecessary();
} catch (Exception e) {
loggedIn = false;
@@ -179,47 +173,6 @@ protected void doStop() throws Exception {
super.doStop();
}
- protected void recoverableConnectIfNecessary() throws Exception {
- try {
- connectIfNecessary();
- } catch (Exception e) {
- loggedIn = false;
-
- // are we interrupted
- InterruptedException ie =
ObjectHelper.getException(InterruptedException.class, e);
- if (ie != null) {
- if (log.isDebugEnabled()) {
- log.debug("Interrupted during connect to: {}",
getEndpoint(), ie);
- }
- throw ie;
- }
-
- if (log.isDebugEnabled()) {
- log.debug("Could not connect to: " + getEndpoint() + ". Will
try to recover.", e);
- }
- }
-
- // recover by re-creating operations which should most likely be able
to recover
- if (!loggedIn) {
- log.debug("Trying to recover connection to: {} with a new FTP
client.", getEndpoint());
- // we want to preserve last FTP activity listener when we set a
new operations
- if (operations instanceof FtpOperations) {
- FtpOperations ftpOperations = (FtpOperations) operations;
- FtpClientActivityListener listener =
ftpOperations.getClientActivityListener();
- setOperations(getEndpoint().createRemoteFileOperations());
- getOperations().setEndpoint(getEndpoint());
- if (listener != null) {
- ftpOperations = (FtpOperations) getOperations();
- ftpOperations.setClientActivityListener(listener);
- }
- } else {
- setOperations(getEndpoint().createRemoteFileOperations());
- getOperations().setEndpoint(getEndpoint());
- }
- connectIfNecessary();
- }
- }
-
protected void connectIfNecessary() throws
GenericFileOperationFailedException {
if (!loggedIn || !getOperations().isConnected()) {
log.debug("Not already connected/logged in. Connecting to: {}",
getEndpoint());
diff --git
a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpBadLoginInProducerConnectionLeakTest.java
b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpBadLoginInProducerConnectionLeakTest.java
index 332a43ce548..56b7227e14b 100644
---
a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpBadLoginInProducerConnectionLeakTest.java
+++
b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpBadLoginInProducerConnectionLeakTest.java
@@ -59,9 +59,7 @@ public void testConnectionLeak() throws Exception {
}
}
- // maximumReconnectAttempts is related to TCP connects, not to FTP
login attempts
- // but having this parameter > 0 leads to two connection attempts
- assertEquals("Expected 4 socket connections to be created", 4,
socketAudits.size());
+ assertEquals("Expected 2 socket connections to be created", 2,
socketAudits.size());
for (Map.Entry<Integer, boolean[]> socketStats :
socketAudits.entrySet()) {
assertTrue("Socket should be connected",
socketStats.getValue()[0]);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> camel-ftp tries reconnects twice as much as maximumReconnectAttempts
> --------------------------------------------------------------------
>
> Key: CAMEL-12850
> URL: https://issues.apache.org/jira/browse/CAMEL-12850
> Project: Camel
> Issue Type: Bug
> Components: camel-ftp
> Affects Versions: 2.22.1
> Reporter: Tadayoshi Sato
> Assignee: Tadayoshi Sato
> Priority: Major
> Attachments: FtpTimeoutWithMaximumReconnectAttemptsTest.java
>
>
> When {{maximumReconnectAttempts > 0}} on the endpoint, both camel-ftp
> consumers and producers retry connecting to an unavailable FTP server twice
> as much as the number of {{maximumReconnectAttempts}}. It is because of the
> following logic in {{RemoteFileConsumer.prePollCheck()}} and
> {{RemoteFileProducer.preWriteCheck()}}:
> {code:java}
> if (getEndpoint().getMaximumReconnectAttempts() > 0) {
> // only use recoverable if we are allowed any re-connect
> attempts
> recoverableConnectIfNecessary();
> } else {
> connectIfNecessary();
> }
> {code}
> where {{recoverableConnectIfNecessary()}} retries
> {{RemoteFileOperations.connect()}} once in case of initial connection failure.
> Digging into the commit history, this logic appears to be introduced due to
> CAMEL-2829 as a workaround. However, since the root cause NET-327 is already
> resolved the logic doesn't seem to be necessary any more. So probably we can
> remove the logic to avoid attempting reconnects twice as much as
> {{maximumReconnectAttempts}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
