From d389672be701a1ba09754f541d7792e05f10c75a Mon Sep 17 00:00:00 2001 From: Marcin Szamotulski Date: Tue, 28 Jun 2022 07:07:14 +0200 Subject: [PATCH] Configurable inbound-governor idle timeout For local connections the inbound governor should not force the idleness timeout. Some clients relay letting the connection idle after it was negotiated and before any mini-protocol is started. We keep the connection manager idle timeout, this is only used for outbound connections, and thus not related to `node-to-client` protocol. --- .../demo/connection-manager.hs | 2 +- .../src/Ouroboros/Network/InboundGovernor.hs | 14 +++++++++----- .../src/Ouroboros/Network/Server2.hs | 2 +- .../test/Test/Ouroboros/Network/Server2.hs | 2 +- .../src/Ouroboros/Network/Diffusion/P2P.hs | 4 ++-- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/ouroboros-network-framework/demo/connection-manager.hs b/ouroboros-network-framework/demo/connection-manager.hs index 09961f0b11e..40f700e0cd0 100644 --- a/ouroboros-network-framework/demo/connection-manager.hs +++ b/ouroboros-network-framework/demo/connection-manager.hs @@ -268,7 +268,7 @@ withBidirectionalConnectionManager snocket socket serverInboundGovernorTracer = ("inbound-governor",) `contramap` debugTracer, serverConnectionLimits = AcceptedConnectionsLimit maxBound maxBound 0, serverConnectionManager = connectionManager, - serverInboundIdleTimeout = protocolIdleTimeout, + serverInboundIdleTimeout = Just protocolIdleTimeout, serverControlChannel = inbgovControlChannel, serverObservableStateVar = observableStateVar } diff --git a/ouroboros-network-framework/src/Ouroboros/Network/InboundGovernor.hs b/ouroboros-network-framework/src/Ouroboros/Network/InboundGovernor.hs index a026770be56..aecf80c4098 100644 --- a/ouroboros-network-framework/src/Ouroboros/Network/InboundGovernor.hs +++ b/ouroboros-network-framework/src/Ouroboros/Network/InboundGovernor.hs @@ -99,7 +99,7 @@ inboundGovernor :: forall (muxMode :: MuxMode) socket peerAddr versionNumber m a => Tracer m (RemoteTransitionTrace peerAddr) -> Tracer m (InboundGovernorTrace peerAddr) -> ServerControlChannel muxMode peerAddr ByteString m a b - -> DiffTime -- protocol idle timeout + -> Maybe DiffTime -- protocol idle timeout -> MuxConnectionManager muxMode socket peerAddr versionNumber ByteString m a b -> StrictTVar m InboundGovernorObservableState @@ -242,11 +242,13 @@ inboundGovernor trTracer tracer serverControlChannel inboundIdleTimeout Nothing -> return Nothing Just csCompletionMap -> do - v <- registerDelay inboundIdleTimeout + mv <- traverse registerDelay inboundIdleTimeout let -- initial state is 'RemoteIdle', if the remote end will not -- start any responders this will unregister the inbound side. csRemoteState :: RemoteState m - csRemoteState = RemoteIdle (LazySTM.readTVar v >>= check) + csRemoteState = RemoteIdle (case mv of + Nothing -> retry + Just v -> LazySTM.readTVar v >>= check) connState = ConnectionState { csMux, @@ -338,9 +340,11 @@ inboundGovernor trTracer tracer serverControlChannel inboundIdleTimeout let state' = unregisterConnection connId state return (Just connId, state') OperationSuccess {} -> do - v <- registerDelay inboundIdleTimeout + mv <- traverse registerDelay inboundIdleTimeout let timeoutSTM :: STM m () - !timeoutSTM = LazySTM.readTVar v >>= check + !timeoutSTM = case mv of + Nothing -> retry + Just v -> LazySTM.readTVar v >>= check let state' = updateRemoteState connId (RemoteIdle timeoutSTM) state diff --git a/ouroboros-network-framework/src/Ouroboros/Network/Server2.hs b/ouroboros-network-framework/src/Ouroboros/Network/Server2.hs index c049e5099b5..ad91b1281a0 100644 --- a/ouroboros-network-framework/src/Ouroboros/Network/Server2.hs +++ b/ouroboros-network-framework/src/Ouroboros/Network/Server2.hs @@ -80,7 +80,7 @@ data ServerArguments (muxMode :: MuxMode) socket peerAddr versionNumber bytes m -- | Time for which all protocols need to be idle to trigger -- 'DemotedToCold' transition. -- - serverInboundIdleTimeout :: DiffTime, + serverInboundIdleTimeout :: Maybe DiffTime, -- | Server control var is passed as an argument; this allows to use the -- server to run and manage responders which needs to be started on diff --git a/ouroboros-network-framework/test/Test/Ouroboros/Network/Server2.hs b/ouroboros-network-framework/test/Test/Ouroboros/Network/Server2.hs index 84b382d0bf1..c589019ad0f 100644 --- a/ouroboros-network-framework/test/Test/Ouroboros/Network/Server2.hs +++ b/ouroboros-network-framework/test/Test/Ouroboros/Network/Server2.hs @@ -567,7 +567,7 @@ withBidirectionalConnectionManager name timeouts WithName name `contramap` inboundTracer, -- InboundGovernorTrace serverConnectionLimits = acceptedConnLimit, serverConnectionManager = connectionManager, - serverInboundIdleTimeout = tProtocolIdleTimeout timeouts, + serverInboundIdleTimeout = Just (tProtocolIdleTimeout timeouts), serverControlChannel = inbgovControlChannel, serverObservableStateVar = observableStateVar } diff --git a/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs b/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs index a6d293da047..0768d9ae7c1 100644 --- a/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs +++ b/ouroboros-network/src/Ouroboros/Network/Diffusion/P2P.hs @@ -724,7 +724,7 @@ runM Interfaces serverTracer = dtLocalServerTracer, serverTrTracer = nullTracer, -- TODO: issue #3320 serverInboundGovernorTracer = dtLocalInboundGovernorTracer, - serverInboundIdleTimeout = local_PROTOCOL_IDLE_TIMEOUT, + serverInboundIdleTimeout = Nothing, serverConnectionLimits = localConnectionLimits, serverConnectionManager = localConnectionManager, serverControlChannel = localControlChannel, @@ -1018,7 +1018,7 @@ runM Interfaces serverInboundGovernorTracer = dtInboundGovernorTracer, serverConnectionLimits = daAcceptedConnectionsLimit, serverConnectionManager = connectionManager, - serverInboundIdleTimeout = daProtocolIdleTimeout, + serverInboundIdleTimeout = Just daProtocolIdleTimeout, serverControlChannel = controlChannel, serverObservableStateVar = observableStateVar })