Open Stack

On 09/08/2014 06:22 AM, Robert Collins wrote:
> On 8 September 2014 05:57, Nejc Saje <nsaje at redhat.com> wrote:
> \
>>> That generator API is pretty bad IMO - because it means you're very
>>> heavily dependent on gc and refcount behaviour to keep things clean -
>>> and there isn't (IMO) a use case for walking the entire ring from the
>>> perspective of an item. Whats the concern with having replicas a part
>>> of the API?
>>
>>
>> Because they don't really make sense conceptually. Hash ring itself doesn't
>> actually 'make' any replicas. The replicas parameter in the current Ironic
>> implementation is used solely to limit the amount of buckets returned.
>> Conceptually, that seems to me the same as take(<replicas>,
>> iterate_nodes()). I don't know python internals enough to know what problems
>> this would cause though, can you please clarify?
>
> I could see replicas being a parameter to a function call, but take(N,
> generator) has the same poor behaviour - generators in general that
> won't be fully consumed rely on reference counting to be freed.
> Sometimes thats absolutely the right tradeoff.

Ok, I can agree with it being a function call.

>
>
>>> its absolutely a partition of the hash space - each spot we hash a
>>> bucket onto is thats how consistent hashing works at all :)
>>
>>
>> Yes, but you don't assign the number of partitions beforehand, it depends on
>> the number of buckets. What you do assign is the amount of times you hash a
>> single bucket onto the ring, which is currently named 'replicas' in
>> Ceilometer code, but I suggested 'distribution_quality' or something
>> similarly descriptive in an earlier e-mail.
>
> I think you misunderstand the code. We do assign the number of
> partitions beforehand - its approximately fixed and independent of the
> number of buckets. More buckets == less times we hash each bucket.

Ah, your first sentence tipped me off that we're not actually speaking 
about the same code :). I'm talking about current Ceilometer code and 
the way the algorithm is described in the original paper. There, the 
number of times we hash a bucket doesn't depend on the number of buckets 
at all. The implementation with an array that Ironic used to have 
definitely needed to define the number of partition, but I don't see the 
need for it with the new approach as well. Why would you want to limit 
yourself to a fixed number of partitions if you're limited only by the 
output range of the hash function?

Cheers,
Nejc

>
> -Rob
>