Re: [PATCH stable-2.16 2/2] Make executeRpcCall only compute rpcCallData once

['Iustin Pop' via ganeti-devel]

On 5 July 2016 at 11:44, 'Brian Foley' via ganeti-devel <
ganeti-devel@googlegroups.com> wrote:

> On Mon, Jul 04, 2016 at 08:32:12PM +0200, Iustin Pop wrote:
> > On 2016-07-04 16:57:24, Ganeti Development List wrote:
> > > This is important for distMCsAsyncTask, because currently every time
> > > config.data is updated, wconfd generates a separate copy of the Base64
> > > encoded, zlib compressed config.data payload string for the
> > > /upload_file_single call sent to each master candidate's noded.
> > >
> > > This patch causes it to generate one copy of the string and send that
> > > in parallel to all MCs.
> > >
> > > Signed-off-by: Brian Foley 
> >
> > LGTM, with the mention that the 3 different names for executing a rpc
> > call are too close together. I don't have good suggestions though, so
> > good as is.
> >
> > thanks!
> > iustin
>
> I agree, the names aren't wonderful and I've probably made them worse. I'll
> clean them up in a later patch.
>
> How about sendSingleRpc, sendMultipleRpcs, sendRpcsUsingData?
>

I though about such naming too but it's still misleading. All calls send
multiple RPCs, the different is whether it's the same payload or not.

What about sendSameRpc, sendVaryingRpcs, sendRpcsUsingData (in the latter
case it's up to the caller whether it's the same or not).

iustin

Re: Issue 1171 in ganeti: gnt-instance migrate error (vif-ganeti)

[ganeti]

Comment #2 on issue 1171 by valles...@gmail.com: gnt-instance migrate error  
(vif-ganeti)

https://code.google.com/p/ganeti/issues/detail?id=1171

domname=$(xm domname $domid)  => It should be 'xl' command -> No, it should  
be whatever the xen_cmd setting is for the cluster.


But right now it's hardcoded in the script, so it's borked.

--
You received this message because this project is configured to send all  
issue notifications to this address.

You may adjust your notification preferences at:
https://code.google.com/hosting/settings

Re: [PATCH master 3/6] Add bandwidth tags extraction and parsing

['Viktor Bachraty' via ganeti-devel]

On Tuesday, July 5, 2016 at 3:15:39 PM UTC+1, Даниил Лещёв wrote:
>
>
>
> 2016-07-05 15:50 GMT+03:00 Viktor Bachraty :
>
>>
>>
>> On Thursday, June 23, 2016 at 3:47:01 PM UTC+1, Даниил Лещёв wrote:
>>>
>>> From: Daniil Leshchev  
>>>
>>> This tags make possible to specify network bandwidth between nodes. 
>>> The syntax is "htools:bandwidth:a::b::N", there N is network speed 
>>> between 'a' and 'b' tags. 
>>> --- 
>>>  src/Ganeti/HTools/Tags.hs   | 60 
>>> + 
>>>  src/Ganeti/HTools/Tags/Constants.hs |  5  
>>>  2 files changed, 59 insertions(+), 6 deletions(-) 
>>>
>>> diff --git a/src/Ganeti/HTools/Tags.hs b/src/Ganeti/HTools/Tags.hs 
>>> index 6fd7ebb..010fd3d 100644 
>>> --- a/src/Ganeti/HTools/Tags.hs 
>>> +++ b/src/Ganeti/HTools/Tags.hs 
>>> @@ -39,17 +39,20 @@ module Ganeti.HTools.Tags 
>>>, getMigRestrictions 
>>>, getRecvMigRestrictions 
>>>, getLocations 
>>> +  , getBandwidth 
>>> +  , getBandwidthGraph 
>>> +  , inheritByPrefixes 
>>>) where 
>>>   
>>> -import Control.Monad (guard, (>=>)) 
>>> -import Data.List (isPrefixOf, isInfixOf, stripPrefix) 
>>> +import Control.Monad ((>=>)) 
>>> +import Data.List (isPrefixOf, stripPrefix) 
>>>  import Data.Maybe (mapMaybe) 
>>>  import qualified Data.Set as S 
>>>   
>>>  import qualified Ganeti.HTools.Node as Node 
>>>  import Ganeti.HTools.Tags.Constants ( standbyPrefix 
>>>  , migrationPrefix, 
>>> allowMigrationPrefix 
>>> -, locationPrefix ) 
>>> +, locationPrefix, bandwidthPrefix ) 
>>>   
>>>  -- * Predicates 
>>>   
>>> @@ -79,9 +82,7 @@ getMigRestrictions = getTags migrationPrefix 
>>>  -- the parts before and after. 
>>>  splitAtColons :: String -> Maybe (String, String) 
>>>   
>>> -splitAtColons (':':':':xs) = do 
>>> -  guard $ not ("::" `isInfixOf` xs) 
>>>
>>  
>> I think the guard was there to make splitAtColons return ("a", "b") from 
>> string "::a::b" instead of ("", "a::b"). Was there any good reason for this 
>> ?
>>
> But in your case splitAtColons will return Nothing instead of ("a", "b"), 
> because "::" is infix of "a::b".
> So I think the guard was there to make splitAtColons return Nothing, when 
> string has more than one entrance of "::" 
>

