Ticket #1512: fix-zero-length-reads.darcspatch

Tue Aug 30 14:01:41 PDT 2011  Kevan Carstensen <kevan@isnotajoke.com>
  * mutable/retrieve: handle the case where self._read_length is 0.
  
  Note that the downloader will still fetch a segment for a zero-length
  read, which is wasteful. Fixing that isn't specifically required to fix
  #1512, but it should probably be fixed before 1.9.

New patches:

[mutable/retrieve: handle the case where self._read_length is 0.
Kevan Carstensen <kevan@isnotajoke.com>**20110830210141
 Ignore-this: fceafbe485851ca53f2774e5a4fd8d30
 
 Note that the downloader will still fetch a segment for a zero-length
 read, which is wasteful. Fixing that isn't specifically required to fix
 #1512, but it should probably be fixed before 1.9.
] {
hunk ./src/allmydata/mutable/retrieve.py 407
             self._start_segment = 0
 
 
-        if self._read_length:
+        # If self._read_length is None, then we want to read the whole
+        # file. Otherwise, we want to read only part of the file, and
+        # need to figure out where to stop reading.
+        if self._read_length is not None:
             # our end segment is the last segment containing part of the
             # segment that we were asked to read.
             self.log("got read length %d" % self._read_length)
hunk ./src/allmydata/mutable/retrieve.py 414
-            end_data = self._offset + self._read_length
+            if self._read_length != 0:
+                end_data = self._offset + self._read_length
 
hunk ./src/allmydata/mutable/retrieve.py 417
-            # We don't actually need to read the byte at end_data, but
-            # the one before it.
-            end = (end_data - 1) // self._segment_size
+                # We don't actually need to read the byte at end_data,
+                # but the one before it.
+                end = (end_data - 1) // self._segment_size
 
hunk ./src/allmydata/mutable/retrieve.py 421
-            assert end < self._num_segments
-            self._last_segment = end
+                assert end < self._num_segments
+                self._last_segment = end
+            else:
+                self._last_segment = self._start_segment
             self.log("got end segment: %d" % self._last_segment)
         else:
             self._last_segment = self._num_segments - 1
hunk ./src/allmydata/test/test_mutable.py 3308
     def test_partial_read_ending_one_byte_after_segment_boundary(self):
         return self._test_partial_read(mathutil.next_multiple(128 * 1024, 3)-50, 51)
 
+    # XXX factor these into a single upload after they pass
     def test_partial_read_zero_length_at_start(self):
         return self._test_partial_read(0, 0)
 
hunk ./src/allmydata/test/test_mutable.py 3318
     def test_partial_read_zero_length_at_segment_boundary(self):
         return self._test_partial_read(mathutil.next_multiple(128 * 1024, 3), 0)
 
-    # XXX factor these into a single upload after they pass
-    _broken = "zero-length reads of mutable files don't work"
-    test_partial_read_zero_length_at_start.todo = _broken
-    test_partial_read_zero_length_in_middle.todo = _broken
-    test_partial_read_zero_length_at_segment_boundary.todo = _broken
 
     def _test_read_and_download(self, node, expected):
         d = node.get_best_readable_version()
}

Context:

