Merge pull request #14622 from vespa-engine/bjorncs/vespa-http-client

Ensure content is consumed to allow connection reuse.  MERGEOK

3 files changed, 33 insertions, 35 deletions

diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
index 2417a4acf71..733e5bca424 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/config/ConnectionParams.java
@@ -47,7 +47,7 @@ public final class ConnectionParams {
         private int traceEveryXOperation = 0;
         private boolean printTraceToStdErr = true;
         private boolean useTlsConfigFromEnvironment = false;
-        private Duration connectionTimeToLive = Duration.ofSeconds(15);
+        private Duration connectionTimeToLive = Duration.ofSeconds(30);
         private Path privateKey;
         private Path certificate;
         private Path caCertificates;
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
index bd5cf761024..735ca0beb40 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
@@ -23,6 +23,7 @@ import org.apache.http.entity.InputStreamEntity;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.message.BasicHeader;
+import org.apache.http.util.EntityUtils;
 
 import javax.net.ssl.SSLContext;
 import java.io.ByteArrayInputStream;
@@ -246,24 +247,21 @@ class ApacheGatewayConnection implements GatewayConnection {
     }
 
     private InputStream executePost(HttpPost httpPost) throws ServerResponseException, IOException {
-        HttpResponse response;
-        try {
-            if (httpClient == null)
-                throw new IOException("Trying to executePost while not having a connection/http client");
-            response = httpClient.execute(httpPost);
-        } catch (Exception e) {
-            httpPost.abort();
-            throw e;
-        }
+        if (httpClient == null)
+            throw new IOException("Trying to executePost while not having a connection/http client");
+        HttpResponse response = httpClient.execute(httpPost);
         try {
             verifyServerResponseCode(response);
             verifyServerVersion(response.getFirstHeader(Headers.VERSION));
             verifySessionHeader(response.getFirstHeader(Headers.SESSION_ID));
         } catch (ServerResponseException e) {
-            httpPost.abort();
+            // Ensure response is consumed to allow connection reuse later on
+            EntityUtils.consumeQuietly(response.getEntity());
             throw e;
         }
-        return response.getEntity().getContent();
+        // Consume response now to allow connection to be reused immediately
+        byte[] responseData = EntityUtils.toByteArray(response.getEntity());
+        return responseData == null ? null : new ByteArrayInputStream(responseData);
     }
 
     private void verifyServerResponseCode(HttpResponse response) throws ServerResponseException {
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java
index 240adc29197..ef90d6853b1 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/communication/IOThreadTest.java
@@ -101,36 +101,36 @@ public class IOThreadTest {
         tester.send("doc1");
         tester.tick(1);
 
-        tester.clock().advance(Duration.ofSeconds(16)); // Default connection ttl is 15
+        tester.clock().advance(Duration.ofSeconds(31)); // Default connection ttl is 30
         tester.tick(3);
 
         assertEquals(1, ioThread.oldConnections().size());
         assertEquals(firstConnection, ioThread.oldConnections().get(0));
         assertNotSame(firstConnection, ioThread.currentConnection());
-        assertEquals(16, firstConnection.lastPollTime().toEpochMilli() / 1000);
+        assertEquals(31, firstConnection.lastPollTime().toEpochMilli() / 1000);
 
         // Check old connection poll pattern (exponential backoff)
-        assertLastPollTimeWhenAdvancing(16, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(18, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(18, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(18, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(18, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(22, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
-        assertLastPollTimeWhenAdvancing(30, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(31, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(33, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(33, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(33, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(33, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(37, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
+        assertLastPollTimeWhenAdvancing(45, 1, firstConnection, tester);
 
         tester.clock().advance(Duration.ofSeconds(200));
         tester.tick(1);