Skip to content

Commit aab8d3b

Browse files
committed
churn: code refactoring
Use a newtype for `ChurnMode`. ChurnMode follows `FetchMode`, hence a newtype actually makes more sense than a separate data type.
1 parent 4ac087c commit aab8d3b

File tree

6 files changed

+47
-30
lines changed

6 files changed

+47
-30
lines changed

ouroboros-network/ouroboros-network.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ library
127127
nothunks,
128128
psqueues >=0.2.3 && <0.3,
129129
random,
130+
quiet,
130131

131132
cardano-prelude,
132133
cardano-slotting,

ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Policies.hs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import System.Random
2424
import NoThunks.Class.Orphans ()
2525

2626
import Cardano.Slotting.Slot (SlotNo (..))
27+
import Ouroboros.Network.BlockFetch.ConsensusInterface (FetchMode (..))
2728
import Ouroboros.Network.Diffusion.Policies
2829
import Ouroboros.Network.ExitPolicy (RepromoteDelay (..))
2930
import Ouroboros.Network.PeerSelection.Governor
@@ -84,8 +85,12 @@ instance Arbitrary ArbitraryDemotion where
8485
newtype ArbitraryChurnMode = ArbitraryChurnMode ChurnMode deriving Show
8586

8687
instance Arbitrary ArbitraryChurnMode where
87-
arbitrary = ArbitraryChurnMode <$>
88-
elements [ChurnModeNormal, ChurnModeBulkSync]
88+
arbitrary = ArbitraryChurnMode . ChurnMode <$>
89+
elements [FetchModeDeadline, FetchModeBulkSync]
90+
shrink (ArbitraryChurnMode (ChurnMode FetchModeDeadline)) =
91+
[ArbitraryChurnMode (ChurnMode FetchModeBulkSync)]
92+
shrink (ArbitraryChurnMode (ChurnMode FetchModeBulkSync)) =
93+
[]
8994

