change CheckResults to use a fat set_data()

i.e. change set_data() to accept lots of parameters, instead of taking
a single dictionary with lots of keys. Also Convert all CheckResults
creators to use it.
This commit is contained in:
Brian Warner 2012-05-25 00:13:23 -07:00
parent d446897282
commit ccfcd4de37
6 changed files with 178 additions and 128 deletions

View File

@ -30,7 +30,27 @@ class CheckResults:
self.healthy = False
def set_needs_rebalancing(self, needs_rebalancing):
self.needs_rebalancing_p = bool(needs_rebalancing)
def set_data(self, data):
def set_data(self,
count_shares_needed, count_shares_expected,
count_shares_good, count_good_share_hosts,
count_recoverable_versions, count_unrecoverable_versions,
servers_responding, sharemap,
count_wrong_shares, list_corrupt_shares, count_corrupt_shares,
list_incompatible_shares, count_incompatible_shares):
data = {"count-shares-needed": count_shares_needed,
"count-shares-expected": count_shares_expected,
"count-shares-good": count_shares_good,
"count-good-share-hosts": count_good_share_hosts,
"count-recoverable-versions": count_recoverable_versions,
"count-unrecoverable-versions": count_unrecoverable_versions,
"servers-responding": servers_responding,
"sharemap": sharemap,
"count-wrong-shares": count_wrong_shares,
"list-corrupt-shares": list_corrupt_shares,
"count-corrupt-shares": count_corrupt_shares,
"list-incompatible-shares": list_incompatible_shares,
"count-incompatible-shares": count_incompatible_shares,
}
self._data = data
def set_summary(self, summary):
assert isinstance(summary, str) # should be a single string

View File

@ -739,9 +739,6 @@ class Checker(log.PrefixingLogMixin):
def _format_results(self, results):
SI = self._verifycap.get_storage_index()
cr = CheckResults(self._verifycap, SI)
d = {}
d['count-shares-needed'] = self._verifycap.needed_shares
d['count-shares-expected'] = self._verifycap.total_shares
verifiedshares = dictutil.DictOfSets() # {sharenum: set(serverid)}
servers = {} # {serverid: set(sharenums)}
@ -761,8 +758,7 @@ class Checker(log.PrefixingLogMixin):
if responded:
servers_responding.add(server_id)
d['count-shares-good'] = len(verifiedshares)
d['count-good-share-hosts'] = len([s for s in servers.keys() if servers[s]])
good_share_hosts = len([s for s in servers.keys() if servers[s]])
assert len(verifiedshares) <= self._verifycap.total_shares, (verifiedshares.keys(), self._verifycap.total_shares)
if len(verifiedshares) == self._verifycap.total_shares:
@ -776,29 +772,33 @@ class Checker(log.PrefixingLogMixin):
self._verifycap.total_shares))
if len(verifiedshares) >= self._verifycap.needed_shares:
cr.set_recoverable(True)
d['count-recoverable-versions'] = 1
d['count-unrecoverable-versions'] = 0
recoverable = 1
unrecoverable = 0
else:
cr.set_recoverable(False)
d['count-recoverable-versions'] = 0
d['count-unrecoverable-versions'] = 1
d['servers-responding'] = list(servers_responding)
d['sharemap'] = verifiedshares
# no such thing as wrong shares of an immutable file
d['count-wrong-shares'] = 0
d['list-corrupt-shares'] = corruptshare_locators
d['count-corrupt-shares'] = len(corruptshare_locators)
d['list-incompatible-shares'] = incompatibleshare_locators
d['count-incompatible-shares'] = len(incompatibleshare_locators)
recoverable = 0
unrecoverable = 1
# The file needs rebalancing if the set of servers that have at least
# one share is less than the number of uniquely-numbered shares
# available.
cr.set_needs_rebalancing(d['count-good-share-hosts'] < d['count-shares-good'])
cr.set_needs_rebalancing(good_share_hosts < len(verifiedshares))
cr.set_data(d)
cr.set_data(
count_shares_needed=self._verifycap.needed_shares,
count_shares_expected=self._verifycap.total_shares,
count_shares_good=len(verifiedshares),
count_good_share_hosts=good_share_hosts,
count_recoverable_versions=recoverable,
count_unrecoverable_versions=unrecoverable,
servers_responding=list(servers_responding),
sharemap=verifiedshares,
count_wrong_shares=0, # no such thing as wrong, for immutable
list_corrupt_shares=corruptshare_locators,
count_corrupt_shares=len(corruptshare_locators),
list_incompatible_shares=incompatibleshare_locators,
count_incompatible_shares=len(incompatibleshare_locators),
)
return cr

