Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open connections are leaking #490

Closed
neongreen opened this issue Sep 19, 2018 · 7 comments · Fixed by #493
Closed

Open connections are leaking #490

neongreen opened this issue Sep 19, 2018 · 7 comments · Fixed by #493

Comments

@neongreen
Copy link

I do a bunch of SQS.deleteMessages in a loop, and experience long waits (5-10 seconds) after getting 99 open connections. This happens with https://github.com/iain/fake_sqs (I haven't tested with real SQS).

This behavior is introduced by a recent commit (made after 1.6.0 was released): 2688190.

My guess is that in the absence of finalizers in conduit-1.3, nothing is closing connections open by Responses. The following diff fixes the behavior of deleteMessage for me:

diff --git a/core/src/Network/AWS/Response.hs b/core/src/Network/AWS/Response.hs
index 4791de6a3c..907b0faa05 100644
--- a/core/src/Network/AWS/Response.hs
+++ b/core/src/Network/AWS/Response.hs
@@ -45,8 +45,9 @@ receiveNull :: (MonadResource m, MonadThrow m)
             -> Proxy a
             -> ClientResponse
             -> m (Response a)
-receiveNull rs _ = stream $ \_ _ x ->
-    liftResourceT (x `connect` pure (Right rs))
+receiveNull rs _ = stream $ \_ _ x -> do
+    sinkLBS x
+    pure (Right rs)

 receiveEmpty :: (MonadResource m, MonadThrow m)
              => (Int -> ResponseHeaders -> () -> Either String (Rs a))

It's still slower than it used to be (I've no idea why), but at least it doesn't leak connections anymore and I get no stuck requests.

Other receiveBlah functions might also be affected.

@naushadh
Copy link
Contributor

Perhaps sinkNull would be quicker?

@neongreen
Copy link
Author

neongreen commented Sep 20, 2018 via email

@LeifW
Copy link
Contributor

LeifW commented Sep 20, 2018

Maybe related to #475?
I was getting a memory leak because apparently things are not freed until you call responseBody on the response (or runResourceT) - and when the responses are just things like 202's or 204's (no content) responseBody is not called. I also talked about it some in the Amazonka gitter channel. My workaround was to just runResourceT on every request.

@neongreen
Copy link
Author

neongreen commented Sep 21, 2018

My workaround was to just runResourceT on every request.

This is basically what amazonka-1.6.0 is doing, but that's broken in the presence of streaming (connections are being closed too early), see #466

@dsturnbull
Copy link
Contributor

Another workaround is to call deleteMessageBatch even when you are just acking a single message. deleteMessageBatch has a response type, which causes the response body to be evaluated and thus no connection leak.

@neongreen
Copy link
Author

neongreen commented Sep 24, 2018 via email

@neongreen
Copy link
Author

neongreen commented Sep 25, 2018

@brendanhay I intend to make a pull-request for this, but I'd love if you (or somebody else) could look at the issue briefly and tell me whether just fixing the receive* family of functions [1] would be enough or you anticipate that more functions would have to be fixed.

[1]

receiveNull :: (MonadResource m, MonadThrow m)
=> Rs a
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveNull rs _ = stream $ \_ _ x ->
liftResourceT (x `connect` pure (Right rs))
receiveEmpty :: (MonadResource m, MonadThrow m)
=> (Int -> ResponseHeaders -> () -> Either String (Rs a))
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveEmpty f _ = stream $ \s h x ->
liftResourceT (x `connect` pure (f s h ()))
receiveXMLWrapper :: (MonadResource m, MonadThrow m)
=> Text
-> (Int -> ResponseHeaders -> [Node] -> Either String (Rs a))
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveXMLWrapper n f = receiveXML (\s h x -> x .@ n >>= f s h)
receiveXML :: (MonadResource m, MonadThrow m)
=> (Int -> ResponseHeaders -> [Node] -> Either String (Rs a))
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveXML = deserialise decodeXML
receiveJSON :: (MonadResource m, MonadThrow m)
=> (Int -> ResponseHeaders -> Object -> Either String (Rs a))
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveJSON = deserialise eitherDecode'
receiveBytes :: (MonadResource m, MonadThrow m)
=> (Int -> ResponseHeaders -> ByteString -> Either String (Rs a))
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveBytes = deserialise (Right . LBS.toStrict)
receiveBody :: (MonadResource m, MonadThrow m)
=> (Int -> ResponseHeaders -> RsBody -> Either String (Rs a))
-> Logger
-> Service
-> Proxy a
-> ClientResponse
-> m (Response a)
receiveBody f _ = stream $ \s h x -> pure (f s h (RsBody x))

mheinzel added a commit to wireapp/wire-server that referenced this issue Feb 17, 2020
The two issues mentioned in our previous snapshot are closed now:
brendanhay/amazonka#466
brendanhay/amazonka#490
mheinzel added a commit to wireapp/wire-server that referenced this issue Feb 17, 2020
The two issues mentioned in our previous snapshot are closed now:
    brendanhay/amazonka#466
    brendanhay/amazonka#490
mheinzel added a commit to wireapp/wire-server that referenced this issue Feb 17, 2020
* snapshots: add sha256 and size to wireapp/amazonka

* pin amazonka to brendanhay/develop

The two issues mentioned in our previous snapshot are closed now:
    brendanhay/amazonka#466
    brendanhay/amazonka#490
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants