[tahoe-lafs-trac-stream] [tahoe-lafs] #1395: error doing a check --verify on files bigger than about 1Gbyte

tahoe-lafs trac at tahoe-lafs.org
Thu Jun 23 14:39:09 PDT 2011


#1395: error doing a check --verify on files bigger than about 1Gbyte
-----------------------------+---------------------------------------------
     Reporter:  sickness     |      Owner:  zooko
         Type:  defect       |     Status:  new
     Priority:  major        |  Milestone:  1.9.0
    Component:  code-        |    Version:  1.8.2
  encoding                   |   Keywords:  memory verify error performance
   Resolution:               |
Launchpad Bug:               |
-----------------------------+---------------------------------------------
Changes (by warner):

 * keywords:  memory verify error performance review-needed => memory verify
     error performance
 * owner:  warner => zooko


Comment:

 I'm -1 on a test that leaves the code in a broken/patched state after an
 exception.. too much opportunity for confusion and hair-tearing debugging
 of
 other problems later. And I'm -1 on a test that depends deeply upon
 internals
 of a library that we don't include a copy of (note that I'm historically
 inconsistent about this one, q.v. Foolscap vs Twisted, but I'm trying to
 mend
 my ways).

 At first, I found your -low-tech-patching test hard to follow, but looking
 more closely, I'm growing fond of it. The original code isn't too hard to
 patch in the right way, so the mock subclass isn't too too weird. It might
 be
 nice if the new variable names were shorter (and since they only appear in
 the
 context of the !TooParallel unit test, they don't need to be as
 fully-qualified as if they were in the original {{{checker.py}}}). But
 after
 getting used to it, I think I'm ok with the way it stands.

 I'd be ok with simple partial fix to the cleanup-after-exception problem.
 Instead of using {{{setUp/tearDown}}} or pushing the whole middle of the
 test
 method into a sub-method, just put the {{{set_up_grid}}} part into a small
 function so you can get it into the Deferred chain. Something like:

 {{{
     d = defer.succeed(None)
     def _start(ign):
         self.set_up_grid(num_servers=4)
         c0 = self.g.clients[0]
         c0.DEFAULT_ENCODING_PARAMETERS = { "k": 1, "happy": 4, "n": 4,
                                            "max_segment_size": 5, }
         self.c0 = c0
         self.uris = {}
         DATA = "data" * 100 # 400/5 = 80 blocks
         return c0.upload(Data(DATA, convergence=""))
     d.addCallback(_start)
     def _do_check(ur):
         n = self.c0.create_node_from_uri(ur.uri)
         return n.check(Monitor(), verify=True)
     d.addCallback(_do_check)
 }}}

 Since the "critical region" where exceptions could cause problems doesn't
 start until after the {{{...checker.ValidatedReadBucketProxy =
 make_mock_VRBP}}}
 line, it's good enough to just capture the first chunk of the code after
 that
 in a function which is run from within the Deferred chain. Any exceptions
 in
 {{{set_up_grid}}} or {{{c0.upload}}} (which are fairly big and complex)
 will
 still get caught, so the cleanup can happen.

-- 
Ticket URL: <http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1395#comment:30>
tahoe-lafs <http://tahoe-lafs.org>
secure decentralized storage


More information about the tahoe-lafs-trac-stream mailing list