Enhancement : Helidon Jersey Connector : Not reading the response fully causes connection to be discarded

[vasanth-bhat]

Environment Details

• Helidon Version: 4.0.8
• Helidon SE or Helidon MP : Both
• JDK version: 21
• OS: Linux

Helidon connector does not reuse connection , when the Response is not Full Read , even when the client program explicitly closes the response. We again end up creating a new request each connection. Even when we are reusing the same Jersey client instance in the client code across requests.

Code Snippet is something like below where just the status is read from response and response is closed..

public  int  ping(String url) {  
         int status;      
         WebTarget webTarget = client.target(url); 
         try(Response response = webTarget.request().accept(MediaType.APPLICATION_JSON).get()){
             status = response.getStatus();
         }
         catch( .) {        
         }
         return status;
}

In the profile , we see that. the Http1ConenctionCache.keepAliveConenction() does not find the connection in the cache always ends up creating a new connection.

We debugged the issue further , and found that the connection was never getting added to the cache.
The adding to the cache happened only when connection.releaseResource() is invoked. This is invoked only when the response is fully read. ( entityFullyRead =true). rr in case where there is no response body. (Content-Length ‘0’). All other cases connection.closeResource() Is invoked which closes the connection.

    @Override
    public void close() {
        if (closed.compareAndSet(false, true)) {
            try {
                if (headers().contains(HeaderValues.CONNECTION_CLOSE)) {
                    connection.closeResource();
                } else {
                    **if (entityFullyRead || entityLength == 0) {**
                        connection.releaseResource();
                    } else {
                        connection.closeResource();
                    }
                }
            } finally {
                whenComplete.complete(null);
            }
        }
    }

[vasanth-bhat]

To test further we added additional line to read and discard the response.

public  int  check(String url) {  
         int status;      
         WebTarget webTarget = client.target(url); 
         try(Response response = webTarget.request().accept(MediaType.APPLICATION_JSON).get()){
             status = response.getStatus();
             **String responseStr = response.readEntity(String.class);**
         }
         catch( .) {        
         }
         return status;
}

Now. We see that connections are re-used and. there is improvement in performance. Below is supporting data from our tests. We see 29% improvement in throughput while using 22.6% less CPU.

[vasanth-bhat]

The creation of new connections an be very expensive when it involves HTTPS end points, that involves SSL handshake.

While we could say that it’s application coding issue that response is not read , it’s not uncommon for service developers to read only status or partially read the response and not read the full Response.

When close() is called on the implementation of Response() , it’s clear that there won’t be further use of that Response by the app. At this point, the implementation of close() , For ex : For example Http1ClientResponseImpl.close(), method of the Response, fully read the response and re-use the connection instead of closing the physical connection .

[vasanth-bhat]

It looks like the HttpURLConenction in JDK code , does seem to read the remaining response, while doing a reset of the connection for reuse.

https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java#L2986

[spericas]

@vasanth-bhat Seems less problematic to support this if responses are length-prefixed (include Content-Length). Is that true in your use case?

[vasanth-bhat]

@spericas
In our specific use-case, these were simple GET requests and did include Content-Length Header in Response.

Her is my thought ,
We could support this approach of connection re-use with reading remaining responses where possible.

For cases where it's not feasible like, where Response is chunked ( Content-Length is not specified) , the connection is not re-used and WARNING Is logged about connection not being reused , so that app developers can see and take appropriate actions.

[logovind]

We have have verified with the official fix, we no longer see the issue and the connection is reused with helidon connector even though we are not reading the response entity.