Merge #3844

3844: Inbound governor idle timeout r=coot a=coot 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. Fixes #3843 Co-authored-by: Marcin Szamotulski <[email protected]>
IntersectMBO · Jun 28, 2022 · 18efc8c · 18efc8c
2 parents b46b5ba + d389672
commit 18efc8c
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
                                 })