Seagate / halon

High availability solution
Apache License 2.0
1 stars 0 forks source link

[HALON-835] processKeepaliveTimeout: improve error message #1504

Closed 1468ca0b-2a64-4fb4-8e52-ea5806644b4c closed 5 years ago

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

Beautify error message of processKeepaliveTimeout

To make the error message more human-friendly added code to get time in seconds and milliseconds.

Resolves: HALON-835

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

Oh. Now we know the culprit! (Yours truly.)

I should had been more careful. Sorry for this mistake. It's good that we didn't let it into the wild.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

Done

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

Done.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

Done.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

ok

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

That day I had this doubt but did not have any input as such to cross verify the result. So just asking.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: prasanna46dk

just to be 100% sure. reply from you on HALON-835

λ> divMod (C.toNanoSecs ts `quot` 10000000) 1000
(-61891,765)

there are 7 zeros there. I thought this is done on purpose. since you showed a result on ghci.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

@prasanna46dk What remains is to rewrite the commit message.

Please fix the formatting of the commit message: remove leading dash from the subject line; add a blank line between subject line and commit message body. See How to Write a Git Commit Message.

Ping me when you have the text of the commit message ready, and I will show you how to massage this branch's commits into “ready for landing” state.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

  1. Check how difficult it would be to change the result type of timeSpecToSeconds and timeSpecToNanoSecs to Integer: :: TimeSpec -> Integer.

Nah, it isn't worth the trouble. (I've just tried.:)

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

Actually, we should use upstream toNanoSecs here.

-- | Convert 'TimeSpec' to nanoseconds.
timeSpecToNanoSecs :: TimeSpec -> Integer
timeSpecToNanoSecs (TimeSpec ts) = C.toNanoSecs ts
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

-- | Convert 'TimeSpec' to nanoseconds.
timeSpecToNanoSecs :: TimeSpec -> Int
timeSpecToNanoSecs (TimeSpec (C.TimeSpec sec nsec)) =
  fromIntegral $ sec * 10^(9 :: Int) + nsec
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

It is usually either printf or ++, rarely both.

1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

This definition is not actually needed.

    toPSFailed = TransitionTo . M0.PSFailed $ printf
                 "Keepalive timed out after %d.%d seconds" s ms
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

  1. The arithmetics is wrong: you should divide by 10^6, not by 10^7. (I recommend to use ^ operator to avoid this kind of errors.)
  2. tnsec variable name is misleading. This value is the number of remaining milli-seconds.
    (s, ms) = divMod (M0.timeSpecToNanoSecs ts `quot` 10^(6 :: Int)) 1000
1468ca0b-2a64-4fb4-8e52-ea5806644b4c commented 5 years ago

Created by: vvv

Adhere to explicit import lists or qualified imports.

import           Text.Printf (printf)