tidy-ups for management of homeless shares #1127

New Issue

zooko · 2010-07-19T05:36:54Z

zooko commented

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

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

zooko added the

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

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

zooko self-assigned this 2010-07-19 05:36:54 +00:00

zooko commented

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

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

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

davidsarah commented

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

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

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

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-78713): > 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

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