2

I have a download button when clicked, it takes about 15 seconds to download a file because it has to SFTP into the server, find the right path/ files, and return the response.

<a class="btn btn-primary btn-sm text-primary btn-download-1" onclick="startDownload('1')"><i class="fa fa-download "></i></a>

This is the startDownload function:

function startDownload(interfaceId) {
    window.location = "/nodes/interface/capture/download?port=" + interfaceId;
}

The backend code in /nodes/interface/capture/download:

public function download_files()
{

    $dir = '';
    $portNumber = Request::get('port');
    $zipMe = false;

    $remotePath = "/home/john/logs/".$dir."/";

    if (!isset($dir) || $dir == null) {
        return redirect()->back()->withInput()->withFlashDanger('SFTP Could not connect.');
    }

    $acsIp =  explode('://', env('ACS_URL'));
    $acsIp =  explode(':',$acsIp[1])[0];
    $sftp = new SFTP($acsIp.':22');

    if (!$sftp->login('john', '***')) {
        return redirect()->back()->withInput()->withFlashDanger('SFTP Could not connect.');
    }

    // Get into the Specified Directory
    $sftpConn = Storage::disk('sftp');

    $SFTPFiles = $sftpConn->allFiles('/'.$dir);

    if ( count($SFTPFiles) > 0 ) {
        foreach ($SFTPFiles as $file) {
            $fileName = $file;
            break;
        }

    } else {
        \Log::info('Files Not found in the Remote!');
        return redirect()->back()->withInput()->withFlashDanger('Files Not found in the Remote!');
    }

    // Create and give 777 permission to remote-files directory
    if (!is_dir(public_path('remote-files/'.$dir))) {
        mkdir(public_path('remote-files/'.$dir), 0777, true);
    }

    $filesToZip = [];

    foreach ( $SFTPFiles as $fileName ) {
        if ( $fileName == '..' || $fileName == '.' ) {
            continue;
        } else if ( $fileName == '' ) {
            \Log::info('File not found');
            continue;
        }

        $fileName     = explode("/", $fileName);
        $onlyFileName = (!empty($fileName) && isset($fileName[1])) ? $fileName[1] : "";
        $filepath = $remotePath.$onlyFileName;

        if (strpos($onlyFileName , $portNumber) !== false) {


            // Download the remote file at specified location in Local
            if (!$sftp->get($filepath, 'remote-files/'.$dir.'/'.$onlyFileName))
            {
                die("Error downloading file ".$filepath);
            }

            $file = public_path('remote-files/'.$dir.'/').$onlyFileName;

            $headers = array(
                'Content-Description: File Transfer',
                'Content-Type: application/octet-stream',
                'Content-Disposition: attachment; filename="'.basename($file).'"',
                'Cache-Control: must-revalidate',
                'Pragma: public',
                'Content-Length: ' . filesize($file)
            );

            return Response::download($file, $onlyFileName, $headers);
        }

        // IF File is exists in Directory
        if ( file_exists( public_path('remote-files/'.$dir.'/').$onlyFileName ) ) {
            $filesToZip[] = public_path('remote-files/'.$dir.'/').$onlyFileName;
            \Log::info('File Generated '.'remote-files/'.$dir.'/'.$onlyFileName);

            // Remove Files from public/remote-files
            $this->removeDirAndFiles('', public_path('remote-files/'.$dir));
            exit;

        } else {
            \Log::info('File not Generated '.'remote-files/'.$dir.'/'.$onlyFileName);
        }
    }
}

This code indeed works but takes about 15 seconds, which is too long for the use case.

Is there a way to speed this up? Is there something wrong with my code or is this to be expected? Should I consider switching to SCP? Should I reconsider authentication?

creyD
  • 1,972
  • 3
  • 26
  • 55
