#2105 closed defect (fixed)
fix the definition of needs-rebalancing
Reported by: | daira | Owned by: | daira |
---|---|---|---|
Priority: | normal | Milestone: | 1.10.1 |
Component: | code-frontend-web | Version: | 1.9.1 |
Keywords: | check verify repair servers-of-happiness blocks-release test-needed | Cc: | |
Launchpad Bug: |
Description
Split from #1784 (also see 1115#comment:27):
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. # TODO: this may be wrong, see ticket #1115 comment:27 and ticket #1784. needs_rebalancing = bool(good_share_hosts < len(verifiedshares))
In filenode.py it is computed as
# TODO: this may be wrong, see ticket #1115 comment:27 and ticket #1784. needs_rebalancing = bool(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 filenode.py and checker.py to be consistent, and to both use the happiness-based definition above.
Change History (17)
comment:1 Changed at 2013-11-14T17:42:46Z by daira
- Component changed from unknown to code-frontend-web
- Milestone changed from undecided to 1.11.0
- Status changed from new to assigned
comment:2 Changed at 2013-11-14T17:49:05Z by daira
comment:3 Changed at 2013-11-14T17:50:32Z by daira
- Version changed from 1.10.0 to 1.9.1
comment:4 Changed at 2013-11-14T17:54:57Z by daira
- Keywords blocks-release added
comment:5 follow-up: ↓ 6 Changed at 2013-11-18T04:31:12Z by zooko
I think this ticket is actually about "needs-repair" rather than about "needs-rebalancing". I'm not 100% sure of the definition of "needs-rebalancing", but I think of it as something like "The file doesn't need repair (i.e. its Happiness level is already above the requirement), but it would be better to move some of the shares around to other servers.
Possible reasons that moving shares might be useful even when it doesn't improve Happiness:
- to optimize for faster-finding of the shares by moving them to servers earlier in the permuted ring, or
- to move shares from servers that are full to servers that are mostly empty (load-balancing total storage)
comment:6 in reply to: ↑ 5 Changed at 2013-12-05T15:53:29Z by daira
Replying to zooko:
I think this ticket is actually about "needs-repair" rather than about "needs-rebalancing". I'm not 100% sure of the definition of "needs-rebalancing", but I think of it as something like "The file doesn't need repair (i.e. its Happiness level is already above the requirement), but it would be better to move some of the shares around to other servers.
From docs/frontends/webapi.rst@0bebbe32#debugging-and-testing-features:
needs-rebalancing: (bool) This field is intended to be True iff reliability could be improved for this file by rebalancing, i.e. by moving some shares to other servers. It may be incorrect in some cases for Tahoe-LAFS up to and including v1.10, and its precise definition is expected to change.
Reliability can be improved when the shares-of-happiness count is less than shares.total. (In some cases, adding more servers may be necessary for an improvement, but I think that's fine.)
comment:7 Changed at 2013-12-05T16:33:08Z by daira
We decided to remove needs-rebalancing for 1.11.
comment:8 Changed at 2013-12-05T18:31:29Z by daira
- Keywords review-needed added
https://github.com/tahoe-lafs/tahoe-lafs/pull/74 implements this ticket (i.e. removes need-rebalancing) and #1784.
comment:9 Changed at 2013-12-05T18:31:40Z by daira
- Owner changed from daira to zooko
- Status changed from assigned to new
comment:10 Changed at 2014-03-19T16:35:18Z by remyroy
- Owner changed from zooko to remyroy
- Status changed from new to assigned
I'll review this patch/pull request.
comment:11 Changed at 2014-03-20T15:32:54Z by remyroy
- Keywords test-needed added; review-needed removed
- Owner changed from remyroy to daira
- Status changed from assigned to new
Review:
Everything seems great except for the use of the somewhat confusing "Happiness Count" expression. Count is fine for something countable like the number of good shares but it seems wrong for this. I think it would be preferable to use something like "Happiness Level", "Happiness Metric" or "Servers-of-Happiness Level".
Reassigning to daira for patch changes.
comment:12 Changed at 2014-03-20T16:08:48Z by daira
Zooko convinced me in the Tahoe hack hour to use "Happiness Level" (because it goes well with "happiness threshold").
comment:13 Changed at 2014-03-20T16:21:14Z by Daira Hopwood <daira@…>
- Resolution set to fixed
- Status changed from new to closed
"comments and a caveat in webapi.rst indicating that the needs-rebalancing field may be computed incorrectly" were added in [b06f8cd8d03a6239/trunk].