[tahoe-lafs-trac-stream] [tahoe-lafs] #1240: remove ResponseCache in favour of MDMFSlotReadProxy's cache

[tahoe-lafs]

#1240: remove ResponseCache in favour of MDMFSlotReadProxy's cache
------------------------------+---------------------------
     Reporter:  davidsarah    |      Owner:  amiller
         Type:  defect        |     Status:  new
     Priority:  major         |  Milestone:  1.10.0
    Component:  code-mutable  |    Version:  1.8.0
   Resolution:                |   Keywords:  mutable cache
Launchpad Bug:                |
------------------------------+---------------------------

Old description:

> {{{MDMFSlotReadProxy}}} can already be initialised with share data which
> it uses in preference to fetching from the server. By exploiting this
> behaviour we can remove {{{ResponseCache}}} which does essentially the
> same thing.
>
> {{{ResponseCache}}} had no functional tests of how it is used (as noted
> in [ticket:1045#comment:39], deactivating the cache by commenting out the
> line that adds entries to it will currently cause all tests to pass). The
> {{{MDMFSlotReadProxy}}}  cache will require a test of this type.
>
> One way to test that the cache is working would be to download a file,
> then break all of the servers holding its shares, then download it again
> in order to check that the shares are retrieved from cache rather than
> from the servers.

New description:

 {{{MDMFSlotReadProxy}}} can already be initialised with share data which
 it uses in preference to fetching from the server. By exploiting this
 behaviour we can remove {{{ResponseCache}}} which does essentially the
 same thing.

 {{{ResponseCache}}} had no functional tests of how it is used (as noted in
 [ticket:1045#comment:39], deactivating the cache by commenting out the
 line that adds entries to it will currently cause all tests to pass). The
 {{{MDMFSlotReadProxy}}}  cache will require a test of this type.

 One way to test that the cache is working would be to download a file,
 then break all of the servers holding its shares, then download it again
 in order to check that the shares are retrieved from cache rather than
 from the servers.

--

Comment (by amiller):

 After several weeks, this mystery is mostly solved. In an earlier commit
 [https://github.com/amiller/tahoe-
 lafs/commit/ff8a0a593c6d6b8f1607bb908884cf233b69eaf3 ff8a0a], I introduced
 an error that prevented the cache from being populated correctly.

 The error was something along the lines of store(H(data), data) followed
 by get(H'(data))->data which resulted in the cache being less effective
 (but not incorrect). This loss of effectiveness was succesfully detected
 by the test_deepcheck_cachemisses in that commit.

 In a later commit [https://github.com/amiller/tahoe-
 lafs/commit/1c50a4d5c711513a22641223eaaa2d4b15ba9160 1c50a4d], I fixed
 this error, but in doing so broke three tests that expected a larger
 number of server reads (i.e., cache misses) during an file download. Very
 small files, such as 'content1' in these tests, are loaded entirely into
 the cache during the 'get blocks' protocol - the actual file download
 produces no additional reads.

 In the newest commit [https://github.com/amiller/tahoe-
 lafs/commit/8d68da2219b46d8dd02c3cf6537fe997220c5628 8d68da2] (which is
 rebased against a recent trunk) I applied the simplest fix to two of these
 tests, which is to increase the size of the test content past the initial
 cache fill. The third test, test_corrupt_all_block_hash_tree_late, is
 fixed by adding 'corrupt_early=True' to the corruption test. I am not
 entirely sure whether this changes the semantics of the test, so I leave
 that as a question for the next reviewer.

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