From: Brian Warner Date: Fri, 25 May 2012 19:56:03 +0000 (-0700) Subject: CheckResults: pass IServer to sharemap=, but get_sharemap() returns serverids X-Git-Url: https://git.rkrishnan.org/specifications/%5B/%5D%20/%22doc.html/reliability?a=commitdiff_plain;h=a4c95609c780e84655830d932c08b9920020c559;p=tahoe-lafs%2Ftahoe-lafs.git CheckResults: pass IServer to sharemap=, but get_sharemap() returns serverids This changes all code which feeds CheckResults(sharemap=) to provide IServer instances, but CheckResults converts these to old-style serverids during output, so downstream code doesn't have to change yet. It adds a temporary get_new_sharemap(), which *does* return IServer instances, so the immutable repairer can build new CheckResults from an old one. This will go away when get_sharemap() is updated to return IServer (and downstream code is updated too). --- diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py index 18f5fa1f..bedda40d 100644 --- a/src/allmydata/check_results.py +++ b/src/allmydata/check_results.py @@ -1,8 +1,8 @@ from zope.interface import implements from allmydata.interfaces import ICheckResults, ICheckAndRepairResults, \ - IDeepCheckResults, IDeepCheckAndRepairResults, IURI -from allmydata.util import base32 + IDeepCheckResults, IDeepCheckAndRepairResults, IURI, IDisplayableServer +from allmydata.util import base32, dictutil class CheckResults: implements(ICheckResults) @@ -42,9 +42,9 @@ class CheckResults: for s in servers_responding: assert isinstance(s, str), s self._servers_responding = servers_responding - for shnum, serverids in sharemap.items(): - for serverid in serverids: - assert isinstance(serverid, str), serverid + for shnum, servers in sharemap.items(): + for server in servers: + assert IDisplayableServer.providedBy(server), server self._sharemap = sharemap self._count_wrong_shares = count_wrong_shares for (serverid, SI, shnum) in list_corrupt_shares: @@ -109,9 +109,19 @@ class CheckResults: return self._count_unrecoverable_versions def get_sharemap(self): + sharemap = dictutil.DictOfSets() + for shnum, servers in self._sharemap.items(): + for s in servers: + sharemap.add(shnum, s.get_serverid()) + return sharemap + def get_new_sharemap(self): + # this one returns IServers, even when get_sharemap returns serverids return self._sharemap def as_dict(self): + sharemap = {} + for shnum, servers in self._sharemap.items(): + sharemap[shnum] = sorted([s.get_serverid() for s in servers]) d = {"count-shares-needed": self._count_shares_needed, "count-shares-expected": self._count_shares_expected, "count-shares-good": self._count_shares_good, @@ -119,7 +129,7 @@ class CheckResults: "count-recoverable-versions": self._count_recoverable_versions, "count-unrecoverable-versions": self._count_unrecoverable_versions, "servers-responding": self._servers_responding, - "sharemap": self._sharemap, + "sharemap": sharemap, "count-wrong-shares": self._count_wrong_shares, "list-corrupt-shares": self._list_corrupt_shares, "count-corrupt-shares": self._count_corrupt_shares, diff --git a/src/allmydata/immutable/checker.py b/src/allmydata/immutable/checker.py index 8521616b..386584f1 100644 --- a/src/allmydata/immutable/checker.py +++ b/src/allmydata/immutable/checker.py @@ -739,7 +739,7 @@ class Checker(log.PrefixingLogMixin): def _format_results(self, results): SI = self._verifycap.get_storage_index() - verifiedshares = dictutil.DictOfSets() # {sharenum: set(serverid)} + verifiedshares = dictutil.DictOfSets() # {sharenum: set(server)} servers = {} # {serverid: set(sharenums)} corruptshare_locators = [] # (serverid, storageindex, sharenum) incompatibleshare_locators = [] # (serverid, storageindex, sharenum) @@ -749,7 +749,7 @@ class Checker(log.PrefixingLogMixin): server_id = server.get_serverid() servers.setdefault(server_id, set()).update(verified) for sharenum in verified: - verifiedshares.setdefault(sharenum, set()).add(server_id) + verifiedshares.setdefault(sharenum, set()).add(server) for sharenum in corrupt: corruptshare_locators.append((server_id, SI, sharenum)) for sharenum in incompatible: diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py index 27c1e11b..d06b5a64 100644 --- a/src/allmydata/immutable/filenode.py +++ b/src/allmydata/immutable/filenode.py @@ -129,13 +129,13 @@ class CiphertextFileNode: servers_responding = set(cr.get_servers_responding()) sm = DictOfSets() assert isinstance(cr.get_sharemap(), DictOfSets) - for shnum, serverids in cr.get_sharemap().items(): - for serverid in serverids: - sm.add(shnum, serverid) + for shnum, servers in cr.get_new_sharemap().items(): + for server in servers: + sm.add(shnum, server) for shnum, servers in ur.get_sharemap().items(): - for s in servers: - sm.add(shnum, s.get_serverid()) - servers_responding.add(s.get_serverid()) + for server in servers: + sm.add(shnum, server) + servers_responding.add(server.get_serverid()) servers_responding = sorted(servers_responding) good_hosts = len(reduce(set.union, sm.values(), set())) diff --git a/src/allmydata/mutable/checker.py b/src/allmydata/mutable/checker.py index d76c0ca0..c8404746 100644 --- a/src/allmydata/mutable/checker.py +++ b/src/allmydata/mutable/checker.py @@ -1,6 +1,6 @@ from allmydata.uri import from_string -from allmydata.util import base32, log +from allmydata.util import base32, log, dictutil from allmydata.check_results import CheckAndRepairResults, CheckResults from allmydata.mutable.common import MODE_CHECK, MODE_WRITE, CorruptShareError @@ -213,13 +213,11 @@ class MutableChecker: where=ft, level=log.WEIRD, umid="EkK8QA") - sharemap = {} + sharemap = dictutil.DictOfSets() for verinfo in vmap: for (shnum, server, timestamp) in vmap[verinfo]: shareid = "%s-sh%d" % (smap.summarize_version(verinfo), shnum) - if shareid not in sharemap: - sharemap[shareid] = [] - sharemap[shareid].append(server.get_serverid()) + sharemap.add(shareid, server) servers_responding = [s.get_serverid() for s in list(smap.get_reachable_servers())] if healthy: diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index 0cc2a918..6f19df14 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -14,6 +14,7 @@ from allmydata.interfaces import IMutableFileNode, IImmutableFileNode,\ MDMF_VERSION from allmydata.check_results import CheckResults, CheckAndRepairResults, \ DeepCheckResults, DeepCheckAndRepairResults +from allmydata.storage_client import StubServer from allmydata.mutable.layout import unpack_header from allmydata.mutable.publish import MutableData from allmydata.storage.mutable import MutableShareFile @@ -67,6 +68,7 @@ class FakeCHKFileNode: def check(self, monitor, verify=False, add_lease=False): nodeid = "\x00"*20 + s = StubServer(nodeid) r = CheckResults(self.my_uri, self.storage_index, healthy=True, recoverable=True, needs_rebalancing=False, @@ -77,7 +79,7 @@ class FakeCHKFileNode: count_recoverable_versions=1, count_unrecoverable_versions=0, servers_responding=[nodeid], - sharemap={1: [nodeid]}, + sharemap={1: [s]}, count_wrong_shares=0, list_corrupt_shares=[], count_corrupt_shares=0, @@ -279,6 +281,7 @@ class FakeMutableFileNode: def check(self, monitor, verify=False, add_lease=False): nodeid = "\x00"*20 + s = StubServer(nodeid) r = CheckResults(self.my_uri, self.storage_index, healthy=True, recoverable=True, needs_rebalancing=False, @@ -289,7 +292,7 @@ class FakeMutableFileNode: count_recoverable_versions=1, count_unrecoverable_versions=0, servers_responding=[nodeid], - sharemap={"seq1-abcd-sh0": [nodeid]}, + sharemap={"seq1-abcd-sh0": [s]}, count_wrong_shares=0, list_corrupt_shares=[], count_corrupt_shares=0, diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index 29e1df00..1946c6c5 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -78,6 +78,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): def test_check(self): c = self.create_fake_client() + sb = c.storage_broker serverid_1 = "\x00"*20 serverid_f = "\xff"*20 u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234) @@ -88,7 +89,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): "count_recoverable_versions": 1, "count_unrecoverable_versions": 0, "servers_responding": [], - "sharemap": {"shareid1": [serverid_1, serverid_f]}, + "sharemap": {"shareid1": [sb.get_stub_server(serverid_1), + sb.get_stub_server(serverid_f)]}, "count_wrong_shares": 0, "list_corrupt_shares": [], "count_corrupt_shares": 0, @@ -159,8 +161,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): 'count-unrecoverable-versions': 0, 'count-shares-needed': 3, 'sharemap': {"shareid1": - ["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "77777777777777777777777777777777"]}, + ["77777777777777777777777777777777", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]}, 'count-recoverable-versions': 1, 'list-corrupt-shares': [["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -185,6 +187,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): def test_check_and_repair(self): c = self.create_fake_client() + sb = c.storage_broker serverid_1 = "\x00"*20 serverid_f = "\xff"*20 u = uri.CHKFileURI("\x00"*16, "\x00"*32, 3, 10, 1234) @@ -196,7 +199,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): "count_recoverable_versions": 1, "count_unrecoverable_versions": 0, "servers_responding": [], - "sharemap": {"shareid1": [serverid_1, serverid_f]}, + "sharemap": {"shareid1": [sb.get_stub_server(serverid_1), + sb.get_stub_server(serverid_f)]}, "count_wrong_shares": 0, "list_corrupt_shares": [], "count_corrupt_shares": 0, @@ -217,7 +221,8 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): "count_recoverable_versions": 1, "count_unrecoverable_versions": 0, "servers_responding": [], - "sharemap": {"shareid1": [serverid_1, serverid_f]}, + "sharemap": {"shareid1": [sb.get_stub_server(serverid_1), + sb.get_stub_server(serverid_f)]}, "count_wrong_shares": 0, "count_corrupt_shares": 0, "list_corrupt_shares": [],