Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs.copyFile fails copying from nfs source to nfs dest file. #36439

Closed
moqtadir opened this issue Dec 8, 2020 · 16 comments
Closed

fs.copyFile fails copying from nfs source to nfs dest file. #36439

moqtadir opened this issue Dec 8, 2020 · 16 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding.

Comments

@moqtadir
Copy link

moqtadir commented Dec 8, 2020

When copying a file from nfs mount to the same nfs mount, fs.copyFileSync fails with ENOTSUP error.
This was not replicated in Node 12.x, but is reproducible consistently in node v14.15.1

To reproduce using 2 line test file
Using node 14.15.1 on CentOS 7 X86_64
And make sure /mnt/nfs_assets/ is an nfs4 mount.

[root@local]# cat <<'EOF' > test.js
const fs = require('fs');
fs.copyFileSync('/mnt/nfs_assets/source.txt', '/mnt/nfs_assets/dest.txt');
EOF

[root@local]# cat <<'EOF' > /mnt/nfs_assets/source.txt
foorbar
EOF

[root@local]# node test.js
internal/fs/utils.js:308
    throw err;
    ^

Error: ENOTSUP: operation not supported on socket, copyfile '/mnt/nfs_assets/source.txt' -> '/mnt/nfs_assets/dest.txt'
    at Object.copyFileSync (fs.js:1991:3)
    at Object.<anonymous> (/home/user/test.js:2:4)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
    at internal/main/run_main_module.js:17:47 {
  errno: -95,
  syscall: 'copyfile',
  code: 'ENOTSUP',
  path: '/mnt/nfs_assets/source.txt',
  dest: '/mnt/nfs_assets/dest.txt'
}

I haven't researched this much, but the ifdef (linux) second was added recently and is now using the copyfile call but does not account for ENOTSUP being returned?

#ifdef __linux__

Relevant commit.
cf34854

Please let me know if you need any additional information.

@Trott Trott added the fs Issues and PRs related to the fs subsystem / file system. label Dec 10, 2020
@Trott
Copy link
Member

Trott commented Dec 10, 2020

@nodejs/libuv Reporter suspects the cited libuv change.

@Trott
Copy link
Member

Trott commented Dec 10, 2020

@moqtadir The commit you refer to is in Node.js 14.9.0 but not 14.8.0. Are you able to test with those two versions to confirm that 14.8.0 does not exhibit the bug and 14.9.0 does?

@Trott Trott added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Dec 10, 2020
@moqtadir
Copy link
Author

@Trott Apologize for the delay, not sure why I didn't get notified on your comments.

Yes I can confirm issue doesn't happen on 14.8 but does on 14.9 - see below for all commands I ran to verify this.

[root@local ~]# # /mnt/nfs_mount is an nfs4 mounted path, create source file to copy.
[root@local ~]# echo "test-data" > /mnt/nfs_mount/source.txt
[root@local ~]# # using docker to test node 14.8 and 14.9 with the same one line script to copy file.
[root@local ~]# docker run   -v /mnt/nfs_mount:/mnt/nfs_mount node:14.8.0-slim node -e "require('fs').copyFileSync('/mnt/nfs_mount/source.txt', '/mnt/nfs_mount/dest.txt');"
[root@local ~]# #check file copied - yes
[root@local ~]# ls /mnt/nfs_mount/dest.txt
/mnt/nfs_mount/dest.txt
[root@local ~]# /bin/rm -f /mnt/nfs_mount/dest.txt  #cleanup dest file before testing 14.9
[root@local ~]# docker run   -v /mnt/nfs_mount:/mnt/nfs_mount node:14.9.0-slim node -e "require('fs').copyFileSync('/mnt/nfs_mount/source.txt', '/mnt/nfs_mount/dest.txt');"
internal/fs/utils.js:298
    throw err;
    ^

Error: ENOTSUP: operation not supported on socket, copyfile '/mnt/nfs_mount/source.txt' -> '/mnt/nfs_mount/dest.txt'
    at Object.copyFileSync (fs.js:1941:3)
    at [eval]:1:15
    at Script.runInThisContext (vm.js:132:18)
    at Object.runInThisContext (vm.js:309:38)
    at Object.<anonymous> ([eval]-wrapper:10:26)
    at Module._compile (internal/modules/cjs/loader.js:1075:30)
    at evalScript (internal/process/execution.js:92:25)
    at internal/main/eval_string.js:23:3 {
  errno: -95,
  syscall: 'copyfile',
  code: 'ENOTSUP',
  path: '/mnt/nfs_mount/source.txt',
  dest: '/mnt/nfs_mount/dest.txt'
}

