tahoe-lafs/trac-2024-07-25

tidy-ups for management of homeless shares #1127

New issue

Closed

opened 2010-07-19 05:36:54 +00:00 by zooko · 9 comments

zooko commented

2010-07-19 05:36:54 +00:00

Owner

In #1118 we saw some potential improvements for the code managing homeless shares comment:120244 and comment:21:/tahoe-lafs/trac-2024-07-25/issues/8623. 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.

In #1118 we saw some potential improvements for the code managing homeless shares [comment:120244](/tahoe-lafs/trac-2024-07-25/issues/1118#issuecomment-120244) and comment:21:[/tahoe-lafs/trac-2024-07-25/issues/8623](/tahoe-lafs/trac-2024-07-25/issues/8623). 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.

tahoe-lafs added the

labels 2010-07-19 05:36:54 +00:00

tahoe-lafs added this to the undecided milestone 2010-07-19 05:36:54 +00:00

zooko commented

2010-07-19 05:39:15 +00:00

Author

Owner

Attachment homeless_shares_in_a_set.dpatch.txt (14568 bytes) added

**Attachment** homeless_shares_in_a_set.dpatch.txt (14568 bytes) added

homeless_shares_in_a_set.dpatch.txt

14 KiB

tahoe-lafs added

and removed

labels 2010-07-20 03:18:15 +00:00

zooko commented

2010-07-20 03:21:02 +00:00

Author

Owner

This merged cleanly with #1118 and it passes unit tests.

davidsarah commented

2010-07-23 00:46:05 +00:00

Author

Owner

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.

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.

zooko commented

2010-07-23 05:48:46 +00:00

Author

Owner

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?

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?

davidsarah commented

2010-08-02 04:29:00 +00:00

Author

Owner

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.

Replying to [zooko](/tahoe-lafs/trac-2024-07-25/issues/1127#issuecomment-120377): > 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.

zooko commented

2010-08-02 06:42:12 +00:00

Author

Owner

Below is a version which is deterministic (sorting the set whenever it needs to choose an item from it).

zooko commented

2010-08-02 06:42:37 +00:00

Author

Owner

Attachment homeless_shares_in_a_set.dpatch_v2.txt (5966 bytes) added

**Attachment** homeless_shares_in_a_set.dpatch_v2.txt (5966 bytes) added

homeless_shares_in_a_set.dpatch_v2.txt