[tahoe-lafs-trac-stream] [tahoe-lafs] #1669: ValueError (need more than 8 values to unpack) during mutable MDMF repair

[tahoe-lafs]

#1669: ValueError (need more than 8 values to unpack) during mutable MDMF repair
-------------------------+-------------------------------------------------
     Reporter:  vikarti  |      Owner:  zooko
         Type:  defect   |     Status:  assigned
     Priority:           |  Milestone:  1.9.2
  critical               |    Version:  1.9.1
    Component:  code-    |   Keywords:  mutable mdmf repair error
  mutable                |  regression preservation
   Resolution:           |
Launchpad Bug:           |
-------------------------+-------------------------------------------------

Comment (by zooko):

 Okay, I think I've finally figured out why this doesn't happen in the unit
 tests. There are no unit tests of mutable file repair that involve
 actually repairing damage!

 test_repairer is all immutable
 The one occurrence of "check_and_repair" in test_mutable says this:

 {{{
 def test_check_and_repair_readcap(self):
     # we can't currently repair from a mutable readcap: #625
 }}}

 So it doesnt' actually repair, just check, since it is a readcap.

 All the occurrences of check_and_repair in test_deepcheck appear to be
 running on perfectly healthy files, so now repairs are performed.

 Here is a patch which adds debugprintouts showing the trail of the
 problem. In practice, e.g. in vikarti's original post to this ticket, I
 believe that "xxx 1", then "xxx 2", then "xxx 3" will be encountered,
 leading to an exception. However when running the unit tests with this
 patch in place, you'll see xxx 1 and 2 but never "xxx 3", because there is
 no test of repair-which-actually-fixes-damage.

 So I currently believe that vikarti's bug will happen every single time
 you have an MDMF with damage and you repair it. Next step: to write a unit
 test that tests that belief! But first, sleep.

 Unsetting my ownership of this ticket in order to signal that I would be
 okay with (indeed, quite grateful for) someone else working on it while I
 sleep.

 {{{
 diff -u -r 1.9.1-pristine/src/allmydata/mutable/publish.py
 1.9.1/src/allmydata/mutable/publish.py
 --- 1.9.1-pristine/src/allmydata/mutable/publish.py     2012-01-12
 16:05:20.000000000 -0400
 +++ 1.9.1/src/allmydata/mutable/publish.py      2012-06-21
 00:13:01.051139281 -0300
 @@ -883,7 +883,8 @@

      def _record_verinfo(self):
          self.versioninfo = self.writers.values()[0].get_verinfo()
 -
 +        if len(self.versioninfo) != 9:
 +            print "xxx 1 %r" % (self.versioninfo,)

      def _connection_problem(self, f, writer):
          """
 @@ -1141,6 +1142,8 @@
          # If we get there, we know that responses correspond to placed
          # shares, and can safely execute these statements.
          if self.versioninfo:
 +            if len(self.versioninfo) != 9:
 +                print "xxx 2 self.versioninfo: %r" % (self.versioninfo,)
              self.log("wrote successfully: adding new share to servermap")
              self._servermap.add_new_share(peerid, writer.shnum,
                                            self.versioninfo, started)
 diff -u -r 1.9.1-pristine/src/allmydata/mutable/servermap.py
 1.9.1/src/allmydata/mutable/servermap.py
 --- 1.9.1-pristine/src/allmydata/mutable/servermap.py   2012-01-12
 16:05:20.000000000 -0400
 +++ 1.9.1/src/allmydata/mutable/servermap.py    2012-06-21
 00:13:13.631090217 -0300
 @@ -254,6 +254,8 @@
          versionmap = self.make_versionmap()
          recoverable_versions = set()
          for (verinfo, shares) in versionmap.items():
 +            if len(verinfo) != 9:
 +                print "xxx 3 %r" % (verinfo,)
              (seqnum, root_hash, IV, segsize, datalength, k, N, prefix,
               offsets_tuple) = verinfo
              shnums = set([shnum for (shnum, peerid, timestamp) in
 shares])
 }}}

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