[bugfix] Sequentially issue read requests, process results concurrently #436
Conversation
|
| @@ -638,6 +622,30 @@ func (c *Client) close(handle string) error { | |||
| } | |||
| } | |||
|
|
|||
| func (c *Client) stat(path string) (*FileStat, error) { | |||
There was a problem hiding this comment.
It’s kind of odd that we were converting the FileStat from Stat into an os.FileInfo to extract the size, while the fstat didn’t. This aligns the two to work the same way, which simplifies the use point in WriteTo.
| // isRegular returns true if the mode describes a regular file. | ||
| func isRegular(mode uint32) bool { | ||
| return mode&S_IFMT == syscall.S_IFREG | ||
| } | ||
|
|
There was a problem hiding this comment.
This check is not really OS dependent, as it works through the internals of SSH flags. With the filexfer this can be done in a sufficiently generic way. I just put this here, as a convenience before the other filexfer implementation.
There was a problem hiding this comment.
I think this PR could solve the reported issues however our users seem too lazy to test and give us a feedback. @puellanivis if you agree I'll merge this and include it in a new release. This seems the only way to get a feedback
|
While I’m OK with merging this and including it in a new release, I would prefer to not issue a new release with the filexfer subpackage in the state it is (sure, it’s internal, so no one can be using it, but it should still be fixed before any release). Let’s go ahead and merge this, I’ll copy over the fixes for the filexfer subpackage from the other PR, and then we can merge both and release. Sound good? |
|
Good thank you. Shoud we do something more for #426 before the next release? This weekend I think I could find 1-2 hours if you want some help there |
|
Ah yes. Adding a |
Originally, it was assumed that we could issue read requests in any arbitrary order, and it would execute the same way. This holds for many cases, but fails for cases where the server might reset the file once an
io.EOFhas been sent back to the client.So, this takes care to issue requests sequentially, and then handle the results as the results come back. There is a bit of a change as well, since we can no longer just read in a chunk until it is completely read, we are stuck having to assume that each chunk read will be read to completion, exclusive from returning a short read and an EOF at the following read. This is only guaranteed per the standard for a regular file. Fortunately, we’re already stating the remote file to get the file length, so we just additionally check the permissions to make sure it’s a regular file.
I brought in the
pool.gochanges from thefilexferimplementation PR, so that I haveresChanPoolas this was useful for this PR.