9095
instance Arbitrary ArbitraryPolicyArguments where
9196
arbitrary = do
@@ -182,11 +187,11 @@ prop_hotToWarmM ArbitraryPolicyArguments{..} seed = do
182187
-> m Property
183188
noneWorse metrics pickedSet = do
184189
scores <- atomically $ case apaChurnMode of
185-
ChurnModeNormal -> do
190+
ChurnMode FetchModeDeadline -> do
186191
hup <- upstreamyness metrics
187192
bup <- fetchynessBlocks metrics
188193
return $ Map.unionWith (+) hup bup
189-
ChurnModeBulkSync ->
194+
ChurnMode FetchModeBulkSync ->
190195
fetchynessBytes metrics
191196
let (picked, notPicked) = Map.partitionWithKey fn scores
192197
maxPicked = maximum $ Map.elems picked

ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,9 @@ import Ouroboros.Network.NodeToNode qualified as NodeToNode
103103
import Ouroboros.Network.PeerSelection.Bootstrap (UseBootstrapPeers)
104104
import Ouroboros.Network.PeerSelection.Churn (PeerChurnArgs (..))
105105
import Ouroboros.Network.PeerSelection.Governor qualified as Governor
106-
import Ouroboros.Network.PeerSelection.Governor.Types
107-
(ChurnMode (ChurnModeNormal), ConsensusModePeerTargets (..),
108-
DebugPeerSelection (..), PeerSelectionActions, PeerSelectionCounters,
106+
import Ouroboros.Network.PeerSelection.Governor.Types (ChurnMode (..),
107+
ConsensusModePeerTargets (..), DebugPeerSelection (..),
108+
PeerSelectionActions, PeerSelectionCounters,
109109
PeerSelectionInterfaces (..), PeerSelectionPolicy (..),
110110
PeerSelectionState, TracePeerSelection (..),
111111
emptyPeerSelectionCounters, emptyPeerSelectionState)
@@ -828,7 +828,7 @@ runM Interfaces
828828
-- demoting/promoting peers.
829829
policyRngVar <- newTVarIO policyRng
830830

831-
churnModeVar <- newTVarIO ChurnModeNormal
831+
churnModeVar <- newTVarIO (ChurnMode FetchModeDeadline)
832832

833833
localRootsVar <- newTVarIO mempty
834834

ouroboros-network/src/Ouroboros/Network/Diffusion/Policies.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,13 @@ simplePeerSelectionPolicy rngVar getChurnMode metrics errorDelay = PeerSelection
150150
hotDemotionPolicy _ _ _ available pickNum = do
151151
mode <- getChurnMode
152152
scores <- case mode of
153-
ChurnModeNormal -> do
153+
ChurnMode FetchModeDeadline -> do
154154
jpm <- joinedPeerMetricAt metrics
155155
hup <- upstreamyness metrics
156156
bup <- fetchynessBlocks metrics
157157
return $ Map.unionWith (+) hup bup `optionalMerge` jpm
158158

159-
ChurnModeBulkSync -> do
159+
ChurnMode FetchModeBulkSync -> do
160160
jpm <- joinedPeerMetricAt metrics
161161
bup <- fetchynessBytes metrics
162162
return $ bup `optionalMerge` jpm

ouroboros-network/src/Ouroboros/Network/PeerSelection/Churn.hs

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import System.Random
3030
import Control.Applicative (Alternative)
3131
import Data.Functor (($>))
3232
import Data.Monoid.Synchronisation (FirstToFinish (..))
33-
import Ouroboros.Network.BlockFetch (FetchMode (..))
3433
import Ouroboros.Network.ConsensusMode (ConsensusMode (..))
3534
import Ouroboros.Network.Diffusion.Policies (churnEstablishConnectionTimeout,
3635
closeConnectionTimeout, deactivateTimeout)
@@ -40,11 +39,13 @@ import Ouroboros.Network.PeerSelection.LedgerPeers.Type
4039
import Ouroboros.Network.PeerSelection.PeerMetric
4140
import Ouroboros.Network.PeerSelection.State.LocalRootPeers (HotValency (..))
4241

43-
-- | Tag indicating churning approach
44-
-- There are three syncing methods that networking layer supports, the legacy
45-
-- method with or without bootstrap peers, and the Genesis method that relies
46-
-- on chain skipping optimization courtesy of consensus, which also provides
47-
42+
-- | Tag indicating churning approach.
43+
--
44+
-- There are three syncing methods supported by ouroboros-network:
45+
--
46+
-- * the legacy method (praos mode) without bootstrap peers,
47+
-- * bootstrap peers, and
48+
-- * the Genesis method which is using it's own targets for syncing.
4849
--
4950
data ChurnRegime = ChurnDefault
5051
-- ^ tag to use Praos targets when caught up, or Genesis
@@ -64,12 +65,12 @@ getPeerSelectionTargets consensus lsj ConsensusModePeerTargets {
6465
_otherwise -> deadlineTargets
6566

6667
pickChurnRegime :: ConsensusMode -> ChurnMode -> UseBootstrapPeers -> ChurnRegime
67-
pickChurnRegime consensus churn ubp =
68-
case (churn, ubp, consensus) of
69-
(ChurnModeNormal, _, _) -> ChurnDefault
70-
(_, _, GenesisMode) -> ChurnDefault
71-
(ChurnModeBulkSync, UseBootstrapPeers _, _) -> ChurnBootstrapPraosSync
72-
(ChurnModeBulkSync, _, _) -> ChurnPraosSync
68+
pickChurnRegime consensus churn bootstrap =
69+
case (consensus, churn, bootstrap) of
70+
(GenesisMode, _, _) -> ChurnDefault
71+
(_, ChurnMode FetchModeDeadline, _) -> ChurnDefault
72+
(_, ChurnMode FetchModeBulkSync, DontUseBootstrapPeers) -> ChurnPraosSync
73+
(_, ChurnMode FetchModeBulkSync, UseBootstrapPeers{}) -> ChurnBootstrapPraosSync
7374

7475
-- | Facilitates composing updates to various targets via back-to-back pipeline
7576
type ModifyPeerSelectionTargets = PeerSelectionTargets -> PeerSelectionTargets
@@ -154,10 +155,7 @@ peerChurnGovernor PeerChurnArgs {
154155
where
155156
updateChurnMode :: STM m ChurnMode
156157
updateChurnMode = do
157-
fm <- getFetchMode
158-
let mode = case fm of
159-
FetchModeDeadline -> ChurnModeNormal
160-
FetchModeBulkSync -> ChurnModeBulkSync
158+
mode <- ChurnMode <$> getFetchMode
161159
writeTVar churnModeVar mode
162160
return mode
163161

ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
{-# LANGUAGE BangPatterns #-}
22
{-# LANGUAGE CPP #-}
33
{-# LANGUAGE DeriveFunctor #-}
4+
{-# LANGUAGE DeriveGeneric #-}
5+
{-# LANGUAGE DerivingStrategies #-}
6+
{-# LANGUAGE DerivingVia #-}
47
{-# LANGUAGE ExistentialQuantification #-}
58
{-# LANGUAGE FlexibleContexts #-}
69
{-# LANGUAGE GADTs #-}
@@ -33,6 +36,7 @@ module Ouroboros.Network.PeerSelection.Governor.Types
3336
, PeerSelectionActions (..)
3437
, PeerSelectionInterfaces (..)
3538
, ChurnMode (..)
39+
, FetchMode (..)
3640
-- * P2P governor internals
3741
, PeerSelectionState (..)
3842
, emptyPeerSelectionState
@@ -133,15 +137,17 @@ import Data.Semigroup (Min (..))
133137
import Data.Set (Set)
134138
import Data.Set qualified as Set
135139
import GHC.Stack (HasCallStack)
140+
import GHC.Generics (Generic (..))
141+
import Quiet (Quiet (..))
136142

137143
import Control.Applicative (Alternative)
144+
import Control.Concurrent.Class.MonadSTM.Strict
138145
import Control.Concurrent.JobPool (Job)
139146
import Control.Exception (Exception (..), SomeException, assert)
140-
import Control.Monad.Class.MonadSTM
141147
import Control.Monad.Class.MonadTime.SI
142148
import System.Random (StdGen)
143149

144-
import Control.Concurrent.Class.MonadSTM.Strict
150+
import Ouroboros.Network.BlockFetch.ConsensusInterface (FetchMode (..))
145151
import Ouroboros.Network.ConsensusMode
146152
import Ouroboros.Network.ExitPolicy
147153
import Ouroboros.Network.PeerSelection.Bootstrap (UseBootstrapPeers (..))
@@ -1768,6 +1774,13 @@ data DebugPeerSelection peeraddr where
17681774
deriving instance (Ord peeraddr, Show peeraddr)
17691775
=> Show (DebugPeerSelection peeraddr)
17701776

1771-
data ChurnMode = ChurnModeBulkSync
1772-
| ChurnModeNormal deriving Show
1777+
-- | Churn mode is set by `churn` and available in peer selection policy. It
1778+
-- follows `FetchMode`, thus it's a newtype wrapper.
1779+
--
1780+
-- It is shared using its own `TVar` to make sure the value available in peer
1781+
-- selection policy is consistent with the value available in churn actions.
1782+
--
1783+
newtype ChurnMode = ChurnMode { getFetchMode :: FetchMode }
1784+
deriving stock Generic
1785+
deriving Show via Quiet ChurnMode
17731786

0 commit comments

Comments
 (0)