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

LRU cache clientSpiCache in HttpClientRequest.discoverHttpImplementation() is not working as new instances are re-created every request #8930

Closed
vasanth-bhat opened this issue Jul 1, 2024 · 1 comment
Assignees
Labels
4.x Version 4.x bug Something isn't working P2 webclient

Comments

@vasanth-bhat
Copy link

vasanth-bhat commented Jul 1, 2024

Environment Details

  • Helidon Version: 4.0.8
  • Helidon SE or Helidon MP. : Both
  • JDK version: 21
  • OS: Linux
  • Docker version (if applicable):

The caching mechanism used by. HttpClientRequest.discoverHttpImplementation() is not working. We end up paying significant cost , when the “HelidonProperties.PROTOCOL_ID” is not explicitly set in Client Config, as it ends up making a new connection on a per request basis to discover the supported HTTP version. This connection is closed ( connection.closeResource() -> socket.close() ) after retrieving the protocol details and this happens on every request .

This is a huge overhead, especially when HTTPS endpoints are involved. This will result in SSL handshake for each such connections.

It looks like the issue is because the LRU cache “clientSpiCache”. used to cache the protocol version for the endpoints is an instance member and not a static member so we end up creating new empty cache for every HTTP request. https://github.com/helidon-io/helidon/blob/main/webclient/api/src/main/java/io/helidon/webclient/api/HttpClientRequest.java#L35

public class HttpClientRequest extends ClientRequestBase<HttpClientRequest, HttpClientResponse> {
……..
private final LruCache<EndpointKey, HttpClientSpi> clientSpiCache = LruCache.create();
private final WebClient webClient;

The clientSpiCache ideally should be static member so that cached content persists across requests. We did try out tests with this change where made this as static member so that cache is reused across requests. , This. significantly improves the performance.

Based on our tests with the sample, we. see about 47% improvement in throughput, changing the clientSpiCache to static member.

@vasanth-bhat vasanth-bhat changed the title LRU cache clientSpiCache in HttpClientRequest.discoverHttpImplementation() is not workign as new instances are re-created every request LRU cache clientSpiCache in HttpClientRequest.discoverHttpImplementation() is not working as new instances are re-created every request Jul 1, 2024
@m0mus m0mus added bug Something isn't working P2 webclient 4.x Version 4.x labels Jul 1, 2024
@spericas spericas closed this as completed Jul 2, 2024
@vasanth-bhat
Copy link
Author

We have have verified taht with the official fix, we no longer seethe issue and cache is reused across requests

@m0mus m0mus added this to Backlog Aug 12, 2024
@m0mus m0mus moved this to Closed in Backlog Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x bug Something isn't working P2 webclient
Projects
Archived in project
Development

No branches or pull requests

3 participants