View File

@ -128,6 +128,7 @@ class CiphertextFileNode:
prr = CheckResults(cr.uri, cr.storage_index)
prr_data = copy.deepcopy(cr.get_data())
verifycap = self._verifycap
servers_responding = set(prr_data['servers-responding'])
sm = prr_data['sharemap']
assert isinstance(sm, DictOfSets), sm
@ -136,14 +137,25 @@ class CiphertextFileNode:
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
prr.set_data(prr_data)
verifycap = self._verifycap
is_healthy = bool(len(sm) >= verifycap.total_shares)
is_recoverable = bool(len(sm) >= verifycap.needed_shares)
prr.set_data(
count_shares_needed=verifycap.needed_shares,
count_shares_expected=verifycap.total_shares,
count_shares_good=len(sm),
count_good_share_hosts=good_hosts,
count_recoverable_versions=int(is_recoverable),
count_unrecoverable_versions=int(not is_recoverable),
servers_responding=list(servers_responding),
sharemap=sm,
count_wrong_shares=0, # no such thing as wrong, for immutable
list_corrupt_shares=prr_data["list-corrupt-shares"],
count_corrupt_shares=prr_data["count-corrupt-shares"],
list_incompatible_shares=prr_data["list-incompatible-shares"],
count_incompatible_shares=prr_data["count-incompatible-shares"],
)
prr.set_healthy(is_healthy)
prr.set_recoverable(is_recoverable)
crr.repair_successful = is_healthy

View File

