From c8be76b1d278a5d1f3c5c7f5cccee17248c943fb Mon Sep 17 00:00:00 2001 From: Marcin Szamotulski Date: Thu, 11 Apr 2024 20:17:51 +0200 Subject: [PATCH] peer-selection: removed localRoots from counters `localRoots` didn't count local connections, but the targets. We don't expose other targets in EKG metrics, so there's no reason to actually include local root targets. --- .../Test/Ouroboros/Network/PeerSelection.hs | 6 ++-- .../Network/PeerSelection/MockEnvironment.hs | 4 +-- .../Test/Ouroboros/Network/Testnet.hs | 4 +-- .../src/Ouroboros/Network/Diffusion/P2P.hs | 6 ++-- .../Network/PeerSelection/Governor.hs | 6 ++-- .../Network/PeerSelection/Governor/Types.hs | 35 +++++-------------- 6 files changed, 20 insertions(+), 41 deletions(-) diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs index 213b5afb13..32107ca780 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection.hs @@ -3292,7 +3292,7 @@ selectGovState :: Eq a selectGovState f = Signal.nub -- TODO: #3182 Rng seed should come from quickcheck. - . Signal.fromChangeEvents (f $! Governor.emptyPeerSelectionState (mkStdGen 42) []) + . Signal.fromChangeEvents (f $! Governor.emptyPeerSelectionState (mkStdGen 42)) . Signal.selectEvents (\case GovernorDebug (TraceGovernorState _ _ st) -> Just $! f st _ -> Nothing) @@ -3336,8 +3336,8 @@ _governorFindingPublicRoots targetNumberOfRootPeers readDomains readUseBootstrap readDomains (ioDNSActions LookupReqAAndAAAA) $ \requestPublicRootPeers -> do publicStateVar <- makePublicPeerSelectionStateVar - debugVar <- newTVarIO $ emptyPeerSelectionState (mkStdGen 42) [] - countersVar <- newTVarIO $ emptyPeerSelectionCounters [] + debugVar <- newTVarIO $ emptyPeerSelectionState (mkStdGen 42) + countersVar <- newTVarIO emptyPeerSelectionCounters peerSelectionGovernor tracer tracer tracer -- TODO: #3182 Rng seed should come from quickcheck. diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs index 83162ca658..3bcf299bc6 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/PeerSelection/MockEnvironment.hs @@ -210,8 +210,8 @@ governorAction mockEnv = do (ledgerStateJudgement mockEnv) usbVar <- playTimedScript (contramap TraceEnvSetUseBootstrapPeers tracerMockEnv) (useBootstrapPeers mockEnv) - debugVar <- StrictTVar.newTVarIO (emptyPeerSelectionState (mkStdGen 42) []) - countersVar <- StrictTVar.newTVarIO (emptyPeerSelectionCounters []) + debugVar <- StrictTVar.newTVarIO (emptyPeerSelectionState (mkStdGen 42)) + countersVar <- StrictTVar.newTVarIO emptyPeerSelectionCounters policy <- mockPeerSelectionPolicy mockEnv actions <- mockPeerSelectionActions tracerMockEnv mockEnv (readTVar usbVar) (readTVar lsjVar) policy exploreRaces -- explore races within the governor diff --git a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Testnet.hs b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Testnet.hs index 9d26d138e6..ead14c6d28 100644 --- a/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Testnet.hs +++ b/ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Testnet.hs @@ -3871,7 +3871,7 @@ selectDiffusionPeerSelectionState :: Eq a selectDiffusionPeerSelectionState f = Signal.nub -- TODO: #3182 Rng seed should come from quickcheck. - . Signal.fromChangeEvents (f $ Governor.emptyPeerSelectionState (mkStdGen 42) []) + . Signal.fromChangeEvents (f $ Governor.emptyPeerSelectionState (mkStdGen 42)) . Signal.selectEvents (\case DiffusionDebugPeerSelectionTrace (TraceGovernorState _ _ st) -> Just (f st) @@ -3883,7 +3883,7 @@ selectDiffusionPeerSelectionState' :: Eq a -> Signal a selectDiffusionPeerSelectionState' f = -- TODO: #3182 Rng seed should come from quickcheck. - Signal.fromChangeEvents (f $ Governor.emptyPeerSelectionState (mkStdGen 42) []) + Signal.fromChangeEvents (f $ Governor.emptyPeerSelectionState (mkStdGen 42)) . Signal.selectEvents (\case DiffusionDebugPeerSelectionTrace (TraceGovernorState _ _ st) -> Just (f st) diff --git a/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs b/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs index cb536adf85..2aa57f17a3 100644 --- a/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs +++ b/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs @@ -808,7 +808,7 @@ runM Interfaces min 2 (targetNumberOfActivePeers daPeerSelectionTargets) } - countersVar <- newTVarIO (emptyPeerSelectionCounters []) + countersVar <- newTVarIO emptyPeerSelectionCounters -- Design notes: -- - We split the following code into two parts: @@ -1062,7 +1062,7 @@ runM Interfaces -- InitiatorOnly mode, run peer selection only: InitiatorOnlyDiffusionMode -> withConnectionManagerInitiatorOnlyMode $ \connectionManager-> do - debugStateVar <- newTVarIO $ emptyPeerSelectionState fuzzRng [] + debugStateVar <- newTVarIO $ emptyPeerSelectionState fuzzRng diInstallSigUSR1Handler connectionManager debugStateVar daPeerMetrics withPeerStateActions' connectionManager $ \peerStateActions-> withPeerSelectionActions' @@ -1090,7 +1090,7 @@ runM Interfaces inboundInfoChannel outboundInfoChannel observableStateVar $ \connectionManager-> do - debugStateVar <- newTVarIO $ emptyPeerSelectionState fuzzRng [] + debugStateVar <- newTVarIO $ emptyPeerSelectionState fuzzRng diInstallSigUSR1Handler connectionManager debugStateVar daPeerMetrics withPeerStateActions' connectionManager $ \peerStateActions -> withPeerSelectionActions' diff --git a/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs b/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs index d6142e8c38..b0148c000a 100644 --- a/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs +++ b/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor.hs @@ -457,9 +457,7 @@ peerSelectionGovernor :: ( Alternative (STM m) -> PeerSelectionPolicy peeraddr m -> m Void peerSelectionGovernor tracer debugTracer countersTracer fuzzRng countersVar publicStateVar debugStateVar actions policy = - JobPool.withJobPool $ \jobPool -> do - localPeers <- map (\(w, h, _) -> (w, h)) - <$> atomically (readLocalRootPeers actions) + JobPool.withJobPool $ \jobPool -> peerSelectionGovernorLoop tracer debugTracer @@ -470,7 +468,7 @@ peerSelectionGovernor tracer debugTracer countersTracer fuzzRng countersVar publ actions policy jobPool - (emptyPeerSelectionState fuzzRng localPeers) + (emptyPeerSelectionState fuzzRng) -- | Our pattern here is a loop with two sets of guarded actions: -- diff --git a/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs b/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs index de305c0d5b..769cd59418 100644 --- a/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs +++ b/ouroboros-network/src/Ouroboros/Network/PeerSelection/Governor/Types.hs @@ -37,7 +37,7 @@ module Ouroboros.Network.PeerSelection.Governor.Types , TimedDecision , MkGuardedDecision , Completion (..) - , PeerSelectionCounters (.., PeerSelectionCountersHWC, numberOfColdPeers, numberOfWarmPeers, numberOfHotPeers, numberOfColdBigLedgerPeers, numberOfWarmBigLedgerPeers, numberOfHotBigLedgerPeers, numberOfColdLocalRootPeers, numberOfWarmLocalRootPeers, numberOfHotLocalRootPeers, localRootsHWC) + , PeerSelectionCounters (.., PeerSelectionCountersHWC, numberOfColdPeers, numberOfWarmPeers, numberOfHotPeers, numberOfColdBigLedgerPeers, numberOfWarmBigLedgerPeers, numberOfHotBigLedgerPeers, numberOfColdLocalRootPeers, numberOfWarmLocalRootPeers, numberOfHotLocalRootPeers) , emptyPeerSelectionCounters , peerSelectionStateToCounters -- * Peer Sharing Auxiliary data type @@ -623,11 +623,6 @@ data PeerSelectionCounters = PeerSelectionCounters { numberOfActiveLocalRootPeers :: Int, numberOfActiveLocalRootPeersDemotions :: Int, - -- | Local root peers with one entry per group. First entry is the number - -- of warm peers in that group the second is the number of hot peers in - -- that group. - localRoots :: ![(HotValency, WarmValency)], - -- -- Share Peers -- (peers received through peer sharing) @@ -661,7 +656,6 @@ data PeerSelectionCounters = PeerSelectionCounters { pattern PeerSelectionCountersHWC :: Int -> Int -> Int -- peers -> Int -> Int -> Int -- big ledger peers -> Int -> Int -> Int -- local roots - -> [(HotValency, WarmValency)] -> PeerSelectionCounters pattern PeerSelectionCountersHWC { numberOfColdPeers, numberOfWarmPeers, @@ -673,8 +667,7 @@ pattern PeerSelectionCountersHWC { numberOfColdPeers, numberOfColdLocalRootPeers, numberOfWarmLocalRootPeers, - numberOfHotLocalRootPeers, - localRootsHWC } + numberOfHotLocalRootPeers } <- (peerSelectionCountersHWC -> PeerSelectionCounters { numberOfKnownPeers = numberOfColdPeers, @@ -687,8 +680,8 @@ pattern PeerSelectionCountersHWC { numberOfColdPeers, numberOfKnownLocalRootPeers = numberOfColdLocalRootPeers, numberOfEstablishedLocalRootPeers = numberOfWarmLocalRootPeers, - numberOfActiveLocalRootPeers = numberOfHotLocalRootPeers, - localRoots = localRootsHWC }) + numberOfActiveLocalRootPeers = numberOfHotLocalRootPeers + }) {-# COMPLETE PeerSelectionCountersHWC #-} @@ -728,8 +721,6 @@ peerSelectionCountersHWC PeerSelectionCounters {..} = numberOfActiveLocalRootPeers, numberOfActiveLocalRootPeersDemotions, - localRoots, - numberOfKnownSharedPeers = numberOfKnownSharedPeers - numberOfEstablishedSharedPeers, numberOfColdSharedPeersPromotions, @@ -800,8 +791,6 @@ peerSelectionStateToCounters numberOfActiveLocalRootPeers = Set.size activeLocalRootsPeersSet, numberOfActiveLocalRootPeersDemotions = Set.size $ activeLocalRootsPeersSet `Set.intersection` inProgressDemoteHot, - localRoots, - numberOfKnownSharedPeers = Set.size knownSharedPeersSet, numberOfColdSharedPeersPromotions = Set.size $ knownSharedPeersSet `Set.intersection` inProgressPromoteCold, numberOfEstablishedSharedPeers = Set.size establishedSharedPeersSet, @@ -834,10 +823,6 @@ peerSelectionStateToCounters activeBigLedgerPeersSet = activePeers `Set.intersection` bigLedgerSet -- local roots - localRoots = - [ (hot, warm) - | (hot, warm, _) <- LocalRootPeers.toGroupSets localRootPeers - ] localRootSet = LocalRootPeers.keysSet localRootPeers -- local roots and big ledger peers are disjoint, hence we can use -- `knownPeersSet`, `establishedPeersSet` and `activePeersSet` below. @@ -862,9 +847,8 @@ peerSelectionStateToCounters -emptyPeerSelectionCounters :: [(HotValency, WarmValency)] - -> PeerSelectionCounters -emptyPeerSelectionCounters localRoots = +emptyPeerSelectionCounters :: PeerSelectionCounters +emptyPeerSelectionCounters = PeerSelectionCounters { numberOfRootPeers = 0, @@ -898,8 +882,6 @@ emptyPeerSelectionCounters localRoots = numberOfActiveLocalRootPeers = 0, numberOfActiveLocalRootPeersDemotions = 0, - localRoots, - numberOfKnownSharedPeers = 0, numberOfColdSharedPeersPromotions = 0, numberOfEstablishedSharedPeers = 0, @@ -910,9 +892,8 @@ emptyPeerSelectionCounters localRoots = } emptyPeerSelectionState :: StdGen - -> [(HotValency, WarmValency)] -> PeerSelectionState peeraddr peerconn -emptyPeerSelectionState rng localRoots = +emptyPeerSelectionState rng = PeerSelectionState { targets = nullPeerSelectionTargets, localRootPeers = LocalRootPeers.empty, @@ -933,7 +914,7 @@ emptyPeerSelectionState rng localRoots = inProgressDemoteHot = Set.empty, inProgressDemoteToCold = Set.empty, fuzzRng = rng, - countersCache = Cache (emptyPeerSelectionCounters localRoots), + countersCache = Cache emptyPeerSelectionCounters, ledgerStateJudgement = TooOld, bootstrapPeersFlag = DontUseBootstrapPeers, hasOnlyBootstrapPeers = False,