mv slow, always uses copyFile, even if fs.rename is possible

[anneb]

After uploading a large file (using tempfiles), the mv function may take quite some time, because the tempfile is being copied instead of being renamed. This takes more time and requires more available disk space.

Pull request #157 tries to rename the file and resort to copyFile only if the rename should fail.

[RomanBurunkov]

Hi @anneb , from the best of my knowledge that was changed from move to copy because mv method doesn't work if you try to mv file between different partitions.

Does your change work for such cases?

[anneb]

Indeed, the standard rename method does not work across partitions. For this reason, the code in the pull request first tries a rename and resorts to 'copy' if the rename should fail.

If this code is applied, It is advisable (not required) to store temp files on the same partition as uploaded files to improve performance.

[RomanBurunkov]

One thing I'm not sure that rename throws an error in case of different partitions.
As far as I remember it doesn't do that and that is the reason why code had been changed to use copy instead of rename.

Do you now something about that?

[anneb]

According to documentation, fs.rename throws error code 'EXDEV' if it cannot rename across devices.

In this commit: a4f13ff, I see that fs.rename gets replaced by copyFile to solve the 'mv across filesystems' problem. The problem of not throwing an error is not mentioned.

I just tested with both Ubuntu and Windows:

app.use(fileUpload({
    useTempFiles: true,
    tempFileDir: `/media/path_to_externaldisk/temp`
  }));

app.use(fileUpload({
    useTempFiles: true,
    tempFileDir: `d:/temp`
  }));

In both cases, fs.rename throws an error with code 'EXDEV' and the code from the pull request then calls the original copyFile code:

const moveFile = (src, dst, callback) => {
  fs.rename(src, dst, err => {
    if (err) {
        // Copy file to dst and delete src whether success.
        copyFile(src, dst, err => err ? callback(err) : deleteFile(src, callback));      
        return;
      }
      callback();
    });
};

[RomanBurunkov]

Hi @anneb ,

Thanks for checking that!
I'm going to merge that fix tomorrow morning.