> In my case I need to split strings like "a::b::c". So I have removed this 
> guard in order to use this function and avoid code duplication.
>

What I meant here, have you verified, that this change won't break the use 
of splitAtColons in the original context - in migration tags ?
 

>  
>
>>  
>>
>>> -  return ("", xs) 
>>> +splitAtColons (':':':':xs) = return ("", xs) 
>>>   
>>>  splitAtColons (x:xs) = do 
>>>(as, bs) <- splitAtColons xs 
>>> @@ -107,3 +108,50 @@ getRecvMigRestrictions ctags ntags = 
>>>  -- from the node tags. 
>>>  getLocations :: [String] -> [String] -> S.Set String 
>>>  getLocations = getTags locationPrefix 
>>> + 
>>> +-- | Given the cluster tags extract the network bandwidth 
>>> +-- from a node tag. 
>>> +getBandwidth :: [String] -> [String] -> S.Set String 
>>> +getBandwidth = getTags bandwidthPrefix 
>>> + 
>>> +-- | Split given string on the  all "::" occurences 
>>> +splitToColonsList :: String -> [String] 
>>> +splitToColonsList str = 
>>> +  case splitAtColons str of 
>>> +Just (f, s) -> f : splitToColonsList s 
>>> +Nothing -> [str] 
>>>
>>  
>> Maybe splitAtColonsList ?
>>
> I have wrote this function in order to split strings like "a::b::c" to 
> ["a", "b", "c"]
> And as you can see I use splitAtColonsList inside.
>
> Should I rename this function or use another approach?
>
 
It was just a small nit, the "to" preposition sounds wierd to me in this 
context, splitAtColonsList sounds more natural.
 

>  
>
>>
>> + 
>>> +-- | Try to parse string into value 
>>> +maybeRead :: Read a => String -> Maybe a 
>>> +maybeRead s = case reads s of 
>>> +[(x,"")] -> Just x 
>>> +_-> Nothing 
>>> + 
>>> +-- | Extract bandwidth graph from cluster tags 
>>> +getBandwidthGraph :: [String] -> [(String, String, Int)] 
>>> +getBandwidthGraph ctags = 
>>> +  let unprefTags = mapMaybe (stripPrefix bandwidthPrefix) ctags 
>>> +  tupleList = mapMaybe (listToTuple . splitToColonsList) unprefTags 
>>> +  in mapMaybe parseInt tupleList 
>>> +  where parseInt (a, b, s) = case maybeRead s :: Maybe Int of 
>>> +  Just i -> Just (a, b, i) 
>>> +  Nothing -> Nothing 
>>> +listToTuple (a:b:c:[]) = Just (a, b, c) 
>>> +listToTuple _ = Nothing 
>>> + 
>>> +-- | Maybe extract string after first occurence of ":" return 
>>> +stripFirstPrefix :: String -> Maybe String 
>>> +stripFirstPrefix (':':_) = Just "" 
>>> +stripFirstPrefix (x:xs) = 
>>> +  case stripFirstPrefix xs of 
>>> +Just pref -> Just (x:pref) 
>>> +Nothing -> Nothing 
>>> +stripFirstPrefix _ = Nothing 
>>>
>>
>> 

Re: [PATCH master 5/6] Add long-time solutions filtering

['Viktor Bachraty' via ganeti-devel]

