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

Http2Client: added clients cache #2405

Merged
merged 1 commit into from
May 13, 2024

Conversation

illia2m4ch
Copy link
Contributor

In case feign.Request.Options settings differ from the default native client java.net.http.HttpClient, a new client is created for each request, which leads to creation of a huge number of clients in memory.
Added a map for caching soft references of clients.

@velo
Copy link
Member

velo commented May 6, 2024

This makes me wonder, shouldn't feign be caching and reusing client at core level instead of the implementation?

@illia2m4ch
Copy link
Contributor Author

This makes me wonder, shouldn't feign be caching and reusing client at core level instead of the implementation?

As I understand it, the problem is that feign has a rich set of parameters for each request (e.g. followRedirects), which may not match the parameters of the native client. Because of this, you need to create a new native client with the necessary parameters. That is, if a native client supports all the same parameters as feign, it does not have this problem.

Therefore, each native client will have its own key (its own set of parameters that it does not maintain at the request level) to retrieve the client. So I'm not sure if there is a better abstract solution at the core level of feign.

@velo velo merged commit 5e91441 into OpenFeign:master May 13, 2024
3 checks passed
velo pushed a commit that referenced this pull request Oct 7, 2024
velo pushed a commit that referenced this pull request Oct 8, 2024
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 this pull request may close these issues.

2 participants