perlundq/yajsync

Optional atomic move to support differing file systems

Closed this issue · 2 comments

The comment of the method FileOps.atomicMove() already tells that this method might fail:

        // this can fail in many ways, maybe we can try to recover some of them?

If the file systems in both path arguments differ the option StandardCopyOption.ATOMIC_MOVE produces an error: java.nio.file.AtomicMoveNotSupportedException: Atomic move between providers is not supported

My proposal to recover this case in https://github.com/perlundq/yajsync/blob/master/src/main/com/github/perlundq/yajsync/util/FileOps.java#L270 :

        if (tempFile.getFileSystem().equals(path.getFileSystem())) {
            Files.move(tempFile, path, StandardCopyOption.ATOMIC_MOVE);
        } else {
            Files.move(tempFile, path);
        }

If the file systems in both path arguments differ the option StandardCopyOption.ATOMIC_MOVE produces an error: java.nio.file.AtomicMoveNotSupportedException: Atomic move between providers is not supported

I don't see how the file systems may end up differ between the arguments? The way this method currently is used, both args are always located in the same directory.

Ok, I now see in https://github.com/perlundq/yajsync/blob/master/src/main/com/github/perlundq/yajsync/session/Receiver.java#L671 the temp file is created on the same file system as the target file.
There may be FileSystems that are not adequate for fast "mergeDataFromPeerAndReplica" (so creating the temp file on another system like memory:/// could be an advantage). Anyway, I will run my tests first with the current implementation, thanks.