Skip to content

Commit

Permalink
p2p-governor: inline some of invariant assertions
Browse files Browse the repository at this point in the history 
It's more useful to get assertion failures where the state changes.
This is only done for some of the state changes.
  • Loading branch information
@coot @karknu
coot authored and karknu committed Jan 26, 2021
1 parent 5ac4b95 commit 6b87c96
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 103 deletions.
54 changes: 29 additions & 25 deletions ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/ActivePeers.hs
View file
Expand Up @@ -14,7 +14,7 @@ import qualified Data.Set as Set

import Control.Monad.Class.MonadSTM
import Control.Concurrent.JobPool (Job(..))
import Control.Exception (SomeException)
import Control.Exception (SomeException, assert)

import Ouroboros.Network.PeerSelection.Types
import Ouroboros.Network.PeerSelection.KnownPeers (KnownPeerInfo(..))
Expand Down Expand Up @@ -124,18 +124,20 @@ jobPromoteWarmPeer PeerSelectionActions{peerStateActions = PeerStateActions {act
--TODO: decide if we should do timeouts here or if we should make that
-- the responsibility of activatePeerConnection
activatePeerConnection peerconn
return $ Completion $ \st _now -> Decision {
decisionTrace = TracePromoteWarmDone peeraddr,
decisionState = st {
activePeers = Set.insert peeraddr
(activePeers st),
establishedStatus = Map.insert peeraddr PeerHot
(establishedStatus st),
inProgressPromoteWarm = Set.delete peeraddr
(inProgressPromoteWarm st)
},
decisionJobs = []
}
return $ Completion $ \st _now ->
assert (peeraddr `Map.member` establishedPeers st)
Decision {
decisionTrace = TracePromoteWarmDone peeraddr,
decisionState = st {
activePeers = Set.insert peeraddr
(activePeers st),
establishedStatus = Map.insert peeraddr PeerHot
(establishedStatus st),
inProgressPromoteWarm = Set.delete peeraddr
(inProgressPromoteWarm st)
},
decisionJobs = []
}


----------------------------
Expand Down Expand Up @@ -232,15 +234,17 @@ jobDemoteActivePeer PeerSelectionActions{peerStateActions = PeerStateActions {de
job :: m (Completion m peeraddr peerconn)
job = do
deactivatePeerConnection peerconn
return $ Completion $ \st _now -> Decision {
decisionTrace = TraceDemoteHotDone peeraddr,
decisionState = st {
activePeers = Set.delete peeraddr
(activePeers st),
establishedStatus = Map.insert peeraddr PeerWarm
(establishedStatus st),
inProgressDemoteHot = Set.delete peeraddr
(inProgressDemoteHot st)
},
decisionJobs = []
}
return $ Completion $ \st _now ->
assert (peeraddr `Map.member` establishedPeers st)
Decision {
decisionTrace = TraceDemoteHotDone peeraddr,
decisionState = st {
activePeers = Set.delete peeraddr
(activePeers st),
establishedStatus = Map.insert peeraddr PeerWarm
(establishedStatus st),
inProgressDemoteHot = Set.delete peeraddr
(inProgressDemoteHot st)
},
decisionJobs = []
}
39 changes: 24 additions & 15 deletions ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/KnownPeers.hs
View file
Expand Up @@ -18,9 +18,10 @@ import Control.Monad.Class.MonadAsync
import Control.Monad.Class.MonadSTM
import Control.Monad.Class.MonadTime
import Control.Monad.Class.MonadTimer
import Control.Exception (Exception(..), SomeException)
import Control.Exception (Exception(..), SomeException, assert)

import Ouroboros.Network.PeerSelection.Types
import Ouroboros.Network.PeerSelection.KnownPeers (KnownPeerInfo (..))
import qualified Ouroboros.Network.PeerSelection.KnownPeers as KnownPeers
import Ouroboros.Network.PeerSelection.Governor.Types

Expand Down Expand Up @@ -291,20 +292,28 @@ aboveTarget PeerSelectionPolicy {
policyPickColdPeersToForget
availableToForget
numPeersToForget
return $ \_now -> Decision {
decisionTrace = TraceForgetColdPeers
targetNumberOfKnownPeers
numKnownPeers
selectedToForget,
decisionState = st {
knownPeers = KnownPeers.delete
selectedToForget
knownPeers,
publicRootPeers = publicRootPeers
Set.\\ selectedToForget
},
decisionJobs = []
}
return $ \_now ->
let knownPeers' = KnownPeers.delete
selectedToForget
knownPeers
publicRootPeers' = publicRootPeers
Set.\\ selectedToForget
in assert
(Map.isSubmapOfBy (\_ KnownPeerInfo {knownPeerSource} ->
knownPeerSource == PeerSourcePublicRoot
|| knownPeerSource == PeerSourceLocalRoot)
(Map.fromSet (const ()) publicRootPeers')
(KnownPeers.toMap knownPeers'))

Decision {
decisionTrace = TraceForgetColdPeers
targetNumberOfKnownPeers
numKnownPeers
selectedToForget,
decisionState = st { knownPeers = knownPeers',
publicRootPeers = publicRootPeers' },
decisionJobs = []
}

| otherwise
= GuardedSkip Nothing
Expand Down
124 changes: 76 additions & 48 deletions ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Monitor.hs
View file
Expand Up @@ -25,6 +25,7 @@ import Control.Monad.Class.MonadSTM
import Control.Monad.Class.MonadTime
import Control.Exception (assert)

import Ouroboros.Network.PeerSelection.KnownPeers (KnownPeerInfo (..))
import qualified Ouroboros.Network.PeerSelection.KnownPeers as KnownPeers
import Ouroboros.Network.PeerSelection.Types
import Ouroboros.Network.PeerSelection.Governor.Types
Expand Down Expand Up @@ -107,41 +108,53 @@ connections PeerSelectionActions{peerStateActions = PeerStateActions {monitorPee
establishedStatus'
check (not (Map.null demotions))
let (demotedToWarm, demotedToCold) = Map.partition (==PeerWarm) demotions
return $ \now -> Decision {
decisionTrace = TraceDemoteAsynchronous demotions,
decisionJobs = [],
decisionState = st {
activePeers = activePeers
Set.\\ Map.keysSet demotions,
establishedPeers = establishedPeers
Map.\\ demotedToCold,

-- Note that we do not use establishedStatus' which
-- has the synchronous ones that are supposed to be
-- handled elsewhere. We just update the async ones:
establishedStatus = demotedToWarm
<> establishedStatus
Map.\\ demotedToCold,

-- Asynchronous transition to cold peer can only be
-- a result of a failure.
knownPeers = KnownPeers.setConnectTime
(Map.keysSet demotedToCold)
(reconnectDelay `addTime` now)
. foldr
((snd .) . KnownPeers.incrementFailCount)
(knownPeers st)
$ (Map.keysSet demotedToCold),

-- When promoting a warm peer, it might happen
-- that the connection will break (or one of the
-- established protocols will error). For that
-- reason we need to adjust 'inProgressPromoteWarm'.
inProgressPromoteWarm
= inProgressPromoteWarm
Set.\\ Map.keysSet demotions
}
}
return $ \now ->
let activePeers1 = activePeers
Set.\\ Map.keysSet demotions

establishedPeers1 = establishedPeers
Map.\\ demotedToCold

-- Note that we do not use establishedStatus' which
-- has the synchronous ones that are supposed to be
-- handled elsewhere. We just update the async ones:
establishedStatus1 = demotedToWarm
<> establishedStatus
Map.\\ demotedToCold

-- Asynchronous transition to cold peer can only be
-- a result of a failure.
knownPeers1 = KnownPeers.setConnectTime
(Map.keysSet demotedToCold)
(reconnectDelay `addTime` now)
. foldr
((snd .) . KnownPeers.incrementFailCount)
(knownPeers st)
$ (Map.keysSet demotedToCold)

in assert
(let establishedPeersSet1 = Map.keysSet establishedPeers1
in activePeers1 `Set.isSubsetOf` establishedPeersSet1
&& Map.keysSet establishedStatus1 == establishedPeersSet1)

Decision {
decisionTrace = TraceDemoteAsynchronous demotions,
decisionJobs = [],
decisionState = st {
activePeers = activePeers1,
establishedPeers = establishedPeers1,
establishedStatus = establishedStatus1,
knownPeers = knownPeers1,

-- When promoting a warm peer, it might happen
-- that the connection will break (or one of the
-- established protocols will error). For that
-- reason we need to adjust 'inProgressPromoteWarm'.
inProgressPromoteWarm
= inProgressPromoteWarm
Set.\\ Map.keysSet demotions
}
}
where
-- Those demotions that occurred not as a result of action by the governor.
-- They're further classified into demotions to warm, and demotions to cold.
Expand Down Expand Up @@ -229,16 +242,31 @@ localRoots actions@PeerSelectionActions{readLocalRootPeers}
selectedToDemote = activePeers `Set.intersection` removedSet
selectedToDemote' = establishedPeers
`Map.restrictKeys` selectedToDemote
return $ \_now -> Decision {
decisionTrace = TraceLocalRootPeersChanged localRootPeers
localRootPeers',
decisionState = st {
localRootPeers = localRootPeers',
publicRootPeers = publicRootPeers',
knownPeers = knownPeers',
inProgressDemoteHot = inProgressDemoteHot
<> selectedToDemote
},
decisionJobs = [ jobDemoteActivePeer actions peeraddr peerconn
| (peeraddr, peerconn) <- Map.assocs selectedToDemote' ]
}
return $ \_now ->

assert
(Map.isSubmapOfBy (\_ KnownPeerInfo {knownPeerSource} ->
knownPeerSource == PeerSourcePublicRoot)
(Map.fromSet (const ()) publicRootPeers')
(KnownPeers.toMap knownPeers'))
. assert
(Map.isSubmapOfBy (\rootPeerAdvertise
KnownPeerInfo {knownPeerAdvertise, knownPeerSource} ->
knownPeerSource == PeerSourceLocalRoot
&& knownPeerAdvertise == rootPeerAdvertise)
localRootPeers'
(KnownPeers.toMap knownPeers'))

$ Decision {
decisionTrace = TraceLocalRootPeersChanged localRootPeers
localRootPeers',
decisionState = st {
localRootPeers = localRootPeers',
publicRootPeers = publicRootPeers',
knownPeers = knownPeers',
inProgressDemoteHot = inProgressDemoteHot
<> selectedToDemote
},
decisionJobs = [ jobDemoteActivePeer actions peeraddr peerconn
| (peeraddr, peerconn) <- Map.assocs selectedToDemote' ]
}
39 changes: 24 additions & 15 deletions ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/RootPeers.hs
View file
Expand Up @@ -12,9 +12,10 @@ import qualified Data.Set as Set
import Control.Concurrent.JobPool (Job(..))
import Control.Monad.Class.MonadSTM
import Control.Monad.Class.MonadTime
import Control.Exception (SomeException)
import Control.Exception (SomeException, assert)

import Ouroboros.Network.PeerSelection.Types
import Ouroboros.Network.PeerSelection.KnownPeers (KnownPeerInfo (..))
import qualified Ouroboros.Network.PeerSelection.KnownPeers as KnownPeers
import Ouroboros.Network.PeerSelection.Governor.Types

Expand Down Expand Up @@ -138,17 +139,25 @@ jobReqPublicRootPeers PeerSelectionActions{requestPublicRootPeers}

publicRootRetryTime :: Time
publicRootRetryTime = addTime publicRootRetryDiffTime now
in Decision {
decisionTrace = TracePublicRootsResults
newPeers
publicRootBackoffs'
publicRootRetryDiffTime,
decisionState = st {
publicRootPeers = publicRootPeers',
knownPeers = knownPeers',
publicRootBackoffs = publicRootBackoffs',
publicRootRetryTime = publicRootRetryTime,
inProgressPublicRootsReq = False
},
decisionJobs = []
}

in assert
(Map.isSubmapOfBy (\_ KnownPeerInfo {knownPeerSource} ->
knownPeerSource == PeerSourcePublicRoot
|| knownPeerSource == PeerSourceLocalRoot)
(Map.fromSet (const ()) publicRootPeers')
(KnownPeers.toMap knownPeers'))

Decision {
decisionTrace = TracePublicRootsResults
newPeers
publicRootBackoffs'
publicRootRetryDiffTime,
decisionState = st {
publicRootPeers = publicRootPeers',
knownPeers = knownPeers',
publicRootBackoffs = publicRootBackoffs',
publicRootRetryTime = publicRootRetryTime,
inProgressPublicRootsReq = False
},
decisionJobs = []
}

0 comments on commit 6b87c96

Please sign in to comment.