This improves on the previous change. Currently, the node and instance
lists shipped around during data loading are (again) association lists.
For instances it's not a big issue, but the node list is rewritten
continuously while we assign instances to nodes, and that is very slow.
The code was originally written for small (10-20 node) clusters, and
today with multinodes… :)
Rewriting to use Node.List/Instance.List makes a bit of a messy patch,
but it allows to remove some custom functions for assoc. list
processing, and also some custom unittests.
At the end, the load time is almost halved, and we spend time now just
in the string parsing code (which is, as we know, slow…).
---
Ganeti/HTools/Loader.hs | 37 ++++++++++++++-----------------------
Ganeti/HTools/Luxi.hs | 4 ++--
Ganeti/HTools/QC.hs | 45 +++++++++++++++++++--------------------------
Ganeti/HTools/Rapi.hs | 4 ++--
Ganeti/HTools/Simu.hs | 9 +++++----
Ganeti/HTools/Text.hs | 6 +++---
hscan.hs | 2 +-
7 files changed, 46 insertions(+), 61 deletions(-)
diff --git a/Ganeti/HTools/Loader.hs b/Ganeti/HTools/Loader.hs
index e1f35a0..dcaef80 100644
--- a/Ganeti/HTools/Loader.hs
+++ b/Ganeti/HTools/Loader.hs
@@ -37,9 +37,7 @@ module Ganeti.HTools.Loader
, Request(..)
) where
-import Data.Function (on)
import Data.List
-import Data.Maybe (fromJust)
import qualified Data.Map as M
import Text.Printf (printf)
@@ -93,35 +91,29 @@ lookupInstance kti inst =
-- | Given a list of elements (and their names), assign indices to them.
assignIndices :: (Element a) =>
[(String, a)]
- -> (NameAssoc, [(Int, a)])
+ -> (NameAssoc, Container.Container a)
assignIndices nodes =
let (na, idx_node) =
unzip . map (\ (idx, (k, v)) -> ((k, idx), (idx, setIdx v idx)))
. zip [0..] $ nodes
- in (M.fromList na, idx_node)
-
--- | Assoc element comparator
-assocEqual :: (Eq a) => (a, b) -> (a, b) -> Bool
-assocEqual = (==) `on` fst
+ in (M.fromList na, Container.fromAssocList idx_node)
-- | For each instance, add its index to its primary and secondary nodes.
-fixNodes :: [(Ndx, Node.Node)]
+fixNodes :: Node.List
-> Instance.Instance
- -> [(Ndx, Node.Node)]
+ -> Node.List
fixNodes accu inst =
let
pdx = Instance.pNode inst
sdx = Instance.sNode inst
- pold = fromJust $ lookup pdx accu
+ pold = Container.find pdx accu
pnew = Node.setPri pold inst
- ac1 = deleteBy assocEqual (pdx, pold) accu
- ac2 = (pdx, pnew):ac1
+ ac2 = Container.add pdx pnew accu
in
if sdx /= Node.noSecondary
- then let sold = fromJust $ lookup sdx accu
+ then let sold = Container.find sdx accu
snew = Node.setSec sold inst
- ac3 = deleteBy assocEqual (sdx, sold) ac2
- in (sdx, snew):ac3
+ in Container.add sdx snew ac2
else ac2
-- | Remove non-selected tags from the exclusion list
@@ -168,11 +160,11 @@ commonSuffix nl il =
mergeData :: [(String, DynUtil)] -- ^ Instance utilisation data
-> [String] -- ^ Exclusion tags
-> [String] -- ^ Untouchable instances
- -> (Node.AssocList, Instance.AssocList, [String])
+ -> (Node.List, Instance.List, [String])
-- ^ Data from backends
-> Result (Node.List, Instance.List, [String])
-mergeData um extags exinsts (nl, il, tags) =
- let il2 = Container.fromAssocList il
+mergeData um extags exinsts (nl, il2, tags) =
+ let il = Container.elems il2
il3 = foldl' (\im (name, n_util) ->
case Container.findByName im name of
Nothing -> im -- skipping unknown instance
@@ -184,10 +176,9 @@ mergeData um extags exinsts (nl, il, tags) =
il4 = Container.map (filterExTags allextags .
updateMovable exinsts) il3
nl2 = foldl' fixNodes nl (Container.elems il4)
- nl3 = Container.fromAssocList
- (map (\ (k, v) -> (k, Node.buildPeers v il4)) nl2)
- node_names = map (Node.name . snd) nl
- inst_names = map (Instance.name . snd) il
+ nl3 = Container.map (\node -> Node.buildPeers node il4) nl2
+ node_names = map Node.name (Container.elems nl)
+ inst_names = map Instance.name il
common_suffix = longestDomain (node_names ++ inst_names)
snl = Container.map (computeAlias common_suffix) nl3
sil = Container.map (computeAlias common_suffix) il4
diff --git a/Ganeti/HTools/Luxi.hs b/Ganeti/HTools/Luxi.hs
index 24441b3..cc80908 100644
--- a/Ganeti/HTools/Luxi.hs
+++ b/Ganeti/HTools/Luxi.hs
@@ -162,7 +162,7 @@ readData master =
)
parseData :: (Result JSValue, Result JSValue, Result JSValue)
- -> Result (Node.AssocList, Instance.AssocList, [String])
+ -> Result (Node.List, Instance.List, [String])
parseData (nodes, instances, cinfo) = do
node_data <- nodes >>= getNodes
let (node_names, node_idx) = assignIndices node_data
@@ -173,5 +173,5 @@ parseData (nodes, instances, cinfo) = do
-- | Top level function for data loading
loadData :: String -- ^ Unix socket to use as source
- -> IO (Result (Node.AssocList, Instance.AssocList, [String]))
+ -> IO (Result (Node.List, Instance.List, [String]))
loadData master = readData master >>= return . parseData
diff --git a/Ganeti/HTools/QC.hs b/Ganeti/HTools/QC.hs
index a94b4a1..b4ab4b0 100644
--- a/Ganeti/HTools/QC.hs
+++ b/Ganeti/HTools/QC.hs
@@ -110,7 +110,7 @@ makeSmallCluster node count =
let fn = Node.buildPeers node Container.empty
namelst = map (\n -> (Node.name n, n)) (replicate count fn)
(_, nlst) = Loader.assignIndices namelst
- in Container.fromAssocList nlst
+ in nlst
-- | Checks if a node is "big" enough
isNodeBig :: Node.Node -> Int -> Bool
@@ -448,11 +448,12 @@ prop_Text_Load_Instance name mem dsk vcpus status pnode
snode pdx sdx =
ndx = if null snode
then [(pnode, pdx)]
else [(pnode, pdx), (snode, rsdx)]
+ nl = Data.Map.fromList ndx
tags = ""
- inst = Text.loadInst ndx
+ inst = Text.loadInst nl
[name, mem_s, dsk_s, vcpus_s, status, pnode, snode, tags]::
Maybe (String, Instance.Instance)
- fail1 = Text.loadInst ndx
+ fail1 = Text.loadInst nl
[name, mem_s, dsk_s, vcpus_s, status, pnode, pnode, tags]::
Maybe (String, Instance.Instance)
_types = ( name::String, mem::Int, dsk::Int
@@ -473,7 +474,8 @@ prop_Text_Load_Instance name mem dsk vcpus status pnode
snode pdx sdx =
isNothing fail1)
prop_Text_Load_InstanceFail ktn fields =
- length fields /= 8 ==> isNothing $ Text.loadInst ktn fields
+ length fields /= 8 ==> isNothing $ Text.loadInst nl fields
+ where nl = Data.Map.fromList ktn
prop_Text_Load_Node name tm nm fm td fd tc fo =
let conv v = if v < 0
@@ -822,35 +824,27 @@ testJobs =
-- | Loader tests
prop_Loader_lookupNode ktn inst node =
- isJust (Loader.lookupNode ktn inst node) == (node `elem` names)
- where names = map fst ktn
+ Loader.lookupNode nl inst node == Data.Map.lookup node nl
+ where nl = Data.Map.fromList ktn
prop_Loader_lookupInstance kti inst =
- isJust (Loader.lookupInstance kti inst) == (inst `elem` names)
- where names = map fst kti
-
-prop_Loader_lookupInstanceIdx kti inst =
- case (Loader.lookupInstance kti inst,
- findIndex (\p -> fst p == inst) kti) of
- (Nothing, Nothing) -> True
- (Just idx, Just ex) -> idx == snd (kti !! ex)
- _ -> False
-
-prop_Loader_assignIndices enames =
- length nassoc == length enames &&
- length kt == length enames &&
- (if not (null enames)
- then maximum (map fst kt) == length enames - 1
+ Loader.lookupInstance il inst == Data.Map.lookup inst il
+ where il = Data.Map.fromList kti
+
+prop_Loader_assignIndices nodes =
+ Data.Map.size nassoc == length nodes &&
+ Container.size kt == length nodes &&
+ (if not (null nodes)
+ then maximum (IntMap.keys kt) == length nodes - 1
else True)
- where (nassoc, kt) = Loader.assignIndices enames
- _types = enames::[(String, Node.Node)]
+ where (nassoc, kt) = Loader.assignIndices (map (\n -> (Node.name n, n))
nodes)
-- | Checks that the number of primary instances recorded on the nodes
-- is zero
prop_Loader_mergeData ns =
- let na = map (\n -> (Node.idx n, n)) ns
- in case Loader.mergeData [] [] [] (na, [], []) of
+ let na = Container.fromAssocList $ map (\n -> (Node.idx n, n)) ns
+ in case Loader.mergeData [] [] [] (na, Container.empty, []) of
Types.Bad _ -> False
Types.Ok (nl, il, _) ->
let nodes = Container.elems nl
@@ -861,7 +855,6 @@ prop_Loader_mergeData ns =
testLoader =
[ run prop_Loader_lookupNode
, run prop_Loader_lookupInstance
- , run prop_Loader_lookupInstanceIdx
, run prop_Loader_assignIndices
, run prop_Loader_mergeData
]
diff --git a/Ganeti/HTools/Rapi.hs b/Ganeti/HTools/Rapi.hs
index 37ac578..58df9bd 100644
--- a/Ganeti/HTools/Rapi.hs
+++ b/Ganeti/HTools/Rapi.hs
@@ -131,7 +131,7 @@ readData master = do
-- | Builds the cluster data from the raw Rapi content
parseData :: (Result String, Result String, Result String)
- -> Result (Node.AssocList, Instance.AssocList, [String])
+ -> Result (Node.List, Instance.List, [String])
parseData (node_body, inst_body, tags_body) = do
node_data <- node_body >>= getNodes
let (node_names, node_idx) = assignIndices node_data
@@ -142,5 +142,5 @@ parseData (node_body, inst_body, tags_body) = do
-- | Top level function for data loading
loadData :: String -- ^ Cluster or URL to use as source
- -> IO (Result (Node.AssocList, Instance.AssocList, [String]))
+ -> IO (Result (Node.List, Instance.List, [String]))
loadData master = readData master >>= return . parseData
diff --git a/Ganeti/HTools/Simu.hs b/Ganeti/HTools/Simu.hs
index c29f3f5..47618b9 100644
--- a/Ganeti/HTools/Simu.hs
+++ b/Ganeti/HTools/Simu.hs
@@ -6,7 +6,7 @@ This module holds the code for parsing a cluster description.
{-
-Copyright (C) 2009 Google Inc.
+Copyright (C) 2009, 2010 Google Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -35,6 +35,7 @@ import Text.Printf (printf)
import Ganeti.HTools.Utils
import Ganeti.HTools.Types
+import qualified Ganeti.HTools.Container as Container
import qualified Ganeti.HTools.Node as Node
import qualified Ganeti.HTools.Instance as Instance
@@ -52,7 +53,7 @@ parseDesc desc =
-- | Builds the cluster data from node\/instance files.
parseData :: String -- ^ Cluster description in text format
- -> Result (Node.AssocList, Instance.AssocList, [String])
+ -> Result (Node.List, Instance.List, [String])
parseData ndata = do
(cnt, disk, mem, cpu) <- parseDesc ndata
let nodes = map (\idx ->
@@ -62,10 +63,10 @@ parseData ndata = do
(fromIntegral cpu) False defaultGroupID
in (idx, Node.setIdx n idx)
) [1..cnt]
- return (nodes, [], [])
+ return (Container.fromAssocList nodes, Container.empty, [])
-- | Builds the cluster data from node\/instance files.
loadData :: String -- ^ Cluster description in text format
- -> IO (Result (Node.AssocList, Instance.AssocList, [String]))
+ -> IO (Result (Node.List, Instance.List, [String]))
loadData = -- IO monad, just for consistency with the other loaders
return . parseData
diff --git a/Ganeti/HTools/Text.hs b/Ganeti/HTools/Text.hs
index 242620e..b4f9b72 100644
--- a/Ganeti/HTools/Text.hs
+++ b/Ganeti/HTools/Text.hs
@@ -135,7 +135,7 @@ loadInst _ s = fail $ "Invalid/incomplete instance data: '"
++ show s ++ "'"
-- a supplied conversion function.
loadTabular :: (Monad m, Element a) =>
[String] -> ([String] -> m (String, a))
- -> m (NameAssoc, [(Int, a)])
+ -> m (NameAssoc, Container.Container a)
loadTabular lines_data convert_fn = do
let rows = map (sepSplit '|') lines_data
kerows <- mapM convert_fn rows
@@ -148,7 +148,7 @@ readData = readFile
-- | Builds the cluster data from text input.
parseData :: String -- ^ Text data
- -> Result (Node.AssocList, Instance.AssocList, [String])
+ -> Result (Node.List, Instance.List, [String])
parseData fdata = do
let flines = lines fdata
(nlines, ilines) = break null flines
@@ -163,5 +163,5 @@ parseData fdata = do
-- | Top level function for data loading
loadData :: String -- ^ Path to the text file
- -> IO (Result (Node.AssocList, Instance.AssocList, [String]))
+ -> IO (Result (Node.List, Instance.List, [String]))
loadData afile = readData afile >>= return . parseData
diff --git a/hscan.hs b/hscan.hs
index 06def66..ad503f6 100644
--- a/hscan.hs
+++ b/hscan.hs
@@ -89,7 +89,7 @@ fixSlash = map (\x -> if x == '/' then '_' else x)
-- | Generates serialized data from loader input
-processData :: Result (Node.AssocList, Instance.AssocList, [String])
+processData :: Result (Node.List, Instance.List, [String])
-> Result (Node.List, Instance.List, String)
processData input_data = do
(nl, il, _) <- input_data >>= Loader.mergeData [] [] []
--
1.7.2.3