From: Brian Warner Date: Fri, 25 May 2012 19:58:02 +0000 (-0700) Subject: CheckResults.get_sharemap() now returns IServers X-Git-Url: https://git.rkrishnan.org/about.html?a=commitdiff_plain;h=957a5315aa8f1a67efb2d72e57934518bfb25184;p=tahoe-lafs%2Ftahoe-lafs.git CheckResults.get_sharemap() now returns IServers Remove temporary get_new_sharemap(). --- diff --git a/src/allmydata/check_results.py b/src/allmydata/check_results.py index d0c2cda3..e9de13ca 100644 --- a/src/allmydata/check_results.py +++ b/src/allmydata/check_results.py @@ -2,7 +2,7 @@ from zope.interface import implements from allmydata.interfaces import ICheckResults, ICheckAndRepairResults, \ IDeepCheckResults, IDeepCheckAndRepairResults, IURI, IDisplayableServer -from allmydata.util import base32, dictutil +from allmydata.util import base32 class CheckResults: implements(ICheckResults) @@ -117,13 +117,6 @@ 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): diff --git a/src/allmydata/immutable/filenode.py b/src/allmydata/immutable/filenode.py index 578445fb..e248dc9b 100644 --- a/src/allmydata/immutable/filenode.py +++ b/src/allmydata/immutable/filenode.py @@ -129,7 +129,7 @@ class CiphertextFileNode: servers_responding = set(cr.get_new_servers_responding()) sm = DictOfSets() assert isinstance(cr.get_sharemap(), DictOfSets) - for shnum, servers in cr.get_new_sharemap().items(): + for shnum, servers in cr.get_sharemap().items(): for server in servers: sm.add(shnum, server) for shnum, servers in ur.get_sharemap().items(): diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index 4439d84e..f4fa2a9d 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -2191,12 +2191,11 @@ class ICheckResults(Interface): healthy file, this will be 0.""" def get_sharemap(): - """Return a dict mapping share identifier to list of serverids - (binary strings). This indicates which servers are holding which - shares. For immutable files, the shareid is an integer (the share - number, from 0 to N-1). For mutable files, it is a string of the form - 'seq%d-%s-sh%d', containing the sequence number, the roothash, and - the share number.""" + """Return a dict mapping share identifier to list of IServer objects. + This indicates which servers are holding which shares. For immutable + files, the shareid is an integer (the share number, from 0 to N-1). + For mutable files, it is a string of the form 'seq%d-%s-sh%d', + containing the sequence number, the roothash, and the share number.""" def get_summary(): """Return a string with a brief (one-line) summary of the results.""" diff --git a/src/allmydata/test/test_checker.py b/src/allmydata/test/test_checker.py index e1a57544..d83025ad 100644 --- a/src/allmydata/test/test_checker.py +++ b/src/allmydata/test/test_checker.py @@ -115,6 +115,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): self.failUnlessIn("Wrong Shares: 0", s) self.failUnlessIn("Recoverable Versions: 1", s) self.failUnlessIn("Unrecoverable Versions: 0", s) + self.failUnlessIn("Good Shares (sorted in share order): Share ID Nickname Node ID shareid1 peer-0 00000000 peer-f ffffffff", s) cr = check_results.CheckResults(u, u.get_storage_index(), healthy=False, recoverable=True, @@ -162,8 +163,7 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin): 'count-unrecoverable-versions': 0, 'count-shares-needed': 3, 'sharemap': {"shareid1": - ["77777777777777777777777777777777", - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"]}, + ["v0-00000000-long", "v0-ffffffff-long"]}, 'count-recoverable-versions': 1, 'list-corrupt-shares': [["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", diff --git a/src/allmydata/test/test_deepcheck.py b/src/allmydata/test/test_deepcheck.py index 86415b29..2ce8382e 100644 --- a/src/allmydata/test/test_deepcheck.py +++ b/src/allmydata/test/test_deepcheck.py @@ -293,8 +293,8 @@ class DeepCheckWebGood(DeepCheckBase, unittest.TestCase): sorted(self.g.get_all_serverids()), where) all_serverids = set() - for (shareid, serverids) in cr.get_sharemap().items(): - all_serverids.update(serverids) + for (shareid, servers) in cr.get_sharemap().items(): + all_serverids.update([s.get_serverid() for s in servers]) self.failUnlessEqual(sorted(all_serverids), sorted(self.g.get_all_serverids()), where) diff --git a/src/allmydata/web/check_results.py b/src/allmydata/web/check_results.py index a0ebf3d3..f1f3b3c8 100644 --- a/src/allmydata/web/check_results.py +++ b/src/allmydata/web/check_results.py @@ -6,7 +6,7 @@ from twisted.web import http, html from allmydata.web.common import getxmlfile, get_arg, get_root, WebError from allmydata.web.operations import ReloadMixin from allmydata.interfaces import ICheckAndRepairResults, ICheckResults -from allmydata.util import base32, idlib +from allmydata.util import base32, idlib, dictutil def json_check_counts(r): d = {"count-shares-good": r.get_share_counter_good(), @@ -20,9 +20,9 @@ def json_check_counts(r): in r.get_corrupt_shares() ], "servers-responding": [idlib.nodeid_b2a(serverid) for serverid in r.get_servers_responding()], - "sharemap": dict([(shareid, [idlib.nodeid_b2a(serverid) - for serverid in serverids]) - for (shareid, serverids) + "sharemap": dict([(shareid, + sorted([s.get_longname() for s in servers])) + for (shareid, servers) in r.get_sharemap().items()]), "count-wrong-shares": r.get_share_counter_wrong(), "count-recoverable-versions": r.get_version_counter_recoverable(), @@ -106,50 +106,49 @@ class ResultsBase: add("Wrong Shares", cr.get_share_counter_wrong()) - sharemap = [] - servers = {} + sharemap_data = [] + shares_on_server = dictutil.DictOfSets() # FIXME: The two tables below contain nickname-and-nodeid table column markup which is duplicated with each other, introducer.xhtml, and deep-check-results.xhtml. All of these (and any other presentations of nickname-and-nodeid) should be combined. for shareid in sorted(cr.get_sharemap().keys()): - serverids = cr.get_sharemap()[shareid] - for i,serverid in enumerate(serverids): - if serverid not in servers: - servers[serverid] = [] - servers[serverid].append(shareid) + servers = sorted(cr.get_sharemap()[shareid], + key=lambda s: s.get_longname()) + for i,s in enumerate(servers): + shares_on_server.add(s, shareid) shareid_s = "" if i == 0: shareid_s = shareid - nickname = sb.get_nickname_for_serverid(serverid) - sharemap.append(T.tr[T.td[shareid_s], - T.td[T.div(class_="nickname")[nickname], - T.div(class_="nodeid")[T.tt[base32.b2a(serverid)]]] - ]) + d = T.tr[T.td[shareid_s], + T.td[T.div(class_="nickname")[s.get_nickname()], + T.div(class_="nodeid")[T.tt[s.get_name()]]] + ] + sharemap_data.append(d) add("Good Shares (sorted in share order)", T.table()[T.tr[T.th["Share ID"], T.th(class_="nickname-and-peerid")[T.div["Nickname"], T.div(class_="nodeid")["Node ID"]]], - sharemap]) + sharemap_data]) add("Recoverable Versions", cr.get_version_counter_recoverable()) add("Unrecoverable Versions", cr.get_version_counter_unrecoverable()) # this table is sorted by permuted order - sb = c.get_storage_broker() permuted_servers = [s for s in sb.get_servers_for_psi(cr.get_storage_index())] - num_shares_left = sum([len(shares) for shares in servers.values()]) + num_shares_left = sum([len(shareids) + for shareids in shares_on_server.values()]) servermap = [] for s in permuted_servers: - nickname = s.get_nickname() - shareids = servers.get(s.get_serverid(), []) + shareids = list(shares_on_server.get(s, [])) shareids.reverse() shareids_s = [ T.tt[shareid, " "] for shareid in sorted(shareids) ] - servermap.append(T.tr[T.td[T.div(class_="nickname")[nickname], - T.div(class_="nodeid")[T.tt[s.get_name()]]], - T.td[shareids_s], - ]) + d = T.tr[T.td[T.div(class_="nickname")[s.get_nickname()], + T.div(class_="nodeid")[T.tt[s.get_name()]]], + T.td[shareids_s], + ] + servermap.append(d) num_shares_left -= len(shareids) if not num_shares_left: break