Move perserver waterline to a niws config (#1754)

* Move perserver waterline to a niws config * Switch test to use niws property
Netflix · Sep 9, 2024 · 66e6cb9 · 66e6cb9
1 parent 412d127
commit 66e6cb9
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 26 deletions.
diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolConfigImpl.java b/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolConfigImpl.java
@@ -16,6 +16,7 @@
 
 package com.netflix.zuul.netty.connectionpool;
 
+import com.netflix.client.config.CommonClientConfigKey;
 import com.netflix.client.config.IClientConfig;
 import com.netflix.client.config.IClientConfigKey;
 import com.netflix.config.CachedDynamicBooleanProperty;
@@ -33,12 +34,18 @@ public class ConnectionPoolConfigImpl implements ConnectionPoolConfig {
     private static final int DEFAULT_CONNECT_TIMEOUT = 500;
     private static final int DEFAULT_IDLE_TIMEOUT = 60000;
     private static final int DEFAULT_MAX_CONNS_PER_HOST = 50;
+    private static final int DEFAULT_PER_SERVER_WATERLINE = 4;
+
+    /**
+     * NOTE that each eventloop has its own connection pool per host, and this is applied per event-loop.
+     */
+    public static final IClientConfigKey<Integer> PER_SERVER_WATERLINE =
+            new CommonClientConfigKey<>("PerServerWaterline", DEFAULT_PER_SERVER_WATERLINE) {};
 
     private final OriginName originName;
     private final IClientConfig clientConfig;
 
     private final CachedDynamicIntProperty MAX_REQUESTS_PER_CONNECTION;
-    private final CachedDynamicIntProperty PER_SERVER_WATERLINE;
 
     private final CachedDynamicBooleanProperty SOCKET_KEEP_ALIVE;
     private final CachedDynamicBooleanProperty TCP_NO_DELAY;
@@ -54,10 +61,6 @@ public ConnectionPoolConfigImpl(final OriginName originName, IClientConfig clien
         this.MAX_REQUESTS_PER_CONNECTION =
                 new CachedDynamicIntProperty(niwsClientName + ".netty.client.maxRequestsPerConnection", 1000);
 
-        // NOTE that the each eventloop has it's own connection pool per host, and this is applied per event-loop.
-        this.PER_SERVER_WATERLINE =
-                new CachedDynamicIntProperty(niwsClientName + ".netty.client.perServerWaterline", 4);
-
         this.SOCKET_KEEP_ALIVE = new CachedDynamicBooleanProperty(niwsClientName + ".netty.client.TcpKeepAlive", false);
         this.TCP_NO_DELAY = new CachedDynamicBooleanProperty(niwsClientName + ".netty.client.TcpNoDelay", false);
 
@@ -92,7 +95,7 @@ public int maxConnectionsPerHost() {
 
     @Override
     public int perServerWaterline() {
-        return PER_SERVER_WATERLINE.get();
+        return clientConfig.getPropertyAsInteger(PER_SERVER_WATERLINE, DEFAULT_PER_SERVER_WATERLINE);
     }
 
     @Override

diff --git a/...core/src/test/java/com/netflix/zuul/netty/connectionpool/PerServerConnectionPoolTest.java b/...core/src/test/java/com/netflix/zuul/netty/connectionpool/PerServerConnectionPoolTest.java
@@ -20,7 +20,6 @@
 import com.netflix.appinfo.InstanceInfo.Builder;
 import com.netflix.client.config.DefaultClientConfigImpl;
 import com.netflix.client.config.IClientConfigKey.Keys;
-import com.netflix.config.ConfigurationManager;
 import com.netflix.spectator.api.Counter;
 import com.netflix.spectator.api.DefaultRegistry;
 import com.netflix.spectator.api.Registry;
@@ -42,7 +41,6 @@
 import io.netty.channel.local.LocalServerChannel;
 import io.netty.handler.codec.DecoderException;
 import io.netty.util.concurrent.Promise;
-import org.apache.commons.configuration.AbstractConfiguration;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -290,27 +288,19 @@ void releaseFromPoolButAlreadyClosed() throws InterruptedException, ExecutionExc
     void releaseFromPoolAboveHighWaterMark() throws InterruptedException, ExecutionException {
         CurrentPassport currentPassport = CurrentPassport.create();
 
-        AbstractConfiguration configuration = ConfigurationManager.getConfigInstance();
-        String propertyName = "whatever.netty.client.perServerWaterline";
-
+        clientConfig.set(ConnectionPoolConfigImpl.PER_SERVER_WATERLINE, 0);
         Promise<PooledConnection> promise = pool.acquire(CLIENT_EVENT_LOOP, currentPassport, new AtomicReference<>());
 
         PooledConnection connection = promise.sync().get();
-        try {
-            configuration.setProperty(propertyName, 0);
-            CLIENT_EVENT_LOOP
-                    .submit(() -> {
-                        assertFalse(pool.release(connection));
-                        assertEquals(1, closeAboveHighWaterMarkCounter.count());
-                        assertFalse(connection.isInPool());
-                    })
-                    .sync();
-            assertTrue(
-                    connection.getChannel().closeFuture().await(5, TimeUnit.SECONDS),
-                    "connection should have been closed");
-        } finally {
-            configuration.setProperty(propertyName, 4);
-        }
+        CLIENT_EVENT_LOOP
+                .submit(() -> {
+                    assertFalse(pool.release(connection));
+                    assertEquals(1, closeAboveHighWaterMarkCounter.count());
+                    assertFalse(connection.isInPool());
+                })
+                .sync();
+        assertTrue(
+                connection.getChannel().closeFuture().await(5, TimeUnit.SECONDS), "connection should have been closed");
     }
 
     @Test