@mmomtchev
Copy link
Contributor

copy_file_range appeared in Linux kernel 4.5 but some newer glibc versions will provide a user-space shim if the kernel support is not present.
@moqtadir , CentOS 7 does not support either of them.
This is a very useful syscall, especially on NFS/CIFS where the copy is supposed to be handled entirely server-side, so I think it should stay.
But maybe a correct auto-detection is to be implemented? @Trott @cjihrig ?

@mmomtchev
Copy link
Contributor

mmomtchev commented Dec 14, 2020

@moqtadir , can you please try to compile and run this on your platform:

#define _GNU_SOURCE  
#include <unistd.h>
#include <sys/syscall.h>
#include <stdio.h>
#include <errno.h>

#ifndef __NR_copy_file_range
#define __NR_copy_file_range (-1)
#endif

int main() {
    int r = syscall(326, 4, 0, 5, 0, 1024, 0);
    printf("r is %d, errno is %d, copy_file_range %d\n", r, errno, __NR_copy_file_range);
}

@mmomtchev
Copy link
Contributor

Or maybe directly this replacing existing_nfs_file and new_nfs_file

#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>

#ifndef __NR_copy_file_range
#define __NR_copy_file_range (-1)
#endif

int main() {
    int in = open("existing_nfs_file", O_RDONLY);
    int out = open("new_nfs_file", O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR);
    int r = syscall(326, in, 0, out, 0, 1, 0);
    printf("in is %d, out is %d, r is %d, errno is %d, copy_file_range %d\n",
            in, out, r, errno, __NR_copy_file_range);
}

@moqtadir
Copy link
Author

moqtadir commented Dec 14, 2020

@mmomtchev The result is below

root@3e2cc13f258f:/# ./a.out
in is 3, out is 4, r is -1, errno is 95, copy_file_range 326

@moqtadir
Copy link
Author

moqtadir commented Dec 14, 2020

Also by the way, there was a similar issue in golang and they basically added exception for ENOTSUPP in addition to EINVAL because a copyfilerange on an NFS mount can return ENOTSUP.

golang/go#40731

@mmomtchev
Copy link
Contributor

@moqtadir , yes precisely, this is the problem and the right solution

@piccit
Copy link

piccit commented Jan 6, 2021

Is there a workaround for this? I'm unclear on the timeline on how long it will take for that libuv fix to be released

@Trott
Copy link
Member

Trott commented Jan 21, 2021

Is there a workaround for this? I'm unclear on the timeline on how long it will take for that libuv fix to be released

I don't have a workaround to suggest but a fix has landed in the libuv code base but has not yet made it into a release. Should be in the next libuv release I imagine, though.

@moqtadir
Copy link
Author

Is there a workaround for this? I'm unclear on the timeline on how long it will take for that libuv fix to be released

You can use require('child_process').spawnSync('/bin/cp', ['-bf', fromFile, toFile], { stdio: 'ignore' })
Obviously adjust flags and copy command to your need based on platform.
Alternatively open the source and destination file streams and copy chunks yourself.

@Trott
Copy link
Member

Trott commented Feb 28, 2021

The fix landed in [email protected] which was in Node.js 15.9.0. So ti should be fixed in Node.js 15.x.

AFAIK, the bug still exists in Node.js 14.

n0mn0m pushed a commit to helxplatform/devops that referenced this issue May 4, 2021
- https://jenkins.renci.org/job/helx-ui/37/console
- nodejs/node#36439

Upstream node fix due in next LTS release, but this is blocking builds. Rolling back to version 12 maintenance LTS https://nodejs.org/en/about/releases/
@Trott
Copy link
Member

Trott commented Jan 1, 2022

Node.js 14.18.2 uses libuv 1.42.0. It is likely fixed. (If someone can test and confirm that it's fixed, that would be helpful.)

Node.js 12.x is using 1.40.0 so I would expect it to still have this bug.

@Trott
Copy link
Member

Trott commented Jan 1, 2022

Node.js 12.x is using 1.40.0 so I would expect it to still have this bug.

Hmmm, but 12.x was reported as not susceptible initially. But maybe it is now because of updates?

@santigimeno
Copy link
Member

This was fixed a long time ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding.
Projects
None yet
Development

No branches or pull requests

5 participants