[NEWS: added summary of all changes since 1.8.2. Needs editing.
Brian Warner <warner@lothar.com>**20110830163205
 Ignore-this: 273899b37a899fc6919b74572454b8b2
] 
[test_mutable.Update: only upload the files needed for each test. refs #1500
Brian Warner <warner@lothar.com>**20110829072717
 Ignore-this: 4d2ab4c7523af9054af7ecca9c3d9dc7
 
 This first step shaves 15% off the runtime: from 139s to 119s on my laptop.
 It also fixes a couple of places where a Deferred was being dropped, which
 would cause two tests to run in parallel and also confuse error reporting.
] 
[Let Uploader retain History instead of passing it into upload(). Fixes #1079.
Brian Warner <warner@lothar.com>**20110829063246
 Ignore-this: 3902c58ec12bd4b2d876806248e19f17
 
 This consistently records all immutable uploads in the Recent Uploads And
 Downloads page, regardless of code path. Previously, certain webapi upload
 operations (like PUT /uri/$DIRCAP/newchildname) failed to pass the History
 object and were left out.
] 
[Fix mutable publish/retrieve timing status displays. Fixes #1505.
Brian Warner <warner@lothar.com>**20110828232221
 Ignore-this: 4080ce065cf481b2180fd711c9772dd6
 
 publish:
 * encrypt and encode times are cumulative, not just current-segment
 
 retrieve:
 * same for decrypt and decode times
 * update "current status" to include segment number
 * set status to Finished/Failed when download is complete
 * set progress to 1.0 when complete
 
 More improvements to consider:
 * progress is currently 0% or 100%: should calculate how many segments are
   involved (remembering retrieve can be less than the whole file) and set it
   to a fraction
 * "fetch" time is fuzzy: what we want is to know how much of the delay is not
   our own fault, but since we do decode/decrypt work while waiting for more
   shares, it's not straightforward
] 
[Teach 'tahoe debug catalog-shares about MDMF. Closes #1507.
Brian Warner <warner@lothar.com>**20110828080931
 Ignore-this: 56ef2951db1a648353d7daac6a04c7d1
] 
[debug.py: remove some dead comments
Brian Warner <warner@lothar.com>**20110828074556
 Ignore-this: 40e74040dd4d14fd2f4e4baaae506b31
] 
[hush pyflakes
Brian Warner <warner@lothar.com>**20110828074254
 Ignore-this: bef9d537a969fa82fe4decc4ba2acb09
] 
[MutableFileNode.set_downloader_hints: never depend upon order of dict.values()
Brian Warner <warner@lothar.com>**20110828074103
 Ignore-this: caaf1aa518dbdde4d797b7f335230faa
 
 The old code was calculating the "extension parameters" (a list) from the
 downloader hints (a dictionary) with hints.values(), which is not stable, and
 would result in corrupted filecaps (with the 'k' and 'segsize' hints
 occasionally swapped). The new code always uses [k,segsize].
] 
[layout.py: fix MDMF share layout documentation
Brian Warner <warner@lothar.com>**20110828073921
 Ignore-this: 3f13366fed75b5e31b51ae895450a225
] 
[teach 'tahoe debug dump-share' about MDMF and offsets. refs #1507
Brian Warner <warner@lothar.com>**20110828073834
 Ignore-this: 3a9d2ef9c47a72bf1506ba41199a1dea
] 
[test_mutable.Version.test_debug: use splitlines() to fix buildslaves
Brian Warner <warner@lothar.com>**20110828064728
 Ignore-this: c7f6245426fc80b9d1ae901d5218246a
 
 Any slave running in a directory with spaces in the name was miscounting
 shares, causing the test to fail.
] 
[test_mutable.Version: exercise 'tahoe debug find-shares' on MDMF. refs #1507
Brian Warner <warner@lothar.com>**20110828005542
 Ignore-this: cb20bea1c28bfa50a72317d70e109672
 
 Also changes NoNetworkGrid to put shares in storage/shares/ .
] 
[test_mutable.py: oops, missed a .todo
Brian Warner <warner@lothar.com>**20110828002118
 Ignore-this: fda09ae86481352b7a627c278d2a3940
] 
[test_mutable: merge davidsarah's patch with my Version refactorings
warner@lothar.com**20110827235707
 Ignore-this: b5aaf481c90d99e33827273b5d118fd0
] 
[Make the immutable/read-only constraint checking for MDMF URIs identical to that for SSK URIs. refs #393
david-sarah@jacaranda.org**20110823012720
 Ignore-this: e1f59d7ff2007c81dbef2aeb14abd721
] 
[Additional tests for MDMF URIs and for zero-length files. refs #393
david-sarah@jacaranda.org**20110823011532
 Ignore-this: a7cc0c09d1d2d72413f9cd227c47a9d5
] 
[Additional tests for zero-length partial reads and updates to mutable versions. refs #393
david-sarah@jacaranda.org**20110822014111
 Ignore-this: 5fc6f4d06e11910124e4a277ec8a43ea
] 
[test_mutable.Version: factor out some expensive uploads, save 25% runtime
Brian Warner <warner@lothar.com>**20110827232737
 Ignore-this: ea37383eb85ea0894b254fe4dfb45544
] 
[SDMF: update filenode with correct k/N after Retrieve. Fixes #1510.
Brian Warner <warner@lothar.com>**20110827225031
 Ignore-this: b50ae6e1045818c400079f118b4ef48
 
 Without this, we get a regression when modifying a mutable file that was
 created with more shares (larger N) than our current tahoe.cfg . The
 modification attempt creates new versions of the (0,1,..,newN-1) shares, but
 leaves the old versions of the (newN,..,oldN-1) shares alone (and throws a
 assertion error in SDMFSlotWriteProxy.finish_publishing in the process).
 
 The mixed versions that result (some shares with e.g. N=10, some with N=20,
 such that both versions are recoverable) cause problems for the Publish code,
 even before MDMF landed. Might be related to refs #1390 and refs #1042.
] 
[layout.py: annotate assertion to figure out 'tahoe backup' failure
Brian Warner <warner@lothar.com>**20110827195253
 Ignore-this: 9b92b954e3ed0d0f80154fff1ff674e5
] 
[Add 'tahoe debug dump-cap' support for MDMF, DIR2-CHK, DIR2-MDMF. refs #1507.
Brian Warner <warner@lothar.com>**20110827195048
 Ignore-this: 61c6af5e33fc88e0251e697a50addb2c
 
 This also adds tests for all those cases, and fixes an omission in uri.py
 that broke parsing of DIR2-MDMF-Verifier and DIR2-CHK-Verifier.
] 
[MDMF: more writable/writeable consistentifications
warner@lothar.com**20110827190602
 Ignore-this: 22492a9e20c1819ddb12091062888b55
] 
[MDMF: s/Writable/Writeable/g, for consistency with existing SDMF code
warner@lothar.com**20110827183357
 Ignore-this: 9dd312acedbdb2fc2f7bef0d0fb17c0b
] 
[setup.cfg: remove no-longer-supported test_mac_diskimage alias. refs #1479
david-sarah@jacaranda.org**20110826230345
 Ignore-this: 40e908b8937322a290fb8012bfcad02a
] 
[test_mutable.Update: increase timeout from 120s to 400s, slaves are failing
Brian Warner <warner@lothar.com>**20110825230140
 Ignore-this: 101b1924a30cdbda9b2e419e95ca15ec
] 
[tests: fix check_memory test
zooko@zooko.com**20110825201116
 Ignore-this: 4d66299fa8cb61d2ca04b3f45344d835
 fixes #1503
] 
[TAG allmydata-tahoe-1.9.0a1
warner@lothar.com**20110825161122
 Ignore-this: 3cbf49f00dbda58189f893c427f65605
] 
Patch bundle hash:
e0d3445f7303246cf5a42da8cf674a38bb13da11