On Tuesday, July 5, 2016 at 2:33:28 PM UTC+1, Даниил Лещёв wrote:
>
>
>
> 2016-07-05 15:49 GMT+03:00 Viktor Bachraty :
>
>>
>>
>> On Thursday, June 23, 2016 at 3:47:05 PM UTC+1, Даниил Лещёв wrote:
>>>
>>> From: Daniil Leshchev  
>>>
>>> Change balancing functions in order to disallow long-time solutions, 
>>> whose K/N metric is less than algLongSolutionsFactor, 
>>> where K is solution's metric improvement and N is an estimated 
>>> time in seconds to perform this solution. 
>>> --- 
>>>  src/Ganeti/HTools/Cluster.hs | 103 
>>> +++ 
>>>  1 file changed, 85 insertions(+), 18 deletions(-) 
>>>
>>> diff --git a/src/Ganeti/HTools/Cluster.hs b/src/Ganeti/HTools/Cluster.hs 
>>> index 65746fd..e5b52cf 100644 
>>> --- a/src/Ganeti/HTools/Cluster.hs 
>>> +++ b/src/Ganeti/HTools/Cluster.hs 
>>> @@ -98,7 +98,7 @@ import Data.List ( nub 
>>>   , (\\) 
>>>   , sort 
>>>   , intercalate) 
>>> -import Data.Maybe (fromJust, fromMaybe, isJust, isNothing) 
>>> +import Data.Maybe (fromJust, fromMaybe, isJust, isNothing, mapMaybe) 
>>>  import Data.Ord (comparing) 
>>>  import Text.Printf (printf) 
>>>   
>>> @@ -115,7 +115,8 @@ import Ganeti.HTools.Cluster.AllocationSolution 
>>>  import Ganeti.HTools.Cluster.Evacuate ( EvacSolution(..), 
>>> emptyEvacSolution 
>>>, updateEvacSolution, 
>>> reverseEvacSolution 
>>>, nodeEvacInstance) 
>>> -import Ganeti.HTools.Cluster.Metrics (compCV, compClusterStatistics) 
>>> +import Ganeti.HTools.Cluster.Metrics (compCV, compClusterStatistics 
>>> +  , optimalCVScore) 
>>>  import Ganeti.HTools.Cluster.Moves (applyMoveEx) 
>>>  import Ganeti.HTools.Cluster.Utils (splitCluster, instancePriGroup 
>>> , availableGroupNodes, iMoveToJob) 
>>> @@ -430,12 +431,80 @@ checkInstanceMove opts nodes_idx ini_tbl@(Table nl 
>>> _ _ _) target = 
>>>  foldl' (checkSingleStep force ini_tbl target) best_migr_tbl 
>>> disk_moves 
>>>in (best_migr_tbl, best_tbl) 
>>>   
>>> +-- | The default network bandwidth value in Mbit/s 
>>> +defaultBandwidth :: Int 
>>> +defaultBandwidth = 100 
>>> + 
>>> +-- | Compute network bandwidth during given move in Mbit/s 
>>> +plcBandwidth :: Table -> Placement -> Int 
>>> +plcBandwidth (Table nl _ _ _) (_, pn, sn, move, _) = 
>>> +  fromMaybe defaultBandwidth (Node.calcBandwidthToNode src dst) 
>>> +  where getNode ndx = Container.find ndx nl 
>>> +(src, dst) = case move of 
>>> +  Failover -> (getNode pn, getNode sn) 
>>> +  FailoverToAny ndx -> (getNode pn, getNode ndx) 
>>> +  ReplacePrimary ndx -> (getNode pn, getNode ndx) 
>>> +  ReplaceSecondary ndx -> (getNode sn, getNode ndx) 
>>> +  ReplaceAndFailover ndx -> (getNode sn, getNode ndx) 
>>> +  FailoverAndReplace ndx -> (getNode sn, getNode ndx) 
>>> + 
>>> +-- | Compute the amount of data to be moved 
>>> +moveVolume :: IMove -> Instance.Instance -> Int 
>>> +moveVolume Failover inst = Instance.mem inst 
>>> +moveVolume (FailoverToAny _) inst = Instance.mem inst 
>>> +moveVolume _ inst = Instance.mem inst + Instance.dsk inst 
>>>
>>
>> I think it will be pretty interesting to see how will this behave in real 
>> life. As it was mentioned, even if the disk move might be bottlenecked by 
>> disk IO, this will penalize larger disk moves over smaller ones.
>>  
>>
>>> + 
>>> +-- | Compute the estimated time to perform move 
>>> +placementTimeEstimation :: Table -> Placement -> Double 
>>> +placementTimeEstimation tbl@(Table _ il _ _) 
>>> +plc@(idx, _, _, move, _) = 
>>> +  (fromIntegral volume * 8) / fromIntegral bandwidth 
>>> +  where volume = moveVolume move (Container.find idx il) 
>>> +bandwidth = plcBandwidth tbl plc 
>>> + 
>>> +-- | Compute the estimated time to perform solution 
>>> +solutionTimeEstimation :: Table -> Double 
>>> +solutionTimeEstimation fin_tbl@(Table _ _ _ plcs) = sum times 
>>> +where times = map (placementTimeEstimation fin_tbl) plcs 
>>> + 
>>> +-- | Filter long-time solutions without enough gain 
>>> +filterLongSolutions :: AlgorithmOptions 
>>> +-> Table 
>>> +-> Table 
>>> +-> Maybe Table 
>>> +filterLongSolutions opts ini_tbl fin_tbl = 
>>> +  let long_sol_th = fromIntegral $ algLongSolutionThreshold opts 
>>> +  long_sol_f = algLongSolutionsFactor opts 
>>> +  fin_t = solutionTimeEstimation fin_tbl 
>>> +  time_metric = fin_t / long_sol_th 
>>> +  Table nl _ ini_cv _ = ini_tbl 
>>> +  Table _ _ fin_cv _ = fin_tbl 
>>> +  opt_cv = optimalCVScore nl 
>>> +  improvement = (ini_cv - opt_cv) / (ini_cv - fin_cv - opt_cv) 
>>> +  in if long_sol_f == 0 || 
>>> +fin_t < long_sol_th || 
>>> +improvement >