sttp
sttp copied to clipboard
[sttp-finagle] Adds concurrent cache to prevent Finagle client leak
sttp-finagle creates a new Finagle HTTP client upon each STTP request, causing a client leak that will result in eventual GC and resolution issues for dests that reflect large clusters.
This pull request adds a ConcurrentHashMap[String, Service[http.Request, FResponse]] to sttp-finagle, keyed upon the Finagle dest. This change will pair a single Finagle client to each backend dest, resolving this issue. I've also added a .jvmopts file to the root of this repository to address some OOM issues I ran into during the execution of sbt test.
Let me know what you think and thanks very much for the consideration!
Before submitting pull request:
- [x] Check if the project compiles by running
sbt compile - [x] Verify docs compilation by running
sbt compileDocs - [x] Check if tests pass by running
sbt test - [x] Format code by running
sbt scalafmt
Could you please elaborate more on the problem as I am not sure what it exactly is. I am not sure but GC here should not be a problem as I think that client is still referenced so it should not be collected.
I think if we wanted such a cache we would need to expose it somehow so that it can be cleared, or at least allow the user to provide a cache implementation. Alternatively the finagle clients could be closed after each request (if a non-standard timeout is given).
Shouldn't the clients be cached by the timeout in the first place? Why are the new clients created, is it always per-host or per-settings?