Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use exponential distribution for P2P session timeouts #1244

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
31 changes: 25 additions & 6 deletions src/P2P/Node.hs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE DerivingStrategies #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE MultiWayIf #-}
{-# LANGUAGE NumericUnderscores #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE RankNTypes #-}
{-# LANGUAGE ScopedTypeVariables #-}
Expand Down Expand Up @@ -585,26 +587,43 @@ newSession conf node = do
let env = peerClientEnv node newPeerInfo
(info, newSes) <- mask $ \restore -> do
now <- getCurrentTimeIntegral
t <- R.randomRIO
( round (0.9 * timeoutMs)
, round (1.1 * timeoutMs)
)
!newSes <- async $ restore $ timeout t
t <- sessionTimeout $ secondsToTimeSpan $ _p2pConfigSessionTimeout conf
!newSes <- async $ restore $ timeout (int $ timeSpanToMicros t)
$ _p2pNodeClientSession node (loggFun node) env newPeerInfo
incrementActiveSessionCount peerDb newPeerInfo
!info <- atomically $ addSession node newPeerInfo newSes now
return (info, newSes)
logg node Debug $ "Started peer session " <> showSessionId newPeerInfo newSes
loggFun node Info $ JsonLog info
where
TimeSpan timeoutMs = secondsToTimeSpan @Double (_p2pConfigSessionTimeout conf)
peerDb = _p2pNodePeerDb node

syncFromPeer_ pinfo
| _p2pConfigPrivate conf = return True
| _p2pNodeDoPeerSync node = syncFromPeer node pinfo
| otherwise = return True

-- | (Roughly) exponentially distributed timespans with the given expectation
-- within the range of a a tenth of the expectation and ten times the
-- expectation.
--
-- The expected value of the actual distribution gets increasinly imprecise
-- as the input value gets smaller, because a minimum result of 5 seconds
-- is implemented. Input values of less than 30s should be avoided.
--
sessionTimeout :: TimeSpan Int -> IO (TimeSpan Int)
sessionTimeout expected = do
x <- exponential (1 / us) :: IO Double
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use System.Random.MWC instead, it's better in basically every way -- including already shipping with a truncated exponential distribution. I used it for exactly this in https://github.com/kadena-io/chainweb-node/pull/1254/files, you could crib the code from there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to use it. However, I run into compatibility issues with our rather old nix snapshot. I don't remember the details. However, this implementation should be the same than what is used in mac-random. Also, I think, modern versions of the random package have much better performance than previous versions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lars -- mwc-random was already in the deps list?

return $! microsToTimeSpan $ round $ max lower $ min upper x
where
us = int $ timeSpanToMicros expected
lower = max 5_000_000 (us / 10) -- at least 5 seconds
upper = us * 10

exponential rate = do
!x <- R.getStdRandom (R.randomR (0, 1))
return $! - log x / rate

-- | Monitor and garbage collect sessions
--
awaitSessions :: P2pNode -> IO ()
Expand Down