[PATCH conductor] RM 3396: Complex provider selection algorithms

Imre Farkas ifarkas at redhat.com
Thu Aug 2 12:38:26 UTC 2012


On 07/30/2012 11:23 AM, Jan Provaznik wrote:
> On 07/22/2012 12:16 PM, ifarkas at redhat.com wrote:
>> This patchset implements the initial framework
>>
>
> Hi Imre,
> this is not a proper review (Jozef is reviewing this patch AFAIK), here
> is a few things we discussed on Friday:
> - provider account's priority is not used when this patch is applied,
> it's replaced with priority groups. I'm fine with your suggestion that
> this account's priority can be used for setting wigh of account when
> choosing random account inside priority group.
> - provider_priority_groups controller is missing permissions checking
> for some actions
> - priority group is not destroyed when the pool is destroyed, priority
> element is not destroyed when account or provider is destroyed
> - config/initializers/provider_selection_strategies.rb: only
> lib/provider.rb could be loaded instead of the each loop
> - I wonder if priority groups shouldn't be associated with a pool family
> isntead of a pool since provider accounts are associated with pool
> family, not pool?

I think the ability to define priority groups on the pools level and 
giving the ability to define a more complex provider selection logic is 
plus. Imagine a situation where you have different pools for your 
different environment (e.g. staging, production) and you want to want to 
use providers with different availability/price ratio.
Does it make sense to you?

> - you use reverse priority ordering (bigger = higher) than what is used
> for account's priority (lower = higher), we should unify this
>
> In future (post 1.1) it would be nice to make provider_selection library
> more abstract (conductor independent) which can be reused in other apps.
> The conductor specific logic would be just inside strategies.
>
> The above are mostly minor notes, overall I think this is a good stuff
> and it can be pushed before 1.1.
>
> Great job, Jan

The other issues are fixed in rev. 2 (except the plans for post 1.1). 
Thanks for the review!
Imre



More information about the aeolus-devel mailing list