From: Brian Warner <warner@lothar.com>
Date: Tue, 15 May 2012 04:53:49 +0000 (-0700)
Subject: immutable.CiphertextFileNode.check_and_repair: simplify for refactoring
X-Git-Url: https://git.rkrishnan.org/reliability?a=commitdiff_plain;h=17c5384f79ef7cc8f18f5c9a8bc037e364c3dbdc;p=tahoe-lafs%2Ftahoe-lafs.git

immutable.CiphertextFileNode.check_and_repair: simplify for refactoring

There were too many nested functions here, making some upcoming changes
too difficult, so let's refactor it first.
---

diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py
index 4afeeda3..c6f31f0a 100644
--- a/src/allmydata/immutable/filenode.py
+++ b/src/allmydata/immutable/filenode.py
@@ -89,68 +89,68 @@ class CiphertextFileNode:
 
 
     def check_and_repair(self, monitor, verify=False, add_lease=False):
-        verifycap = self._verifycap
-        storage_index = verifycap.storage_index
-        sb = self._storage_broker
-        servers = sb.get_connected_servers()
-        sh = self._secret_holder
-
-        c = Checker(verifycap=verifycap, servers=servers,
-                    verify=verify, add_lease=add_lease, secret_holder=sh,
+        c = Checker(verifycap=self._verifycap,
+                    servers=self._storage_broker.get_connected_servers(),
+                    verify=verify, add_lease=add_lease,
+                    secret_holder=self._secret_holder,
                     monitor=monitor)
         d = c.start()
-        def _maybe_repair(cr):
-            crr = CheckAndRepairResults(storage_index)
-            crr.pre_repair_results = cr
-            if cr.is_healthy():
-                crr.post_repair_results = cr
-                return defer.succeed(crr)
-            else:
-                crr.repair_attempted = True
-                crr.repair_successful = False # until proven successful
-                def _gather_repair_results(ur):
-                    assert IUploadResults.providedBy(ur), ur
-                    # clone the cr (check results) to form the basis of the
-                    # prr (post-repair results)
-                    prr = CheckResults(cr.uri, cr.storage_index)
-                    prr.data = copy.deepcopy(cr.data)
-
-                    servers_responding = set(prr.data['servers-responding'])
-                    sm = prr.data['sharemap']
-                    assert isinstance(sm, DictOfSets), sm
-                    for shnum, servers in ur.get_sharemap().items():
-                        for s in servers:
-                            sm.add(shnum, s.get_serverid())
-                            servers_responding.add(s.get_serverid())
-                    servers_responding = sorted(servers_responding)
-                    prr.data['servers-responding'] = servers_responding
-                    prr.data['count-shares-good'] = len(sm)
-                    good_hosts = len(reduce(set.union, sm.itervalues(), set()))
-                    prr.data['count-good-share-hosts'] = good_hosts
-                    is_healthy = bool(len(sm) >= verifycap.total_shares)
-                    is_recoverable = bool(len(sm) >= verifycap.needed_shares)
-                    prr.set_healthy(is_healthy)
-                    prr.set_recoverable(is_recoverable)
-                    crr.repair_successful = is_healthy
-                    prr.set_needs_rebalancing(len(sm) >= verifycap.total_shares)
-
-                    crr.post_repair_results = prr
-                    return crr
-                def _repair_error(f):
-                    # as with mutable repair, I'm not sure if I want to pass
-                    # through a failure or not. TODO
-                    crr.repair_successful = False
-                    crr.repair_failure = f
-                    return f
-                r = Repairer(self, storage_broker=sb, secret_holder=sh,
-                             monitor=monitor)
-                d = r.start()
-                d.addCallbacks(_gather_repair_results, _repair_error)
-                return d
-
-        d.addCallback(_maybe_repair)
+        d.addCallback(self._maybe_repair, monitor)
+        return d
+
+    def _maybe_repair(self, cr, monitor):
+        crr = CheckAndRepairResults(self._verifycap.storage_index)
+        crr.pre_repair_results = cr
+        if cr.is_healthy():
+            crr.post_repair_results = cr
+            return defer.succeed(crr)
+
+        crr.repair_attempted = True
+        crr.repair_successful = False # until proven successful
+        def _repair_error(f):
+            # as with mutable repair, I'm not sure if I want to pass
+            # through a failure or not. TODO
+            crr.repair_successful = False
+            crr.repair_failure = f
+            return f
+        r = Repairer(self, storage_broker=self._storage_broker,
+                     secret_holder=self._secret_holder,
+                     monitor=monitor)
+        d = r.start()
+        d.addCallbacks(self._gather_repair_results, _repair_error,
+                       callbackArgs=(cr, crr,))
         return d
 
+    def _gather_repair_results(self, ur, cr, crr):
+        assert IUploadResults.providedBy(ur), ur
+        # clone the cr (check results) to form the basis of the
+        # prr (post-repair results)
+        prr = CheckResults(cr.uri, cr.storage_index)
+        prr.data = copy.deepcopy(cr.data)
+
+        servers_responding = set(prr.data['servers-responding'])
+        sm = prr.data['sharemap']
+        assert isinstance(sm, DictOfSets), sm
+        for shnum, servers in ur.get_sharemap().items():
+            for s in servers:
+                sm.add(shnum, s.get_serverid())
+                servers_responding.add(s.get_serverid())
+        servers_responding = sorted(servers_responding)
+        prr.data['servers-responding'] = servers_responding
+        prr.data['count-shares-good'] = len(sm)
+        good_hosts = len(reduce(set.union, sm.itervalues(), set()))
+        prr.data['count-good-share-hosts'] = good_hosts
+        verifycap = self._verifycap
+        is_healthy = bool(len(sm) >= verifycap.total_shares)
+        is_recoverable = bool(len(sm) >= verifycap.needed_shares)
+        prr.set_healthy(is_healthy)
+        prr.set_recoverable(is_recoverable)
+        crr.repair_successful = is_healthy
+        prr.set_needs_rebalancing(len(sm) >= verifycap.total_shares)
+
+        crr.post_repair_results = prr
+        return crr
+
     def check(self, monitor, verify=False, add_lease=False):
         verifycap = self._verifycap
         sb = self._storage_broker