On Fri, Jul 15, 2011 at 01:10:42PM +0100, Guido Trotter wrote: > On Fri, Jul 08, 2011 at 04:21:51PM +0200, Iustin Pop wrote: > > Currently, the tieredAlloc/iterateAlloc functions will not return > > until the allocation fails; this means unit-testing their > > functionality (e.g. that an instance can be allocated) is slow, since > > they will allocate all possible instances. > > > > This patch adds an optional limit that allows allocation to return > > early; this makes the cluster unittests twice as fast. > > LGTM, with comments: > > > --- > > htools/Ganeti/HTools/Cluster.hs | 35 +++++++++++++++++++++++++---------- > > htools/Ganeti/HTools/QC.hs | 4 ++-- > > htools/hspace.hs | 5 +++-- > > 3 files changed, 30 insertions(+), 14 deletions(-) > > > > diff --git a/htools/Ganeti/HTools/Cluster.hs > > b/htools/Ganeti/HTools/Cluster.hs > > index 0b0d271..b74addb 100644 > > --- a/htools/Ganeti/HTools/Cluster.hs > > +++ b/htools/Ganeti/HTools/Cluster.hs > > @@ -1036,45 +1036,60 @@ tryNodeEvac _ ini_nl ini_il mode idxs = > > -- | Recursively place instances on the cluster until we're out of space. > > iterateAlloc :: Node.List > > -> Instance.List > > + -> Maybe Int > > -> Instance.Instance > > -> AllocNodes > > -> [Instance.Instance] > > -> [CStats] > > -> Result AllocResult > > -iterateAlloc nl il newinst allocnodes ixes cstats = > > +iterateAlloc nl il limit newinst allocnodes ixes cstats = > > let depth = length ixes > > newname = printf "new-%d" depth::String > > newidx = length (Container.elems il) + depth > > newi2 = Instance.setIdx (Instance.setName newinst newname) newidx > > + newlimit = case limit of > > + Nothing -> limit > > + Just n -> Just (n-1) > > You can write this as newlimit = maybe Nothing Just . (flip (-) 1) > or maybe Nothing Just . ((+) (-1))
Mmm, I can fmap this to make it even more clear: fmap (flip (-1) 1) as Maybe is a functor instance, and we only need to work on the 'inside' value. > > + let newsol = Ok (errs, nl', il', ixes', cstats') > > + ixes_cnt = length ixes' > > + (stop, newlimit) = case limit of > > + Nothing -> (False, Nothing) > > + Just n -> (n <= ixes_cnt, > > + Just (n - ixes_cnt)) in > > Similar here, I guess. Not really, as the mutation function would be too unreadable? maybe (False, Nothing) (\n -> n <= ixes_cnt, Just (n - ixes_cnt)) limit thanks! iustin
