Skip to content

Commit

Permalink
Fix SFTP Timeout Issue (#1913)
Browse files Browse the repository at this point in the history 
* add timeout setting

* update deps

* force timeout

* dont auto disconnect on timeout

* override ftp check

* use timespan correctly

* bump version

* tidy up
  • Loading branch information
@JFriel
JFriel authored Aug 5, 2024
1 parent d27866b commit fda4b6b
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 29 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [8.2.3] - 2024-08-06

- Fix issue with SFTP downloader timeouts

## [8.2.2] - 2024-08-01

- Add DQE PostLoad runner
Expand Down
51 changes: 29 additions & 22 deletions Rdmp.Core/DataLoad/Modules/FTP/FTPDownloader.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand Down Expand Up @@ -66,18 +66,18 @@ public FTPDownloader()
[DemandsInitialization("The directory on the FTP server that you want to download files from")]
public string? RemoteDirectory { get; set; }

[DemandsInitialization("True to set keep alive",DefaultValue = true)]
[DemandsInitialization("True to set keep alive", DefaultValue = true)]
public bool KeepAlive { get; set; }


public void Initialize(ILoadDirectory directory,DiscoveredDatabase dbInfo)
public void Initialize(ILoadDirectory directory, DiscoveredDatabase dbInfo)
{
_directory = directory;
}

public ExitCodeType Fetch(IDataLoadJob job,GracefulCancellationToken cancellationToken)
public ExitCodeType Fetch(IDataLoadJob job, GracefulCancellationToken cancellationToken)
{
return DownloadFilesOnFTP(_directory ?? throw new InvalidOperationException("No output directory set"),job);
return DownloadFilesOnFTP(_directory ?? throw new InvalidOperationException("No output directory set"), job);
}

private FtpClient SetupFtp()
Expand All @@ -86,35 +86,42 @@ private FtpClient SetupFtp()
var username = FTPServer.Username ?? "anonymous";
var password = string.IsNullOrWhiteSpace(FTPServer.Password) ? "guest" : FTPServer.GetDecryptedPassword();
var c = new FtpClient(host, username, password);

if (TimeoutInSeconds > 0)
{
c.Config.ConnectTimeout = TimeoutInSeconds * 1000;
c.Config.ReadTimeout = TimeoutInSeconds * 1000;
c.Config.DataConnectionConnectTimeout = TimeoutInSeconds * 1000;
c.Config.DataConnectionReadTimeout = TimeoutInSeconds * 1000;
}
// Enable periodic NOOP keepalive operations to keep connection active until we're done
c.Config.Noop = true;
c.AutoConnect();

return c;
}

