Invalid cross-device link

[siemdejong]

Over at Lightning-AI/pytorch-lightning#20270 (full traceback: Lightning-AI/pytorch-lightning#20270 (comment)) the following error is raised

OSError: [Errno 18] Invalid cross-device link: '/tmp/tmp657zwu1c' -> '/path/to/checkpoint.ckpt'

and during handling of the above exception

PermissionError: [Errno 1] Operation not permitted

if trying to save checkpoints via lightning's _atomic_save function that now uses a transaction whereas there were no issues before the transaction was used.

An example of a source and target device interaction is saving training checkpoints from a compute node to a slow storage device.

[siemdejong]

Interestingly, the checkpoint file is created at the destination with permissions

-rwxrwxrwx 1 nobody nogroup 573648714 Apr 25 08:39 '/path/to/checkpoint.ckpt'

[siemdejong]

Would it be acceptable to test for the source and destination to exist on the same device and if so, use shutil.copyfile instead of the default copy_function in shutil.move(src, dst, copy_function=shutil.copy2). And do you think this would solve the relevant issue?

shutil.copyfile does not copy metadata and file permissions, while shutil.copy2 does.

[martindurant]

test for the source and destination to exist on the same device

Is there any reasonable way to do that? Or would you advocate simply trying with copy2 and falling back in the case that files?

[siemdejong]

The following might be able to achieve this via os.stat(path).st_dev, but I'm not sure this if this is exactly what one would expect.

@lru_cache
def device_for_path(path: str) -> int:
    """
    Recursively find the st_dev of the nearest existing ancestor of `path`.
    Uses LRU cache to avoid repeated os.stat calls on the same paths.
    """
    if os.path.exists(path):
        return os.stat(path).st_dev
    return device_for_path(os.path.dirname(path))

def paths_on_same_device(path1, path2):
    """
    Return True if path1 and path2 would reside on the same filesystem device,
    even if they don’t actually exist yet.
    """
    return device_for_path(path1) == device_for_path(path2)

I would say check if paths are on the same device before using copy2 as it would do a copy (with copyfile) and after that fail with a copystat, leaving a copied file. But I'm not sure how much overhead this device check would have.

https://github.com/python/cpython/blob/e8cf3a1a641d6fa0bfa84a4f8363ff1e42abda30/Lib/shutil.py#L522-L523

Another approach then might be to manually apply copyfile first and then copystats second in LocalFileOpener.commit() and cache a possible error, but that would result in a file on a remote device with permissions that may differ from the permissions it would have on the local device, hence my question if that is acceptable.

The easiest solution would indeed be to just do shutil.move, let it decide to use copy2 in case of exdev error and catch the permission error copystat would throw, but this may be a breaking change.

The copy_function arg could also be exposed in some way so that packages using fsspec can decide for themselves.

[martindurant]

Your function I think would still result in many many calls when processing a lot of files, and I imagine is problematic on windows at least (if permissions work there at all!).

manually apply copyfile first and then copystats second in LocalFileOpener.commit()

It's actually probably the best we can do, since some mountable devices don't support a full permissioning model anyway. See also #1826 for applying the default "umask" to temporary files on commit.

[siemdejong]

Can I contribute with a PR here?

[martindurant]

Yes, please do