Opened at 2010-07-19T05:36:54Z
Closed at 2010-08-07T21:24:33Z
#1127 closed enhancement (fixed)
tidy-ups for management of homeless shares
| Reported by: | zooko | Owned by: | zooko |
|---|---|---|---|
| Priority: | minor | Milestone: | 1.8.0 |
| Component: | code-peerselection | Version: | 1.7.1 |
| Keywords: | cleanup reviewed | Cc: | kevan |
| Launchpad Bug: |
Description
In #1118 we saw some potential improvements for the code managing homeless shares comment:20:ticket:1118 and comment:21:ticket:1118. Also I thought that the homeless_shares attribute ought to be a set rather than a list.
Here's a patch which changes it from list to set. Untested! Also this patch doesn't have the bugfix for #1118.
Attachments (2)
Change History (11)
Changed at 2010-07-19T05:39:15Z by zooko
comment:1 Changed at 2010-07-20T03:18:15Z by davidsarah
- Keywords cleanup added
- Priority changed from major to minor
- Version changed from 1.7.0 to 1.7.1
comment:2 Changed at 2010-07-20T03:21:02Z by zooko
- Keywords review-needed added
comment:3 Changed at 2010-07-23T00:46:05Z by davidsarah
Doesn't this make the behaviour less deterministic, because
shares_to_ask = set(list(self.homeless_shares)[:num_shares])
will pick an arbitrary subset of num_shares elements, rather than the num_shares elements that were first added? The determinism of the current code is useful when debugging unit tests, at least.
comment:4 follow-up: ↓ 6 Changed at 2010-07-23T05:48:46Z by zooko
Hm, that's a good thing to think about. I do like determinism for the reasons you mention—for testing and debugging. I think that this will be deterministic since self.homeless_shares happens to contain small integers (share numbers) and since CPython sets happen to store small integers in order. Actually even if they weren't in order they would still probably be deterministic. Good enough?
comment:5 Changed at 2010-07-23T05:48:57Z by zooko
- Owner changed from zooko to davidsarah
comment:6 in reply to: ↑ 4 Changed at 2010-08-02T04:29:00Z by davidsarah
Replying to zooko:
Hm, that's a good thing to think about. I do like determinism for the reasons you mention—for testing and debugging. I think that this will be deterministic since self.homeless_shares happens to contain small integers (share numbers) and since CPython sets happen to store small integers in order. Actually even if they weren't in order they would still probably be deterministic. Good enough?
That is only true for current CPython. I'd prefer it to be deterministic per spec.
comment:7 Changed at 2010-08-02T06:42:12Z by zooko
Below is a version which is deterministic (sorting the set whenever it needs to choose an item from it).
Changed at 2010-08-02T06:42:37Z by zooko
comment:8 Changed at 2010-08-02T06:46:27Z by davidsarah
- Keywords reviewed added; review-needed removed
- Milestone changed from undecided to 1.8.0
- Owner changed from davidsarah to zooko
+1
comment:9 Changed at 2010-08-07T21:24:33Z by davidsarah
- Resolution set to fixed
- Status changed from new to closed
Applied in 69c48ebde6730c3a.
This merged cleanly with #1118 and it passes unit tests.