private ExitCodeType DownloadFilesOnFTP(ILoadDirectory destination,IDataLoadEventListener listener)
private ExitCodeType DownloadFilesOnFTP(ILoadDirectory destination, IDataLoadEventListener listener)
{
var files = GetFileList().ToArray();

listener.OnNotify(this,new NotifyEventArgs(ProgressEventType.Information,
$"Identified the following files on the FTP server:{string.Join(',',files)}"));
listener.OnNotify(this, new NotifyEventArgs(ProgressEventType.Information,
$"Identified the following files on the FTP server:{string.Join(',', files)}"));

var forLoadingContainedCachedFiles = false;

foreach (var file in files)
{
var action = GetSkipActionForFile(file,destination);
var action = GetSkipActionForFile(file, destination);

listener.OnNotify(this,new NotifyEventArgs(ProgressEventType.Information,
listener.OnNotify(this, new NotifyEventArgs(ProgressEventType.Information,
$"File {file} was evaluated as {action}"));

switch (action)
{
case SkipReason.DoNotSkip:
listener.OnNotify(this,
new NotifyEventArgs(ProgressEventType.Information,$"About to download {file}"));
Download(file,destination);
new NotifyEventArgs(ProgressEventType.Information, $"About to download {file}"));
Download(file, destination);
break;
case SkipReason.InForLoading:
forLoadingContainedCachedFiles = true;
Expand All @@ -141,9 +148,9 @@ protected enum SkipReason
IsImaginaryFile
}

protected SkipReason GetSkipActionForFile(string file,ILoadDirectory destination)
protected SkipReason GetSkipActionForFile(string file, ILoadDirectory destination)
{
if (file.StartsWith(".",StringComparison.Ordinal))
if (file.StartsWith(".", StringComparison.Ordinal))
return SkipReason.IsImaginaryFile;

//if there is a regex pattern
Expand All @@ -155,7 +162,7 @@ protected SkipReason GetSkipActionForFile(string file,ILoadDirectory destination
}


private static bool ValidateServerCertificate(object _1,X509Certificate _2,X509Chain _3,
private static bool ValidateServerCertificate(object _1, X509Certificate _2, X509Chain _3,
SslPolicyErrors _4) => true; //any cert will do! yay


Expand All @@ -164,18 +171,18 @@ protected virtual IEnumerable<string> GetFileList()
return _connection.Value.GetNameListing().ToList().Where(_connection.Value.FileExists);
}

protected virtual void Download(string file,ILoadDirectory destination)
protected virtual void Download(string file, ILoadDirectory destination)
{
var remotePath = !string.IsNullOrWhiteSpace(RemoteDirectory)
? $"{RemoteDirectory}/{file}"
: file;

var destinationFileName = Path.Combine(destination.ForLoading.FullName,file);
_connection.Value.DownloadFile(destinationFileName,remotePath);
var destinationFileName = Path.Combine(destination.ForLoading.FullName, file);
_connection.Value.DownloadFile(destinationFileName, remotePath);
_filesRetrieved.Add(remotePath);
}

public virtual void LoadCompletedSoDispose(ExitCodeType exitCode,IDataLoadEventListener postLoadEventListener)
public virtual void LoadCompletedSoDispose(ExitCodeType exitCode, IDataLoadEventListener postLoadEventListener)
{
if (exitCode != ExitCodeType.Success || !DeleteFilesOffFTPServerAfterSuccesfulDataLoad) return;

Expand All @@ -186,15 +193,15 @@ public virtual void LoadCompletedSoDispose(ExitCodeType exitCode,IDataLoadEventL
}


public void Check(ICheckNotifier notifier)
public virtual void Check(ICheckNotifier notifier)
{
try
{
SetupFtp();
}
catch (Exception e)
{
notifier.OnCheckPerformed(new CheckEventArgs("Failed to SetupFTP",CheckResult.Fail,e));
notifier.OnCheckPerformed(new CheckEventArgs("Failed to SetupFTP", CheckResult.Fail, e));
}
}
}
26 changes: 22 additions & 4 deletions Rdmp.Core/DataLoad/Modules/FTP/SFTPDownloader.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) The University of Dundee 2018-2019
// Copyright (c) The University of Dundee 2018-2024
// This file is part of the Research Data Management Platform (RDMP).
// RDMP is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
// RDMP is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Expand All @@ -10,6 +10,7 @@
using System.Threading;
using Rdmp.Core.Curation;
using Rdmp.Core.Curation.Data;
using Rdmp.Core.ReusableLibraryCode.Checks;
using Rdmp.Core.ReusableLibraryCode.Progress;
using Renci.SshNet;

Expand All @@ -30,7 +31,7 @@ public class SFTPDownloader : FTPDownloader

public SFTPDownloader(Lazy<SftpClient> connection)
{
_connection = new Lazy<SftpClient>(SetupSftp,LazyThreadSafetyMode.ExecutionAndPublication);
_connection = new Lazy<SftpClient>(SetupSftp, LazyThreadSafetyMode.ExecutionAndPublication);
}

public SFTPDownloader()
Expand All @@ -44,12 +45,29 @@ private SftpClient SetupSftp()
var username = FTPServer.Username ?? "anonymous";
var password = string.IsNullOrWhiteSpace(FTPServer.Password) ? "guest" : FTPServer.GetDecryptedPassword();
var c = new SftpClient(host, username, password);
if (TimeoutInSeconds > 0)
{
c.OperationTimeout = TimeSpan.FromSeconds(TimeoutInSeconds);
c.ConnectionInfo.Timeout = TimeSpan.FromSeconds(TimeoutInSeconds);
}
c.Connect();
if (KeepAlive)
c.KeepAliveInterval = TimeSpan.FromMilliseconds(KeepAliveIntervalMilliseconds);
return c;
}

public override void Check(ICheckNotifier notifier)
{
try
{
SetupSftp();
}
catch (Exception e)
{
notifier.OnCheckPerformed(new CheckEventArgs("Failed to SetupSFTP", CheckResult.Fail, e));
}
}


protected override void Download(string file, ILoadDirectory destination)
{
Expand All @@ -61,8 +79,8 @@ protected override void Download(string file, ILoadDirectory destination)

var destinationFilePath = Path.Combine(destination.ForLoading.FullName, file);

using (var dest=File.Create(destinationFilePath))
_connection.Value.DownloadFile(fullFilePath,dest);
using (var dest = File.Create(destinationFilePath))
_connection.Value.DownloadFile(fullFilePath, dest);
_filesRetrieved.Add(fullFilePath);
}

Expand Down
6 changes: 3 additions & 3 deletions SharedAssemblyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@
[assembly: AssemblyTrademark("")]
[assembly: AssemblyCulture("")]

[assembly: AssemblyVersion("8.2.2")]
[assembly: AssemblyFileVersion("8.2.2")]
[assembly: AssemblyInformationalVersion("8.2.2")]
[assembly: AssemblyVersion("8.2.3")]
[assembly: AssemblyFileVersion("8.2.3")]
[assembly: AssemblyInformationalVersion("8.2.3")]

0 comments on commit fda4b6b

Please sign in to comment.