search

pkg / sftp

SFTP support for the go.crypto/ssh package
BSD 2-Clause "Simplified" License
1.5k stars 380 forks source link

Sequentially issue write requests, process results concurrently #482

Closed puellanivis closed 2 years ago

puellanivis commented 2 years ago

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 writeAtWithConcurrency to issue sequential writes as well, not just ReadFromWithConcurrency

drakkan commented 2 years ago

Thanks @puellanivis ! Is the note about concurrent writes still valid after this PR? Do we still need to truncate the file after an error?

puellanivis commented 2 years ago

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.

drakkan commented 2 years ago

I did a quick test and the patch seems ok for me, @ncw can you please provide your feedback? Thanks

puellanivis commented 2 years ago

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.