Use the same code path for local and remote syncs #572
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gdt
changed the title
gdt
changed the title
In the Copy module, local syncs do a direct copy and that's it. Remote syncs meanwhile check for already transferred files, check for partially transferred resumable files, can use the rsync algorithm and an external copyprog. Make local and remote syncs use the same code path. The functionality for both cases is now the same, but since the code was optimized for the remote case then there could be some optimization opportunities for local syncs. This is something this patch does not include.
tleedjarv
force-pushed
the
local-remote-copy-2
branch
from
658e59c
to
f3fde14
Compare
|
Rebased after more than 2 years. Having thought about it some more, I'm not convinced this is the right way to go. At least not without having some exceptions for the local sync anyway. Otherwise it can introduce more IO than what's done currently for local syncs (and remember: "local" can mean that sync happens on a network mount, which could even be a WAN, not just LAN). But the code is here for future reference. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This came out of the discussion at #571.
Closes #570
It is my third iteration. The first iteration was very similar to this, just more hackish and with more changes in
Remote. For second iteration, I wanted to make theRemotemodule simpler (could drop theregisterRootCmdWithConnectionfunction completely) but unfortunately that's not possible. It would mean the signatures of remote functions inCopywould have to change and that would break compatibility with 2.51. This third iteration is fully compatible with 2.51.The code path taken by local and remote syncs is now the same. Maybe there are some optimizations that make sense locally (for example, bigger buffers? then again, all buffers are currently quite small so this question does not have to come in here). This PR does not look at that side of things at all.
I have done some very brief testing locally and remotely with ssh and socket server and also with 2.51.
(As draft so it doesn't get accidentally merged. The change itself is finished as of now.)