Try latest http2 library and star re-using connections again

wireapp · Jul 15, 2024 · bba3c8b · bba3c8b
1 parent 71a30ab
commit bba3c8b
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 36 deletions.
diff --git a/libs/wire-api-federation/default.nix b/libs/wire-api-federation/default.nix
@@ -6,7 +6,6 @@
 , aeson
 , aeson-pretty
 , amqp
-, async
 , base
 , bytestring
 , bytestring-conversion
@@ -54,7 +53,6 @@ mkDerivation {
   libraryHaskellDepends = [
     aeson
     amqp
-    async
     base
     bytestring
     bytestring-conversion

diff --git a/libs/wire-api-federation/src/Wire/API/Federation/Client.hs b/libs/wire-api-federation/src/Wire/API/Federation/Client.hs
@@ -34,7 +34,6 @@ module Wire.API.Federation.Client
   )
 where
 
-import Control.Concurrent.Async
 import Control.Exception qualified as E
 import Control.Monad.Catch
 import Control.Monad.Codensity
@@ -63,7 +62,6 @@ import Network.HTTP.Media qualified as HTTP
 import Network.HTTP.Types qualified as HTTP
 import Network.HTTP2.Client qualified as HTTP2
 import Network.Wai.Utilities.Error qualified as Wai
-import OpenSSL.Session qualified as SSL
 import Servant.Client
 import Servant.Client.Core
 import Servant.Types.SourceT
@@ -123,27 +121,13 @@ liftCodensity = FederatorClient . lift . lift . lift
 headersFromTable :: HTTP2.TokenHeaderTable -> [HTTP.Header]
 headersFromTable (headerList, _) = flip map headerList $ first HTTP2.tokenKey
 
--- This opens a new http2 connection. Using a http2-manager leads to this problem https://wearezeta.atlassian.net/browse/WPB-4787
--- FUTUREWORK: Replace with H2Manager.withHTTP2Request once the bugs are solved.
-withNewHttpRequest :: H2Manager.Target -> HTTP2.Request -> (HTTP2.Response -> IO a) -> IO a
-withNewHttpRequest target req k = do
-  ctx <- SSL.context
-  let cacheLimit = 20
-      sslRemoveTrailingDot = False
-      tcpConnectionTimeout = 30_000_000
-  sendReqMVar <- newEmptyMVar
-  thread <- liftIO . async $ H2Manager.startPersistentHTTP2Connection ctx target cacheLimit sslRemoveTrailingDot tcpConnectionTimeout sendReqMVar
-  let newConn = H2Manager.HTTP2Conn thread (putMVar sendReqMVar H2Manager.CloseConnection) sendReqMVar
-  H2Manager.sendRequestWithConnection newConn req $ \resp -> do
-    k resp <* newConn.disconnect
-
 performHTTP2Request ::
   Http2Manager ->
   H2Manager.Target ->
   HTTP2.Request ->
   IO (Either FederatorClientHTTP2Error (ResponseF Builder))
-performHTTP2Request _mgr target req = try $ do
-  withNewHttpRequest target req $ consumeStreamingResponseWith $ \resp -> do
+performHTTP2Request mgr target req = try $ do
+  H2Manager.withHTTP2Request mgr target req $ consumeStreamingResponseWith $ \resp -> do
     b <-
       fmap (fromRight mempty)
         . runExceptT
@@ -250,7 +234,7 @@ withHTTP2StreamingRequest successfulStatus req handleResponse = do
       $ Codensity
       $ \k ->
         E.catches
-          (withNewHttpRequest (False, hostname, port) req' (consumeStreamingResponseWith (k . Right)))
+          (H2Manager.withHTTP2Request env.ceHttp2Manager (False, hostname, port) req' (consumeStreamingResponseWith (k . Right)))
           [ E.Handler $ k . Left . FederatorClientHTTP2Error,
             E.Handler $ k . Left . FederatorClientHTTP2Error . FederatorClientConnectionError,
             E.Handler $ k . Left . FederatorClientHTTP2Error . FederatorClientHTTP2Exception,

diff --git a/libs/wire-api-federation/wire-api-federation.cabal b/libs/wire-api-federation/wire-api-federation.cabal
@@ -86,7 +86,6 @@ library
   build-depends:
       aeson                  >=2.0.1.0
     , amqp
-    , async
     , base                   >=4.6     && <5.0
     , bytestring
     , bytestring-conversion

diff --git a/nix/haskell-pins.nix b/nix/haskell-pins.nix
@@ -270,6 +270,23 @@ let
       };
     };
 
+    http2 = {
+      src = pkgs.fetchFromGitHub {
+        owner = "kazu-yamamoto";
+        repo = "http2";
+        rev = "90dde392d8d0edba77c66fe99966267ed9369c30";
+        hash = "sha256-RkvJxpMkNbDGSPkjt+h5smhe1cr6bnqYeAfyc5YbwIU=";
+      };
+    };
+
+    http-semantics = {
+      src = pkgs.fetchFromGitHub {
+        owner = "kazu-yamamoto";
+        repo = "http-semantics";
+        rev = "d4315ba8d4d23ec09a7642064d20353a296ceb89";
+        hash = "sha256-ChpXT+FMqdm2WkEBR1hq4cjKPj3WmjeePvTnQgcJzlM=";
+      };
+    };
   };
 
   hackagePins = {
@@ -281,19 +298,6 @@ let
     };
 
     # start pinned dependencies for http2
-
-    # this contains an important fix to the initialization of the window size
-    # and should be switched to upstream as soon as we can
-    http2 = {
-      version = "5.2.5";
-      sha256 = "sha256-FCd4lPydwWqm2lrhgYtPW+BuXGqmmA8KFrB87SYEowY=";
-    };
-
-    http-semantics = {
-      version = "0.1.2";
-      sha256 = "sha256-S4rGBCIKVPpLPumLcVzrPONrbWm8VBizqxI3dXNIfr0=";
-    };
-
     network-run = {
       version = "0.3.0";
       sha256 = "sha256-FP2GZKwacC+TLLwEIVgKBtnKplYPf5xOIjDfvlbQV0o=";

diff --git a/nix/manual-overrides.nix b/nix/manual-overrides.nix
@@ -70,8 +70,12 @@ hself: hsuper: {
   tls = hsuper.tls_2_0_5;
   tls-session-manager = hsuper.tls-session-manager_0_0_5;
 
+  # http-semantics is not released with the right version yet, but http2 bounds
+  # have already changed.
+  http2 = hlib.doJailbreak hsuper.http2;
+
   # warp requires curl in its testsuite
-  warp = hlib.addTestToolDepends hsuper.warp [ curl ];
+  warp = hlib.doJailbreak (hlib.addTestToolDepends hsuper.warp [ curl ]);
 
   # -----------------
   # flags and patches