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

Threads created by aws/client are never cleaned up #80

Closed
levand opened this issue Jun 6, 2019 · 5 comments
Closed

Threads created by aws/client are never cleaned up #80

levand opened this issue Jun 6, 2019 · 5 comments
Labels
bug Something isn't working http-client

Comments

@levand
Copy link

levand commented Jun 6, 2019

Invoking cognitect.aws.client.api/client causes the underlying HTTP library to create a pool of ~9 threads (which seems excessive itself, but that's a different issue). These are never released, even when the returned client object goes out of scope.

Eventually, in applications that make repeated calls to client, the JVM will hit the OS limit on thread creation and crash.

Workaround: memoize invocations of the client function, or otherwise ensure that the client function isn't invoked more than once for each service.

@dchelimsky dchelimsky added the bug Something isn't working label Jun 7, 2019
@thenonameguy
Copy link

thenonameguy commented Jun 25, 2019

This is a major problem for us, this has caused thousands of threads to be created:
Screen Shot 2019-06-25 at 1 52 17 PM
Screen Shot 2019-06-25 at 1 52 02 PM

We will be working on replacing the internal http client with ours as mentioned:
#82 (comment)

Here are is the initial step at decomplecting some parts of the internals:
whitepages#1
It should address most of the http-client related issues. With these changes we can allocate just one cognitect.http-client instance to multiple aws/clients.

@thenonameguy
Copy link

Even with the above PR we were seeing a constant increase of threads, after some digging I found the cause:

#(http/submit (http/create {}) (request-map (URI. uri)))

Since the credentials are being refetched this causes a nice constant rate of threads:
Screen Shot 2019-06-27 at 5 44 56 PM

@dchelimsky
Copy link
Contributor

@thenonameguy thanks for digging out the metadata utils issue. We'll definitely address that (probably in the next release). As for the rest, we don't accept PRs for this project. We are currently working on an interface to supply your own http client, and we do plan to address some of the other issues your PR deals with.

@dchelimsky
Copy link
Contributor

@levand there is already a cognitect.aws.client.api/stop fn that shuts down resources on an aws-api client (and its underlying http-client). This was broken due to the bug that @thenonameguy fished out (see comments above), but will be fixed in the coming release. Additionally, we're adding a default-http-client fn that you can use to create an http-client and then pass it to the constructor of each of your aws-api clients in order to share a single client. We didn't want to do that implicitly because we want you to able to tune this as you like (e.g. you may want to 5 clients, each with their own underlying http-client, but not 50).

@dchelimsky
Copy link
Contributor

Fixed in 0.8.335.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working http-client
Projects
None yet
Development

No branches or pull requests

3 participants