@ -126,14 +126,11 @@ class MutableChecker:
self._monitor.raise_if_cancelled()
r.set_servermap(smap.copy())
healthy = True
data = {}
report = []
summary = []
vmap = smap.make_versionmap()
recoverable = smap.recoverable_versions()
unrecoverable = smap.unrecoverable_versions()
data["count-recoverable-versions"] = len(recoverable)
data["count-unrecoverable-versions"] = len(unrecoverable)
if recoverable:
report.append("Recoverable Versions: " +
@ -164,7 +161,6 @@ class MutableChecker:
report.append("Best Recoverable Version: " +
smap.summarize_version(best_version))
counters = self._count_shares(smap, best_version)
data.update(counters)
s = counters["count-shares-good"]
k = counters["count-shares-needed"]
N = counters["count-shares-expected"]
@ -180,45 +176,44 @@ class MutableChecker:
# find a k and N from somewhere
first = list(unrecoverable)[0]
# not exactly the best version, but that doesn't matter too much
data.update(self._count_shares(smap, first))
counters = self._count_shares(smap, first)
# leave needs_rebalancing=False: the file being unrecoverable is
# the bigger problem
else:
# couldn't find anything at all
data["count-shares-good"] = 0
data["count-shares-needed"] = 3 # arbitrary defaults
data["count-shares-expected"] = 10
data["count-good-share-hosts"] = 0
data["count-wrong-shares"] = 0
counters = {
"count-shares-good": 0,
"count-shares-needed": 3, # arbitrary defaults
"count-shares-expected": 10,
"count-good-share-hosts": 0,
"count-wrong-shares": 0,
}
corrupt_share_locators = []
if self.bad_shares:
data["count-corrupt-shares"] = len(self.bad_shares)
data["list-corrupt-shares"] = locators = []
report.append("Corrupt Shares:")
summary.append("Corrupt Shares:")
for (server, shnum, f) in sorted(self.bad_shares):
serverid = server.get_serverid()
locators.append( (serverid, self._storage_index, shnum) )
s = "%s-sh%d" % (server.get_name(), shnum)
if f.check(CorruptShareError):
ft = f.value.reason
else:
ft = str(f)
report.append(" %s: %s" % (s, ft))
summary.append(s)
p = (serverid, self._storage_index, shnum, f)
r.problems.append(p)
msg = ("CorruptShareError during mutable verify, "
"serverid=%(serverid)s, si=%(si)s, shnum=%(shnum)d, "
"where=%(where)s")
log.msg(format=msg, serverid=server.get_name(),
si=base32.b2a(self._storage_index),
shnum=shnum,
where=ft,
level=log.WEIRD, umid="EkK8QA")
else:
data["count-corrupt-shares"] = 0
data["list-corrupt-shares"] = []
for (server, shnum, f) in sorted(self.bad_shares):
serverid = server.get_serverid()
locator = (serverid, self._storage_index, shnum)
corrupt_share_locators.append(locator)
s = "%s-sh%d" % (server.get_name(), shnum)
if f.check(CorruptShareError):
ft = f.value.reason
else:
ft = str(f)
report.append(" %s: %s" % (s, ft))
summary.append(s)
p = (serverid, self._storage_index, shnum, f)
r.problems.append(p)
msg = ("CorruptShareError during mutable verify, "
"serverid=%(serverid)s, si=%(si)s, shnum=%(shnum)d, "
"where=%(where)s")
log.msg(format=msg, serverid=server.get_name(),
si=base32.b2a(self._storage_index),
shnum=shnum,
where=ft,
level=log.WEIRD, umid="EkK8QA")
sharemap = {}
for verinfo in vmap:
@ -227,14 +222,27 @@ class MutableChecker:
if shareid not in sharemap:
sharemap[shareid] = []
sharemap[shareid].append(server.get_serverid())
data["sharemap"] = sharemap
data["servers-responding"] = [s.get_serverid() for s in
list(smap.get_reachable_servers())]
servers_responding = [s.get_serverid() for s in
list(smap.get_reachable_servers())]
r.set_data(
count_shares_needed=counters["count-shares-needed"],
count_shares_expected=counters["count-shares-expected"],
count_shares_good=counters["count-shares-good"],
count_good_share_hosts=counters["count-good-share-hosts"],
count_recoverable_versions=len(recoverable),
count_unrecoverable_versions=len(unrecoverable),
servers_responding=servers_responding,
sharemap=sharemap,
count_wrong_shares=counters["count-wrong-shares"],
list_corrupt_shares=corrupt_share_locators,
count_corrupt_shares=len(corrupt_share_locators),
list_incompatible_shares=[],
count_incompatible_shares=0,
)
r.set_healthy(healthy)
r.set_recoverable(bool(recoverable))
r.set_needs_rebalancing(needs_rebalancing)
r.set_data(data)
if healthy:
r.set_summary("Healthy")
else:

View File

@ -67,23 +67,25 @@ class FakeCHKFileNode:
def check(self, monitor, verify=False, add_lease=False):
r = CheckResults(self.my_uri, self.storage_index)
data = {}
data["count-shares-needed"] = 3
data["count-shares-expected"] = 10
data["count-good-share-hosts"] = 10
data["count-wrong-shares"] = 0
nodeid = "\x00"*20
data["count-corrupt-shares"] = 0
data["list-corrupt-shares"] = []
data["sharemap"] = {1: [nodeid]}
data["servers-responding"] = [nodeid]
data["count-recoverable-versions"] = 1
data["count-unrecoverable-versions"] = 0
r.set_data(
count_shares_needed=3,
count_shares_expected=10,
count_shares_good=10,
count_good_share_hosts=10,
count_recoverable_versions=1,
count_unrecoverable_versions=0,
servers_responding=[nodeid],
sharemap={1: [nodeid]},
count_wrong_shares=0,
list_corrupt_shares=[],
count_corrupt_shares=0,
list_incompatible_shares=[],
count_incompatible_shares=0,
)
r.set_healthy(True)
r.set_recoverable(True)
data["count-shares-good"] = 10
r.problems = []
r.set_data(data)
r.set_needs_rebalancing(False)
return defer.succeed(r)
def check_and_repair(self, monitor, verify=False, add_lease=False):
@ -277,23 +279,25 @@ class FakeMutableFileNode:
def check(self, monitor, verify=False, add_lease=False):
r = CheckResults(self.my_uri, self.storage_index)
data = {}
data["count-shares-needed"] = 3
data["count-shares-expected"] = 10
data["count-good-share-hosts"] = 10
data["count-wrong-shares"] = 0
data["count-corrupt-shares"] = 0
data["list-corrupt-shares"] = []
nodeid = "\x00"*20
data["sharemap"] = {"seq1-abcd-sh0": [nodeid]}
data["servers-responding"] = [nodeid]
data["count-recoverable-versions"] = 1
data["count-unrecoverable-versions"] = 0
r.set_data(
count_shares_needed=3,
count_shares_expected=10,
count_shares_good=10,
count_good_share_hosts=10,
count_recoverable_versions=1,
count_unrecoverable_versions=0,
servers_responding=[nodeid],
sharemap={"seq1-abcd-sh0": [nodeid]},
count_wrong_shares=0,
list_corrupt_shares=[],
count_corrupt_shares=0,
list_incompatible_shares=[],
count_incompatible_shares=0,
)
r.set_healthy(True)
r.set_recoverable(True)
data["count-shares-good"] = 10
r.problems = []
r.set_data(data)
r.set_needs_rebalancing(False)
return defer.succeed(r)

View File

@ -85,19 +85,21 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
cr.set_healthy(True)
cr.set_needs_rebalancing(False)
cr.set_summary("groovy")
data = { "count-shares-needed": 3,
"count-shares-expected": 9,
"count-shares-good": 10,
"count-good-share-hosts": 11,
"count-corrupt-shares": 0,
"list-corrupt-shares": [],
"count-wrong-shares": 0,
data = { "count_shares_needed": 3,
"count_shares_expected": 9,
"count_shares_good": 10,
"count_good_share_hosts": 11,
"count_recoverable_versions": 1,
"count_unrecoverable_versions": 0,
"servers_responding": [],
"sharemap": {"shareid1": [serverid_1, serverid_f]},
"count-recoverable-versions": 1,
"count-unrecoverable-versions": 0,
"servers-responding": [],
"count_wrong_shares": 0,
"list_corrupt_shares": [],
"count_corrupt_shares": 0,
"list_incompatible_shares": [],
"count_incompatible_shares": 0,
}
cr.set_data(data)
cr.set_data(**data)
w = web_check_results.CheckResultsRenderer(c, cr)
html = self.render2(w)
@ -122,9 +124,9 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
cr.set_healthy(False)
cr.set_recoverable(False)
cr.set_summary("rather dead")
data["count-corrupt-shares"] = 1
data["list-corrupt-shares"] = [(serverid_1, u.get_storage_index(), 2)]
cr.set_data(data)
data["count_corrupt_shares"] = 1
data["list_corrupt_shares"] = [(serverid_1, u.get_storage_index(), 2)]
cr.set_data(**data)
html = self.render2(w)
s = self.remove_tags(html)
self.failUnlessIn("File Check Results for SI=2k6avp", s) # abbreviated
@ -187,38 +189,42 @@ class WebResultsRendering(unittest.TestCase, WebRenderingMixin):
pre_cr.set_recoverable(True)
pre_cr.set_needs_rebalancing(False)
pre_cr.set_summary("illing")
data = { "count-shares-needed": 3,
"count-shares-expected": 10,
"count-shares-good": 6,
"count-good-share-hosts": 7,
"count-corrupt-shares": 0,
"list-corrupt-shares": [],
"count-wrong-shares": 0,
data = { "count_shares_needed": 3,
"count_shares_expected": 10,
"count_shares_good": 6,
"count_good_share_hosts": 7,
"count_recoverable_versions": 1,
"count_unrecoverable_versions": 0,
"servers_responding": [],
"sharemap": {"shareid1": [serverid_1, serverid_f]},
"count-recoverable-versions": 1,
"count-unrecoverable-versions": 0,
"servers-responding": [],
"count_wrong_shares": 0,
"list_corrupt_shares": [],
"count_corrupt_shares": 0,
"list_incompatible_shares": [],
"count_incompatible_shares": 0,
}
pre_cr.set_data(data)
pre_cr.set_data(**data)
post_cr = check_results.CheckResults(u, u.get_storage_index())
post_cr.set_healthy(True)
post_cr.set_recoverable(True)
post_cr.set_needs_rebalancing(False)
post_cr.set_summary("groovy")
data = { "count-shares-needed": 3,
"count-shares-expected": 10,
"count-shares-good": 10,
"count-good-share-hosts": 11,
"count-corrupt-shares": 0,
"list-corrupt-shares": [],
"count-wrong-shares": 0,
data = { "count_shares_needed": 3,
"count_shares_expected": 10,
"count_shares_good": 10,
"count_good_share_hosts": 11,
"count_recoverable_versions": 1,
"count_unrecoverable_versions": 0,
"servers_responding": [],
"sharemap": {"shareid1": [serverid_1, serverid_f]},
"count-recoverable-versions": 1,
"count-unrecoverable-versions": 0,
"servers-responding": [],
"count_wrong_shares": 0,
"count_corrupt_shares": 0,
"list_corrupt_shares": [],
"list_incompatible_shares": [],
"count_incompatible_shares": 0,
}
post_cr.set_data(data)
post_cr.set_data(**data)
crr = check_results.CheckAndRepairResults(u.get_storage_index())
crr.pre_repair_results = pre_cr