On Wed, Jun 24, 2015 at 11:30:10PM +0300, Oleg Ponomarev wrote:
> Instance pinning is introduced in ganeti locations design document.
> It adds new instance tag of form htools:desiredlocation:x where x is
> a location tag of a desired primary node. This implemented by adding
> second component to the instance locationScore.
> The metric is extended with the following component:
> 
> * Sum of dissatisfied desired locations number among all cluster instances.
>   Instance desired location is dissatisfied when instance is assigned tag
>   of the form htools:desiredlocation:x but its primary node is not tagged
>   with x

Actually, with your implementation, you handled desired location tags the
same way as exclusion and location tags: if the cluster is tagged 
htools:desiredlocation:x
then tags starting with x are location tags. I quite like this approach!
But then the metric whould read

 * Sum of dissatisfied desired locations number among all cluster instances.
   An instance desired location is dissatisfied when the instance is assigned
   a desired-location tag x where the node is not tagged with the location tag
   x.

This need to update documentation also affects the patch with the 
design-document
and the man page.

> @@ -534,8 +535,19 @@ calcFmemOfflineOrForthcoming node allInstances =
>           . filter (not . Instance.usesMemory)
>           $ nodeInstances
>  
> +-- | Calculates the desired location score of an instance, given its primary 
> node.

Do not use lines with more than 80 characters.

> +getInstanceDsrdLocScore :: Node -- ^ the primary node of the instance
> +                        -> Instance.Instance -- ^ the original instance
> +                        -> Int -- ^ the instance with the desired location
> +                               -- score updated

As it is an Int, the description text probably should read "the desired 
location score".
It is not a modified version of the instance.

> +getInstanceDsrdLocScore p t =
> +        desiredLocationScore (Instance.dsrdLocTags t) (locationTags p)
> +  where desiredLocationScore instTags nodeTags =
> +          Set.size instTags - Set.size ( instTags `Set.intersection` 
> nodeTags )
> +        -- this way we get the number of unsatisfied desired locations

Rest LGTM. Thanks.

-- 
Klaus Aehlig
Google Germany GmbH, Dienerstr. 12, 80331 Muenchen
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores

Reply via email to