code-8
  • 54,650
  • 106
  • 352
  • 604
  • What's the file size you are trying to download? I don't know, if you are facing the same issue or not, but anyway take a look [here](https://stackoverflow.com/questions/8849240/why-when-i-transfer-a-file-through-sftp-it-takes-longer-than-ftp). – reyad Jul 24 '20 at 15:14
  • 1
    Where is it spending its time? Opening the SFTP connection? Actually transferring the file? How big is the file? What rate (bytes per second) are you getting? – Kenster Jul 24 '20 at 16:01
  • The file size is 4kb. How do I debug where is it taking time ? How do I check the rate of transferring ? – code-8 Jul 24 '20 at 16:06
  • Why don't you log each operation with SFTP with time or just differential time and you will see the impact of each one. Also `$dir` is empty so remote path has two slashes. – ZeroWorks Jul 29 '20 at 15:38
  • @cyber8200 for measuring the time do in this way, at the start of the script do $start = time(); during the execution you can do $mid = time(); $time_spent = $mid - $start; this will give you the number of secs that passed from $start to $mid, and so on you can iterate to see which interval is taking a lot of time – d3javu999 Jul 29 '20 at 15:58
  • @d3javu999, let me do that, thanks for your suggestion, but SFTP using PHP should not take 7 seconds, right ? – code-8 Jul 29 '20 at 15:59
  • Does it matter in speed, if I used password vs. key to authenticated ? – code-8 Jul 29 '20 at 15:59
  • @cyber8200 I would say not, especially for a 4kb file. Why you are looping through all the files? I suspect is that the reason of all the time spent because for each of them you are doing this $sftp->get($filepath, 'remote-files/'.$dir.'/'.$onlyFileName) – d3javu999 Jul 29 '20 at 16:04
  • I need to check all file name and only download the specific one – code-8 Jul 29 '20 at 16:22
  • If you add a map, database or something that is matching the id to the file path, you won't have to loop through all the files and the entire application will result more efficient – d3javu999 Jul 29 '20 at 16:59
  • @d3javu999 I will do that. – code-8 Jul 29 '20 at 17:40
  • Great, let me know how it goes the time monitoring – d3javu999 Jul 29 '20 at 17:43
  • https://stackoverflow.com/a/24792785/5300921 is it mcrypt extension missing? – UnpassableWizard Jul 29 '20 at 20:37
  • Maybe I'm loosing something but `$sftp->get` is called only once when filename contains `$portNumber`... so how many files are in this folder? Do you know file name or only `$portNumber`? if you know file name you coud try to download directly... If not... i'd file mapping as @d3javu999 suggested. – ZeroWorks Jul 30 '20 at 17:31
  • I think I can download base on the actual name, but the SFTP somehow is really slow – code-8 Jul 30 '20 at 17:32
  • Can you check @Rustyjim suggested it has sense. Also... the timings... woud be nice to know where the script stucks... – ZeroWorks Jul 31 '20 at 08:19
  • Looping through all $SFTPFiles seems like a bad design, as remote file list grows the problem will be bigger and bigger... – Bart Jul 31 '20 at 19:08
  • SFTP is slower compared to FTP as the maximum size of the packets is dictated by the protocol itself. Each packet in SFTP is encrypted before being written to the outgoing socket from the client which is decrypted when received by the server. This of-course leads to slow transfer rates but very secure transfer. Source: Google – Jaswant Singh Aug 05 '20 at 12:39

2 Answers2

6

I changed your function to measure the time in different parts, so you can understand which is the piece that is slowing down your application by looking at the log for this string "### [TIME] -- "

public function download_files()
{
    $start = time();
    $dir = '';
    $portNumber = Request::get('port');
    $zipMe = false;

    \Log::info("### [TIME] -- t1 =  " . (time() - $start));

    $remotePath = "/home/john/logs/".$dir."/";

    if (!isset($dir) || $dir == null) {
        return redirect()->back()->withInput()->withFlashDanger('SFTP Could not connect.');
    }

    $acsIp =  explode('://', env('ACS_URL'));
    $acsIp =  explode(':',$acsIp[1])[0];

    $t1 = time();

    $sftp = new SFTP($acsIp.':22');

    $t2 = time();
    \Log::info("### [TIME] -- SFTP Instantiation took " . ($t2 - $t1) . " secs");

    if (!$sftp->login('john', '***')) {
        return redirect()->back()->withInput()->withFlashDanger('SFTP Could not connect.');
    }

    $t1 = time();
    // Get into the Specified Directory
    $sftpConn = Storage::disk('sftp');

    $SFTPFiles = $sftpConn->allFiles('/'.$dir);

    $t2 = time();
    \Log::info("### [TIME] -- SFTPFiles list took " . ($t2 - $t1) . " secs");


    // this loop is not clear to me, you basically take the first element and
    // exit the loop
    if ( count($SFTPFiles) > 0 ) {
        foreach ($SFTPFiles as $file) {
            $fileName = $file;
            break;
        }
    } else {
        \Log::info('Files Not found in the Remote!');
        return redirect()->back()->withInput()->withFlashDanger('Files Not found in the Remote!');
}

    $t1 = time();

    // Create and give 777 permission to remote-files directory
    if (!is_dir(public_path('remote-files/'.$dir))) {
        mkdir(public_path('remote-files/'.$dir), 0777, true);
    }

    $t2 = time();
    \Log::info("### [TIME] -- Directory creation took " . ($t2 - $t1) . " secs");

    $filesToZip = [];
    $t1 = time();
    foreach ( $SFTPFiles as $fileName ) 
    {
        $start_loop_time = time();
        \Log::info("### [TIME] -- Loop for $fileName took " . (time() - $t1) . " secs");

        if ( $fileName == '..' || $fileName == '.' ) {
            continue;
        } else if ( $fileName == '' ) {
            \Log::info('File not found');
            continue;
        }

        $fileName     = explode("/", $fileName);
        $onlyFileName = (!empty($fileName) && isset($fileName[1])) ? $fileName[1] : "";
        $filepath = $remotePath.$onlyFileName;

        if (strpos($onlyFileName , $portNumber) !== false) {

             $responseCreationStart = time();
            // Download the remote file at specified location in Local
            if (!$sftp->get($filepath, 'remote-files/'.$dir.'/'.$onlyFileName))
            {
                die("Error downloading file ".$filepath);
            }

            $file = public_path('remote-files/'.$dir.'/').$onlyFileName;

            $headers = array(
                'Content-Description: File Transfer',
                'Content-Type: application/octet-stream',
                'Content-Disposition: attachment; filename="'.basename($file).'"',
                'Cache-Control: must-revalidate',
                'Pragma: public',
                'Content-Length: ' . filesize($file)
            );
            $responseCreationEnd = time();
            \Log::info("### [TIME] -- Response creation took " . ($responseCreationEnd  - $responseCreationStart ) . " secs");
            return Response::download($file, $onlyFileName, $headers);

        }

        // IF File is exists in Directory
        if ( file_exists( public_path('remote-files/'.$dir.'/').$onlyFileName ) ) {
            $t3 = time();
            $filesToZip[] = public_path('remote-files/'.$dir.'/').$onlyFileName;
            \Log::info('File Generated '.'remote-files/'.$dir.'/'.$onlyFileName);

            // Remove Files from public/remote-files
            $this->removeDirAndFiles('', public_path('remote-files/'.$dir));
            $t4 = time();
            \Log::info("### [TIME] -- Deletion took " . ($t4 - $t3) . " secs");
            exit;

        } else {
            \Log::info('File not Generated '.'remote-files/'.$dir.'/'.$onlyFileName);
        }

        \Log::info("### [TIME] -- Loop end reached in  " . (time() - $start_loop_time ) . " secs");
    }
}
d3javu999
  • 323
  • 1
  • 8
1

Do you set the $dir variable anywhere in your code? Because as how I read it, the only contents of it is, and always will be, an empty string.

Does this take a long time in other browsers as well? Is there anything notable that pops up in the "Network" tab of the browser inspector after pressing the button?

Furthermore, perhaps you could try to put the function in a button element instead of a hyperlink.
Perhaps this delay is some sort of timeout that the browser has internally, since it is expecting another page to load?

So instead of using the hyperlink for the button, I would suggest giving this a try:

<button class="btn btn-primary btn-sm text-primary btn-download-1" onclick="startDownload('1')"><i class="fa fa-download "></i></button>

I'm curious to know what will happen. Keep us updated!

code-8
  • 54,650
  • 106
  • 352
  • 604
Nardwacht
  • 565
  • 3
  • 5