<fix>[core]: cap client keep-alive below agent socket timeout

core/src/main/java/org/zstack/core/CoreGlobalProperty.java

@@ -48,6 +48,9 @@ public class CoreGlobalProperty {
     public static int REST_FACADE_MAX_PER_ROUTE;
     @GlobalProperty(name = "RESTFacade.maxTotal", defaultValue = "128")
     public static int REST_FACADE_MAX_TOTAL;
+    // client keep-alive cap, must be < agent socket_timeout to avoid reusing a closed connection
+    @GlobalProperty(name = "RESTFacade.keepAliveTimeMillis", defaultValue = "5000")
+    public static int REST_FACADE_KEEPALIVE_TIME;
     /**
      * When set RestServer.maskSensitiveInfo to true, sensitive info will be
      * masked see @NoLogging.

core/src/main/java/org/zstack/core/rest/RESTFacadeImpl.java

@@ -1,6 +1,8 @@
 package org.zstack.core.rest;
 
 import org.apache.http.HttpStatus;
+import org.apache.http.conn.ConnectionKeepAliveStrategy;
+import org.apache.http.impl.client.DefaultConnectionKeepAliveStrategy;
 import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
 import org.apache.http.impl.nio.client.HttpAsyncClients;
 import org.apache.http.impl.nio.conn.PoolingNHttpClientConnectionManager;
@@ -160,6 +162,11 @@ void init() {
                 CoreGlobalProperty.REST_FACADE_MAX_TOTAL);
     }
 
+    // cap the agent-advertised keep-alive to keepAliveMs; also cap when the agent sends none (duration < 0)
+    static long cappedKeepAlive(long serverDuration, long capMs) {
+        return (serverDuration < 0 || serverDuration > capMs) ? capMs : serverDuration;
+    }
+
     // timeout are in milliseconds
     private static AsyncRestTemplate createAsyncRestTemplate(int readTimeout, int connectTimeout, int maxPerRoute, int maxTotal) {
         PoolingNHttpClientConnectionManager connectionManager;
@@ -172,8 +179,14 @@ private static AsyncRestTemplate createAsyncRestTemplate(int readTimeout, int co
         connectionManager.setDefaultMaxPerRoute(maxPerRoute);
         connectionManager.setMaxTotal(maxTotal);
 
+        // cap client keep-alive below agent socket_timeout so we never reuse a connection the agent already closed
+        final long keepAliveMs = CoreGlobalProperty.REST_FACADE_KEEPALIVE_TIME;
+        ConnectionKeepAliveStrategy keepAliveStrategy = (response, context) ->
+                cappedKeepAlive(DefaultConnectionKeepAliveStrategy.INSTANCE.getKeepAliveDuration(response, context), keepAliveMs);
+
         CloseableHttpAsyncClient httpAsyncClient = HttpAsyncClients.custom()
                 .setConnectionManager(connectionManager)
+                .setKeepAliveStrategy(keepAliveStrategy)
                 .build();
 
         HttpComponentsAsyncClientHttpRequestFactory cf = new HttpComponentsAsyncClientHttpRequestFactory(httpAsyncClient);

test/src/test/groovy/org/zstack/test/integration/core/rest/RestFacadeKeepAliveCase.groovy

@@ -0,0 +1,82 @@
+package org.zstack.test.integration.core.rest
+
+import org.springframework.http.HttpEntity
+import org.zstack.core.CoreGlobalProperty
+import org.zstack.core.rest.RESTFacadeImpl
+import org.zstack.header.errorcode.ErrorCode
+import org.zstack.header.rest.AsyncRESTCallback
+import org.zstack.test.integration.ZStackTest
+import org.zstack.testlib.EnvSpec
+import org.zstack.testlib.SubCase
+import org.zstack.testlib.WebBeanConstructor
+import org.zstack.utils.URLBuilder
+
+import java.util.concurrent.TimeUnit
+
+class RestFacadeKeepAliveCase extends SubCase {
+    EnvSpec env
+    String BASE_URL = "/test-keep-alive"
+
+    @Override
+    void clean() {
+        env.delete()
+    }
+
+    @Override
+    void setup() {
+        useSpring(ZStackTest.springSpec)
+    }
+
+    @Override
+    void environment() {
+        env = env {
+        }
+    }
+
+    @Override
+    void test() {
+        env.create {
+            testKeepAliveDefault()
+            testKeepAliveCap()
+            testAsyncPostStillWorks()
+        }
+    }
+
+    // keep-alive cap defaults to 5s, must be < agent socket_timeout
+    void testKeepAliveDefault() {
+        assert CoreGlobalProperty.REST_FACADE_KEEPALIVE_TIME == 5000
+    }
+
+    // client never keeps a connection longer than the cap, even when the agent advertises more or nothing
+    void testKeepAliveCap() {
+        assert RESTFacadeImpl.cappedKeepAlive(-1, 5000) == 5000
+        assert RESTFacadeImpl.cappedKeepAlive(10000, 5000) == 5000
+        assert RESTFacadeImpl.cappedKeepAlive(2000, 5000) == 2000
+    }
+
+    // the keep-alive-capped async client still completes a normal post
+    void testAsyncPostStillWorks() {
+        RESTFacadeImpl restf = bean(RESTFacadeImpl.class)
+
+        env.simulator(BASE_URL) { HttpEntity<String> e ->
+            return e.toString()
+        }
+
+        boolean success = false
+        String url = URLBuilder.buildHttpUrl("127.0.0.1", WebBeanConstructor.port, BASE_URL)
+        restf.asyncJsonPost(url, "ping", new AsyncRESTCallback(null) {
+            @Override
+            void fail(ErrorCode err) {
+            }
+
+            @Override
+            void success(HttpEntity<String> responseEntity) {
+                success = true
+            }
+        }, TimeUnit.MILLISECONDS, 5000)
+
+        retryInSecs {
+            assert success
+        }
+    }
+}