Sequentially issue write requests, process results concurrently#482
Merged
Merged
Conversation
Collaborator
|
Thanks @puellanivis ! Is the note about concurrent writes still valid after this PR? Do we still need to truncate the file after an error? |
Collaborator
Author
|
It’s definitely much less likely, but I think the standard defines that while things can potentially be handled by the server out-of-order, the responses returned have to be essentially identical to doing them in order. So, it’s probably unlikely that this concern would remain. |
Collaborator
|
I did a quick test and the patch seems ok for me, @ncw can you please provide your feedback? Thanks |
drakkan
approved these changes
Dec 9, 2021
Collaborator
Author
|
I’m going to go ahead and merge this. I would like to also address #468 before we cut any sort of release, though. Edit: I had linked to the wrong issue. |
ncw
added a commit
to rclone/rclone
that referenced
this pull request
Jan 14, 2022
This stops the SFTP library issuing out of order writes which fixes the problems uploading to `serve sftp` from the `sftp` backend. This was fixes upstream in this pull request: pkg/sftp#482 Fixes #5806
ncw
added a commit
to rclone/rclone
that referenced
this pull request
Mar 3, 2022
This stops the SFTP library issuing out of order writes which fixes the problems uploading to `serve sftp` from the `sftp` backend. This was fixes upstream in this pull request: pkg/sftp#482 Fixes #5806
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Supersedes #480
Originally, it was assumed that we could issue write requests in any arbitrary order, and while this holds and is correct, it can cause locality issues for servers. Issuing write requests sequentially means the server can efficiently handle the writes in order without needing to seek around the file so much.
So, this takes care to issue requests sequentially, and then handles the results concurrently as they come back.
Also, this changes
writeAtWithConcurrencyto issue sequential writes as well, not justReadFromWithConcurrency