On Tue, Jan 26, 2016 at 3:09 PM, 'Klaus Aehlig' via ganeti-devel <
[email protected]> wrote:

> Values output by /proc/stat are total values of time since the last reboot,
> in ticks that usually are 0.01s; so after roughly 250 CPU-days, the total
> time can no longer be represented as a 32-bit signed integer. Note that
> this
> can actually happen on a node with a high enough number of CPUs or long
> enough
> uptime. Therefore, take Integer as data type, to avoid being hit by
> overflows.
>
> Signed-off-by: Klaus Aehlig <[email protected]>
> ---
>  src/Ganeti/Cpu/LoadParser.hs         | 20 ++++++++++----------
>  src/Ganeti/Cpu/Types.hs              | 20 ++++++++++----------
>  src/Ganeti/DataCollectors/CPUload.hs | 10 +++++-----
>  src/Ganeti/DataCollectors/Types.hs   |  2 +-
>  4 files changed, 26 insertions(+), 26 deletions(-)
>
> diff --git a/src/Ganeti/Cpu/LoadParser.hs b/src/Ganeti/Cpu/LoadParser.hs
> index c321101..e2ffa01 100644
> --- a/src/Ganeti/Cpu/LoadParser.hs
> +++ b/src/Ganeti/Cpu/LoadParser.hs
> @@ -53,16 +53,16 @@ import Ganeti.Cpu.Types
>  oneCPUstatParser :: Parser CPUstat
>  oneCPUstatParser =
>    let nameP = stringP
> -      userP = numberP
> -      niceP = numberP
> -      systemP = numberP
> -      idleP = numberP
> -      iowaitP = numberP
> -      irqP = numberP
> -      softirqP = numberP
> -      stealP = numberP
> -      guestP = numberP
> -      guest_niceP = numberP
> +      userP = integerP
> +      niceP = integerP
> +      systemP = integerP
> +      idleP = integerP
> +      iowaitP = integerP
> +      irqP = integerP
> +      softirqP = integerP
> +      stealP = integerP
> +      guestP = integerP
> +      guest_niceP = integerP
>    in
>      CPUstat <$> nameP <*> userP <*> niceP <*> systemP <*> idleP <*>
> iowaitP
>              <*> irqP <*> softirqP <*> stealP <*> guestP <*> guest_niceP
> diff --git a/src/Ganeti/Cpu/Types.hs b/src/Ganeti/Cpu/Types.hs
> index 9fc31ef..5786435 100644
> --- a/src/Ganeti/Cpu/Types.hs
> +++ b/src/Ganeti/Cpu/Types.hs
> @@ -61,14 +61,14 @@ emptyCPUavgload = CPUavgload { cavCpuNumber = 1
>  -- | This is the format of the data parsed by the input file.
>  $(buildObject "CPUstat" "cs"
>    [ simpleField "name"       [t| String |]
> -  , simpleField "user"       [t| Int |]
> -  , simpleField "nice"       [t| Int |]
> -  , simpleField "system"     [t| Int |]
> -  , simpleField "idle"       [t| Int |]
> -  , simpleField "iowait"     [t| Int |]
> -  , simpleField "irq"        [t| Int |]
> -  , simpleField "softirq"    [t| Int |]
> -  , simpleField "steal"      [t| Int |]
> -  , simpleField "guest"      [t| Int |]
> -  , simpleField "guest_nice" [t| Int |]
> +  , simpleField "user"       [t| Integer |]
> +  , simpleField "nice"       [t| Integer |]
> +  , simpleField "system"     [t| Integer |]
> +  , simpleField "idle"       [t| Integer |]
> +  , simpleField "iowait"     [t| Integer |]
> +  , simpleField "irq"        [t| Integer |]
> +  , simpleField "softirq"    [t| Integer |]
> +  , simpleField "steal"      [t| Integer |]
> +  , simpleField "guest"      [t| Integer |]
> +  , simpleField "guest_nice" [t| Integer |]
>    ])
> diff --git a/src/Ganeti/DataCollectors/CPUload.hs
> b/src/Ganeti/DataCollectors/CPUload.hs
> index 65ac423..42f4e88 100644
> --- a/src/Ganeti/DataCollectors/CPUload.hs
> +++ b/src/Ganeti/DataCollectors/CPUload.hs
> @@ -5,7 +5,7 @@
>
>  {-
>
> -Copyright (C) 2013 Google Inc.
> +Copyright (C) 2013, 2016 Google Inc.
>  All rights reserved.
>

Shouldn't this be in a different patch?


>
>  Redistribution and use in source and binary forms, with or without
> @@ -111,17 +111,17 @@ dcReport colData =
>    in buildDCReport cpuLoadData
>
>  -- | Data stored by the collector in mond's memory.
> -type Buffer = Seq.Seq (ClockTime, [Int])
> +type Buffer = Seq.Seq (ClockTime, [Integer])
>
>  -- | Compute the load from a CPU.
> -computeLoad :: CPUstat -> Int
> +computeLoad :: CPUstat -> Integer
>  computeLoad cpuData =
>    csUser cpuData + csNice cpuData + csSystem cpuData
>    + csIowait cpuData + csIrq cpuData + csSoftirq cpuData
>    + csSteal cpuData + csGuest cpuData + csGuestNice cpuData
>
>  -- | Reads and Computes the load for each CPU.
> -dcCollectFromFile :: FilePath -> IO (ClockTime, [Int])
> +dcCollectFromFile :: FilePath -> IO (ClockTime, [Integer])
>  dcCollectFromFile inputFile = do
>    contents <-
>      ((E.try $ readFile inputFile) :: IO (Either IOError String)) >>=
> @@ -178,7 +178,7 @@ computeAverage s w ticks =
>              (timestampR, listR) = rightmost
>              workInWindow = zipWith (-) listL listR
>              timediff = timestampL - timestampR
> -            overall = fromInteger (timediff * ticks) / 1000000 :: Double
> +            overall = fromIntegral (timediff * ticks) / 1000000 :: Double
>          if overall > 0
>            then BT.Ok $ map (flip (/) overall . fromIntegral) workInWindow
>            else BT.Bad $ "Time covered by data is not sufficient."
> diff --git a/src/Ganeti/DataCollectors/Types.hs
> b/src/Ganeti/DataCollectors/Types.hs
> index 3bd31b3..20386ce 100644
> --- a/src/Ganeti/DataCollectors/Types.hs
> +++ b/src/Ganeti/DataCollectors/Types.hs
> @@ -145,7 +145,7 @@ instance JSON DCVersion where
>
>  -- | Type for the value field of the `CollectorMap` below.
>  data CollectorData =
> -  CPULoadData (Seq.Seq (ClockTime, [Int]))
> +  CPULoadData (Seq.Seq (ClockTime, [Integer]))
>    | InstanceCpuLoad (Map.Map String (Seq.Seq (ClockTime, Double)))
>
>  instance NFData ClockTime where
> --
> 2.7.0.rc3.207.g0ac5344
>
>
Rest LGTM

-- 
Lisa Velden
Software Engineer
[email protected]

Google Germany GmbH
Erika-Mann-Straße 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Reply via email to