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

Use copy instead of rename for version folder on other file systems #1634

Closed
hashworks opened this issue Apr 13, 2015 · 14 comments

Comments

Projects
None yet
5 participants
@hashworks
Copy link

commented Apr 13, 2015

I've set the version folder to a different device for staged file versioning, now I'm getting the following errors:

Puller: final: rename /home/hashworks/filename /mnt/WDGREEN2TB/Backups/Cloud/filename: invalid cross-device link

Syncthing should use soft links here.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Apr 13, 2015

Symbolic link is just a reference to an existing file, it does not make a copy of it, hence if you were to delete the original file the link pointed to, the link would be invalid, and versioning would be useless (as it would be a bunch of invalid symlinks).

I guess what you are referring to is copying the file (rather than renaming it).

@hashworks hashworks changed the title Use soft links for version folder on other devices Use copy instead of hard links for version folder on other devices Apr 13, 2015

@hashworks

This comment has been minimized.

Copy link
Author

commented Apr 13, 2015

You're right, I've adjusted the title.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Apr 13, 2015

We should try renaming, but resort to copying I guess.

@calmh calmh changed the title Use copy instead of hard links for version folder on other devices Use copy instead of hard links for version folder on other file systems Apr 15, 2015

@calmh calmh modified the milestone: Unplanned Jan 1, 2016

@calmh calmh removed the help-wanted label Jan 1, 2016

@lkwg82

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

is this still relevant? switching from copy to rename seems a no brainer or on the other side a seldom edge case.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

It is still relevant when we cross device boundaries.

@calmh calmh changed the title Use copy instead of hard links for version folder on other file systems Use copy instead of rename for version folder on other file systems Apr 20, 2016

@lkwg82

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

so the other question is thisdifficult to change from copy to rename?

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

No, someone just needs to do it.

@lkwg82

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

Are we talking about?

osutil.Rename(filePath, dst)

err = osutil.Rename(filePath, dst)

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

Potentially have a osutil.Move which does a rename or a copy+delete if the rename fails. (Or RenameOrCopyAndDelete or whatever you want to call it)

@calmh

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

I'm not super keen on doing that in the general case. This would need to be a special thing for versioning, if we're going to do it. There should be no case where we need to do a copy+delete to rename a temp file to it's real name, and if we would do that we would need to do it with hashing and crap as well

@lkwg82

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

mhm, so you in general say that some logic in osutil.go need a review to maybe cleaned up? Because I see some lines in here, which do exactly this.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

commented Apr 20, 2016

Hence why I proposed osutil.Move which is used explicitly for versioning.

@calmh calmh removed this from the Unplanned (Contributions Welcome) milestone Feb 11, 2018

@endolith

This comment has been minimized.

Copy link

commented Jun 26, 2018

Ohhh, am I getting this error because I'm syncing the files to a https://github.com/trapexit/mergerfs drivepool made up of multiple drives?

@calmh

This comment has been minimized.

Copy link
Member

commented Jun 26, 2018

AudriusButkevicius added a commit to AudriusButkevicius/syncthing that referenced this issue Feb 6, 2019

lib/versioner: Restore for all versioners, cross-device support
Fixes syncthing#4631
Fixes syncthing#4586
Fixes syncthing#1634
Fixes syncthing#5338
Fixes syncthing#5419

This also:
1. Forces all versioners to use the same tagging, meaning that trash can versioner will start adding timestamps (cleanup is mtime based so it's ok)
2. Removes Rename function which seems not to be used anywhere important anymore, and TryRename is now RenameOrCopy
3. Versioning (and ton of other stuff) should now happen across filesystems when possible.
4. Adds versioning support for untagged and old file.txt~2014... tagged files. (There was no other way, in order to support trashcan versioner)

AudriusButkevicius added a commit that referenced this issue Apr 28, 2019

lib/versioner: Restore for all versioners, cross-device support (#5514)
* lib/versioner: Restore for all versioners, cross-device support

Fixes #4631
Fixes #4586
Fixes #1634
Fixes #5338
Fixes #5419

@calmh calmh added this to the v1.1.3 milestone Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.