NIFI-4386 Fixing connection handling in FetchFileTransfer

Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>

This closes #2203.
This commit is contained in:
Bryan Bende 2017-10-09 15:25:27 -04:00 committed by Pierre Villard
parent 13e42678b6
commit 883c223ced
1 changed files with 111 additions and 98 deletions

View File

@ -17,20 +17,6 @@
package org.apache.nifi.processors.standard; package org.apache.nifi.processors.standard;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.apache.nifi.annotation.lifecycle.OnStopped; import org.apache.nifi.annotation.lifecycle.OnStopped;
import org.apache.nifi.components.AllowableValue; import org.apache.nifi.components.AllowableValue;
@ -50,6 +36,20 @@ import org.apache.nifi.stream.io.StreamUtils;
import org.apache.nifi.util.StopWatch; import org.apache.nifi.util.StopWatch;
import org.apache.nifi.util.Tuple; import org.apache.nifi.util.Tuple;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
/** /**
* A base class for FetchSFTP, FetchFTP processors. * A base class for FetchSFTP, FetchFTP processors.
* *
@ -230,6 +230,8 @@ public abstract class FetchFileTransfer extends AbstractProcessor {
transfer = transferWrapper.getFileTransfer(); transfer = transferWrapper.getFileTransfer();
} }
boolean closeConnection = false;
try {
// Pull data from remote system. // Pull data from remote system.
final InputStream in; final InputStream in;
try { try {
@ -242,32 +244,28 @@ public abstract class FetchFileTransfer extends AbstractProcessor {
} }
}); });
if(!transfer.flush(flowFile)) { if (!transfer.flush(flowFile)) {
throw new IOException("completePendingCommand returned false, file transfer failed"); throw new IOException("completePendingCommand returned false, file transfer failed");
} }
transferQueue.offer(new FileTransferIdleWrapper(transfer, System.nanoTime()));
} catch (final FileNotFoundException e) { } catch (final FileNotFoundException e) {
closeConnection = false;
getLogger().error("Failed to fetch content for {} from filename {} on remote host {} because the file could not be found on the remote system; routing to {}", getLogger().error("Failed to fetch content for {} from filename {} on remote host {} because the file could not be found on the remote system; routing to {}",
new Object[] {flowFile, filename, host, REL_NOT_FOUND.getName()}); new Object[]{flowFile, filename, host, REL_NOT_FOUND.getName()});
session.transfer(session.penalize(flowFile), REL_NOT_FOUND); session.transfer(session.penalize(flowFile), REL_NOT_FOUND);
session.getProvenanceReporter().route(flowFile, REL_NOT_FOUND); session.getProvenanceReporter().route(flowFile, REL_NOT_FOUND);
return; return;
} catch (final PermissionDeniedException e) { } catch (final PermissionDeniedException e) {
closeConnection = false;
getLogger().error("Failed to fetch content for {} from filename {} on remote host {} due to insufficient permissions; routing to {}", getLogger().error("Failed to fetch content for {} from filename {} on remote host {} due to insufficient permissions; routing to {}",
new Object[] {flowFile, filename, host, REL_PERMISSION_DENIED.getName()}); new Object[]{flowFile, filename, host, REL_PERMISSION_DENIED.getName()});
session.transfer(session.penalize(flowFile), REL_PERMISSION_DENIED); session.transfer(session.penalize(flowFile), REL_PERMISSION_DENIED);
session.getProvenanceReporter().route(flowFile, REL_PERMISSION_DENIED); session.getProvenanceReporter().route(flowFile, REL_PERMISSION_DENIED);
return; return;
} catch (final ProcessException | IOException e) { } catch (final ProcessException | IOException e) {
try { closeConnection = true;
transfer.close();
} catch (final IOException e1) {
getLogger().warn("Failed to close connection to {}:{} due to {}", new Object[] {host, port, e.toString()}, e);
}
getLogger().error("Failed to fetch content for {} from filename {} on remote host {}:{} due to {}; routing to comms.failure", getLogger().error("Failed to fetch content for {} from filename {} on remote host {}:{} due to {}; routing to comms.failure",
new Object[] {flowFile, filename, host, port, e.toString()}, e); new Object[]{flowFile, filename, host, port, e.toString()}, e);
session.transfer(session.penalize(flowFile), REL_COMMS_FAILURE); session.transfer(session.penalize(flowFile), REL_COMMS_FAILURE);
return; return;
} }
@ -306,7 +304,7 @@ public abstract class FetchFileTransfer extends AbstractProcessor {
// file doesn't exist -- effectively the same as removing it. Move on. // file doesn't exist -- effectively the same as removing it. Move on.
} catch (final IOException ioe) { } catch (final IOException ioe) {
getLogger().warn("Successfully fetched the content for {} from {}:{}{} but failed to remove the remote file due to {}", getLogger().warn("Successfully fetched the content for {} from {}:{}{} but failed to remove the remote file due to {}",
new Object[] {flowFile, host, port, filename, ioe}, ioe); new Object[]{flowFile, host, port, filename, ioe}, ioe);
} }
} else if (COMPLETION_MOVE.getValue().equalsIgnoreCase(completionStrategy)) { } else if (COMPLETION_MOVE.getValue().equalsIgnoreCase(completionStrategy)) {
String targetDir = context.getProperty(MOVE_DESTINATION_DIR).evaluateAttributeExpressions(flowFile).getValue(); String targetDir = context.getProperty(MOVE_DESTINATION_DIR).evaluateAttributeExpressions(flowFile).getValue();
@ -320,7 +318,22 @@ public abstract class FetchFileTransfer extends AbstractProcessor {
transfer.rename(flowFile, filename, target); transfer.rename(flowFile, filename, target);
} catch (final IOException ioe) { } catch (final IOException ioe) {
getLogger().warn("Successfully fetched the content for {} from {}:{}{} but failed to rename the remote file due to {}", getLogger().warn("Successfully fetched the content for {} from {}:{}{} but failed to rename the remote file due to {}",
new Object[] {flowFile, host, port, filename, ioe}, ioe); new Object[]{flowFile, host, port, filename, ioe}, ioe);
}
}
} finally {
if (transfer != null) {
if (closeConnection) {
getLogger().debug("Closing FileTransfer...");
try {
transfer.close();
} catch (final IOException e) {
getLogger().warn("Failed to close connection to {}:{} due to {}", new Object[]{host, port, e.getMessage()}, e);
}
} else {
getLogger().debug("Returning FileTransfer to pool...");
transferQueue.offer(new FileTransferIdleWrapper(transfer, System.nanoTime()));
}
} }
} }
} }