[tahoe-lafs-trac-stream] [tahoe-lafs] #1115: count-good-share-hosts is calculated incorrectly (post-repair says 10 hosts have good shares but there are only 4 hosts)

[tahoe-lafs]

#1115: count-good-share-hosts is calculated incorrectly (post-repair says 10 hosts
have good shares but there are only 4 hosts)
-------------------------+-------------------------------------------------
     Reporter:  zooko    |      Owner:  davidsarah
         Type:  defect   |     Status:  assigned
     Priority:  major    |  Milestone:  1.9.2
    Component:  code-    |    Version:  1.7.0
  frontend-web           |   Keywords:  usability easy repair servers-of-
   Resolution:           |  happiness upload review-needed
Launchpad Bug:           |
-------------------------+-------------------------------------------------

Comment (by davidsarah):

 The change to the computation of {{{count-good-share-hosts}}} at
 [source:1.9.2/src/allmydata/immutable/checker.py at 5499#L765] looks correct.
 A minor simplification is possible:
 {{{
 d['count-good-share-hosts'] = len([s for s in servers.keys() if
 servers[s]])
 }}}
 could just be
 {{{
 d['count-good-share-hosts'] = len(servers)
 }}}
 since the {{{servers}}} dict never includes any entries that are empty
 sets. I don't think this simplification is needed for 1.9.2.

 I spotted another problem, though: the value of {{{needs-rebalancing}}} is
 computed inconsistently between {{{checker.py}}} and {{{filenode.py}}}. In
 {{{checker.py}}} it is computed as:
 {{{
 # 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'])
 }}}
 In {{{filenode.py}}} it is computed as
 {{{
 prr.set_needs_rebalancing(len(sm) >= verifycap.total_shares)
 }}}
 where {{{len(sm)}}} is equal to {{{count-shares-good}}}. I don't
 understand this latter definition at all, it looks completely wrong. The
 definition in {{{checker.py}}} is more subtly wrong because it credits
 servers that only have duplicated shares as contributing to existing
 balance. The correct definition should be something like 'iff the
 happiness count is less than the number of uniquely-numbered good shares
 available'.

 I propose to change the definition in {{{filenode.py}}} to be consistent
 with {{{checker.py}}} in 1.9.2, and then change it to use the happiness
 count in 1.10, as part of #1784.

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