[tahoe-lafs-trac-stream] [tahoe-lafs] #1525: SFTP: handle download failures correctly

[tahoe-lafs]

#1525: SFTP: handle download failures correctly
-------------------------+-------------------------------------------------
     Reporter:           |      Owner:  davidsarah
  davidsarah             |     Status:  assigned
         Type:  defect   |  Milestone:  1.10.0
     Priority:  major    |    Version:  1.9.0a1
    Component:  code-    |   Keywords:  sftp hang error streaming
  frontend               |  performance
   Resolution:           |
Launchpad Bug:           |
-------------------------+-------------------------------------------------

Old description:

> Brian noticed that a Deferred was being dropped at
> [source:src/allmydata/frontends/sftpd.py at 5179#L676]. (Although this code
> changed when MDMF landed [5179], the immutable path of the
> [source:src/allmydata/frontends/sftpd.py at 5127#L675 previous code] also
> dropped a Deferred, at line 680.)
>
> It is incorrect to drop the Deferred, because that will cause SFTP read
> requests to hang if downloading the previous file contents fails. However
> fixing it while still allowing streaming reads is a bit tricky. The
> intention is that we download the file in parallel with accepting SFTP
> requests, and block each read request until the relevant part of the file
> is available (and prior writes have completed). If the download fails,
> then any SFTP read requests past the prefix that was read correctly
> should also fail. Also, if the file was written and not truncated, the
> SFTP close request should fail, at least in the case when the new
> contents depend on the part of the old contents that we weren't able to
> download.

New description:

 Brian noticed that a Deferred was being dropped at
 [source:src/allmydata/frontends/sftpd.py at 5179#L676]. (Although this code
 changed when MDMF landed [5179], the immutable path of the
 [source:src/allmydata/frontends/sftpd.py at 5127#L675 previous code] also
 dropped a Deferred, at line 680.)

 It is incorrect to drop the Deferred, because that will cause SFTP read
 requests to hang if downloading the previous file contents fails. However
 fixing it while still allowing streaming reads is a bit tricky. The
 intention is that we download the file in parallel with accepting SFTP
 requests, and block each read request until the relevant part of the file
 is available (and prior writes have completed). If the download fails,
 then any SFTP read requests past the prefix that was read correctly should
 also fail. Also, if the file was written and not truncated, the SFTP close
 request should fail, at least in the case when the new contents depend on
 the part of the old contents that we weren't able to download.

--

Comment (by warner):

 On this morning's dev call, we agreed that if the tests can be made to
 pass soon (in the next week), we can land this, else we'll kick it out to
 1.11. davidsarah is still on the hook to fix them.

-- 
Ticket URL: <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1525#comment:7>
tahoe-lafs <https://tahoe-lafs.org>
secure decentralized storage