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.

Sign up for GitHub

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

Jump to bottom

Replace remaining FPs in ConnectionPoolConfigImpl with niws style #1784

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Replace remaining FPs in ConnectionPoolConfigImpl with niws style #1784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae7b882
Switch ConnectionPoolConfigImpl to only use niws configuration
jguerra May 10, 2024
3ac14a8
Add unit tests
jguerra Jun 20, 2024
41568b5
Import order
jguerra Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 33 additions & 37 deletions zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolConfigImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.config.IClientConfigKey;
import com.netflix.config.CachedDynamicBooleanProperty;
import com.netflix.config.CachedDynamicIntProperty;
import com.netflix.zuul.origins.OriginName;
import java.util.Objects;

Expand All @@ -29,49 +27,47 @@
*/
public class ConnectionPoolConfigImpl implements ConnectionPoolConfig {

private static final int DEFAULT_BUFFER_SIZE = 32 * 1024;
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;
static final int DEFAULT_BUFFER_SIZE = 32 * 1024;
static final int DEFAULT_CONNECT_TIMEOUT = 500;
static final int DEFAULT_IDLE_TIMEOUT = 60000;
static final int DEFAULT_MAX_CONNS_PER_HOST = 50;
static final int DEFAULT_PER_SERVER_WATERLINE = 4;
static final int DEFAULT_MAX_REQUESTS_PER_CONNECTION = 1000;

// TODO(argha-c): Document why these values were chosen, as opposed to defaults of 32k/64k
static final int DEFAULT_WRITE_BUFFER_HIGH_WATER_MARK = 32 * 1024;
static final int DEFAULT_WRITE_BUFFER_LOW_WATER_MARK = 8 * 1024;

/**
* 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) {};
new CommonClientConfigKey<>("PerServerWaterline") {};

public static final IClientConfigKey<Boolean> CLOSE_ON_CIRCUIT_BREAKER =
new CommonClientConfigKey<>("CloseOnCircuitBreaker", true) {};
new CommonClientConfigKey<>("CloseOnCircuitBreaker") {};

private final OriginName originName;
private final IClientConfig clientConfig;
public static final IClientConfigKey<Integer> MAX_REQUESTS_PER_CONNECTION =
new CommonClientConfigKey<>("maxRequestsPerConnection") {};

private final CachedDynamicIntProperty MAX_REQUESTS_PER_CONNECTION;
public static final IClientConfigKey<Boolean> TCP_KEEP_ALIVE = new CommonClientConfigKey<>("TcpKeepAlive") {};

private final CachedDynamicBooleanProperty SOCKET_KEEP_ALIVE;
private final CachedDynamicBooleanProperty TCP_NO_DELAY;
private final CachedDynamicIntProperty WRITE_BUFFER_HIGH_WATER_MARK;
private final CachedDynamicIntProperty WRITE_BUFFER_LOW_WATER_MARK;
private final CachedDynamicBooleanProperty AUTO_READ;
public static final IClientConfigKey<Boolean> TCP_NO_DELAY = new CommonClientConfigKey<>("TcpKeepAlive") {};

public ConnectionPoolConfigImpl(final OriginName originName, IClientConfig clientConfig) {
this.originName = Objects.requireNonNull(originName, "originName");
String niwsClientName = originName.getNiwsClientName();
this.clientConfig = clientConfig;
public static final IClientConfigKey<Boolean> AUTO_READ = new CommonClientConfigKey<>("AutoRead") {};

this.MAX_REQUESTS_PER_CONNECTION =
new CachedDynamicIntProperty(niwsClientName + ".netty.client.maxRequestsPerConnection", 1000);
public static final IClientConfigKey<Integer> WRITE_BUFFER_HIGH_WATER_MARK =
new CommonClientConfigKey<>("WriteBufferHighWaterMark") {};

this.SOCKET_KEEP_ALIVE = new CachedDynamicBooleanProperty(niwsClientName + ".netty.client.TcpKeepAlive", false);
this.TCP_NO_DELAY = new CachedDynamicBooleanProperty(niwsClientName + ".netty.client.TcpNoDelay", false);
public static final IClientConfigKey<Integer> WRITE_BUFFER_LOW_WATER_MARK =
new CommonClientConfigKey<>("WriteBufferLowWaterMark") {};

// TODO(argha-c): Document why these values were chosen, as opposed to defaults of 32k/64k
this.WRITE_BUFFER_HIGH_WATER_MARK =
new CachedDynamicIntProperty(niwsClientName + ".netty.client.WriteBufferHighWaterMark", 32 * 1024);
this.WRITE_BUFFER_LOW_WATER_MARK =
new CachedDynamicIntProperty(niwsClientName + ".netty.client.WriteBufferLowWaterMark", 8 * 1024);
this.AUTO_READ = new CachedDynamicBooleanProperty(niwsClientName + ".netty.client.AutoRead", false);
private final OriginName originName;
private final IClientConfig clientConfig;

public ConnectionPoolConfigImpl(final OriginName originName, IClientConfig clientConfig) {
this.originName = Objects.requireNonNull(originName, "originName");
this.clientConfig = clientConfig;
}

@Override
Expand All @@ -86,7 +82,7 @@ public int getConnectTimeout() {

@Override
public int getMaxRequestsPerConnection() {
return MAX_REQUESTS_PER_CONNECTION.get();
return clientConfig.getPropertyAsInteger(MAX_REQUESTS_PER_CONNECTION, DEFAULT_MAX_REQUESTS_PER_CONNECTION);
}

@Override
Expand All @@ -108,12 +104,12 @@ public int getIdleTimeout() {

@Override
public boolean getTcpKeepAlive() {
return SOCKET_KEEP_ALIVE.get();
return clientConfig.getPropertyAsBoolean(TCP_KEEP_ALIVE, false);
}

@Override
public boolean getTcpNoDelay() {
return TCP_NO_DELAY.get();
return clientConfig.getPropertyAsBoolean(TCP_NO_DELAY, false);
}

@Override
Expand All @@ -128,17 +124,17 @@ public int getTcpSendBufferSize() {

@Override
public int getNettyWriteBufferHighWaterMark() {
return WRITE_BUFFER_HIGH_WATER_MARK.get();
return clientConfig.getPropertyAsInteger(WRITE_BUFFER_HIGH_WATER_MARK, DEFAULT_WRITE_BUFFER_HIGH_WATER_MARK);
}

@Override
public int getNettyWriteBufferLowWaterMark() {
return WRITE_BUFFER_LOW_WATER_MARK.get();
return clientConfig.getPropertyAsInteger(WRITE_BUFFER_LOW_WATER_MARK, DEFAULT_WRITE_BUFFER_LOW_WATER_MARK);
}

@Override
public boolean getNettyAutoRead() {
return AUTO_READ.get();
return clientConfig.getPropertyAsBoolean(AUTO_READ, false);
}

@Override
Expand Down
218 changes: 218 additions & 0 deletions ...ore/src/test/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolConfigImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
/*
* Copyright 2024 Netflix, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.netflix.zuul.netty.connectionpool;

import static com.netflix.zuul.netty.connectionpool.ConnectionPoolConfigImpl.MAX_REQUESTS_PER_CONNECTION;
import static com.netflix.zuul.netty.connectionpool.ConnectionPoolConfigImpl.PER_SERVER_WATERLINE;
import static com.netflix.zuul.netty.connectionpool.ConnectionPoolConfigImpl.TCP_KEEP_ALIVE;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.netflix.client.config.DefaultClientConfigImpl;
import com.netflix.client.config.IClientConfigKey;
import com.netflix.zuul.origins.OriginName;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

/**
* @author Justin Guerra
* @since 6/20/24
*/
class ConnectionPoolConfigImplTest {

private ConnectionPoolConfig connectionPoolConfig;
private DefaultClientConfigImpl clientConfig;

@BeforeEach
void setup() {
OriginName originName = OriginName.fromVip("whatever");
clientConfig = DefaultClientConfigImpl.getEmptyConfig();
connectionPoolConfig = new ConnectionPoolConfigImpl(originName, clientConfig);
}

@Test
void testGetConnectTimeout() {
assertEquals(ConnectionPoolConfigImpl.DEFAULT_CONNECT_TIMEOUT, connectionPoolConfig.getConnectTimeout());
}

@Test
void testGetConnectTimeoutOverride() {
clientConfig.set(IClientConfigKey.Keys.ConnectTimeout, 1000);
assertEquals(1000, connectionPoolConfig.getConnectTimeout());
}

@Test
void testGetMaxRequestsPerConnection() {
assertEquals(
ConnectionPoolConfigImpl.DEFAULT_MAX_REQUESTS_PER_CONNECTION,
connectionPoolConfig.getMaxRequestsPerConnection());
}

@Test
void testGetMaxRequestsPerConnectionOverride() {
clientConfig.set(MAX_REQUESTS_PER_CONNECTION, 2000);
assertEquals(2000, connectionPoolConfig.getMaxRequestsPerConnection());
}

@Test
void testMaxConnectionsPerHost() {
assertEquals(ConnectionPoolConfigImpl.DEFAULT_MAX_CONNS_PER_HOST, connectionPoolConfig.maxConnectionsPerHost());
}

@Test
void testMaxConnectionsPerHostOverride() {
clientConfig.set(IClientConfigKey.Keys.MaxConnectionsPerHost, 60);
assertEquals(60, connectionPoolConfig.maxConnectionsPerHost());
}

@Test
void testPerServerWaterline() {
assertEquals(ConnectionPoolConfigImpl.DEFAULT_PER_SERVER_WATERLINE, connectionPoolConfig.perServerWaterline());
}

@Test
void testPerServerWaterlineOverride() {
clientConfig.set(PER_SERVER_WATERLINE, 5);
assertEquals(5, connectionPoolConfig.perServerWaterline());
}

@Test
void testGetIdleTimeout() {
assertEquals(ConnectionPoolConfigImpl.DEFAULT_IDLE_TIMEOUT, connectionPoolConfig.getIdleTimeout());
}

@Test
void testGetIdleTimeoutOverride() {
clientConfig.set(IClientConfigKey.Keys.ConnIdleEvictTimeMilliSeconds, 70000);
assertEquals(70000, connectionPoolConfig.getIdleTimeout());
}

@Test
void testGetTcpKeepAlive() {
assertFalse(connectionPoolConfig.getTcpKeepAlive());
}

@Test
void testGetTcpKeepAliveOverride() {
clientConfig.set(TCP_KEEP_ALIVE, true);
assertTrue(connectionPoolConfig.getTcpKeepAlive());
}

@Test
void testGetTcpNoDelay() {
assertFalse(connectionPoolConfig.getTcpNoDelay());
}

@Test
void testGetTcpNoDelayOverride() {
clientConfig.set(ConnectionPoolConfigImpl.TCP_NO_DELAY, true);
assertTrue(connectionPoolConfig.getTcpNoDelay());
}

@Test
void testGetTcpReceiveBufferSize() {
assertEquals(ConnectionPoolConfigImpl.DEFAULT_BUFFER_SIZE, connectionPoolConfig.getTcpReceiveBufferSize());
}

@Test
void testGetTcpReceiveBufferSizeOverride() {
clientConfig.set(IClientConfigKey.Keys.ReceiveBufferSize, 40000);
assertEquals(40000, connectionPoolConfig.getTcpReceiveBufferSize());
}

@Test
void testGetTcpSendBufferSize() {
assertEquals(ConnectionPoolConfigImpl.DEFAULT_BUFFER_SIZE, connectionPoolConfig.getTcpSendBufferSize());
}

@Test
void testGetTcpSendBufferSizeOverride() {
clientConfig.set(IClientConfigKey.Keys.SendBufferSize, 40000);
assertEquals(40000, connectionPoolConfig.getTcpSendBufferSize());
}

@Test
void testGetNettyWriteBufferHighWaterMark() {
assertEquals(
ConnectionPoolConfigImpl.DEFAULT_WRITE_BUFFER_HIGH_WATER_MARK,
connectionPoolConfig.getNettyWriteBufferHighWaterMark());
}

@Test
void testGetNettyWriteBufferHighWaterMarkOverride() {
clientConfig.set(ConnectionPoolConfigImpl.WRITE_BUFFER_HIGH_WATER_MARK, 40000);
assertEquals(40000, connectionPoolConfig.getNettyWriteBufferHighWaterMark());
}

@Test
void testGetNettyWriteBufferLowWaterMark() {
assertEquals(
ConnectionPoolConfigImpl.DEFAULT_WRITE_BUFFER_LOW_WATER_MARK,
connectionPoolConfig.getNettyWriteBufferLowWaterMark());
}

@Test
void testGetNettyWriteBufferLowWaterMarkOverride() {
clientConfig.set(ConnectionPoolConfigImpl.WRITE_BUFFER_LOW_WATER_MARK, 10000);
assertEquals(10000, connectionPoolConfig.getNettyWriteBufferLowWaterMark());
}

@Test
void testGetNettyAutoRead() {
assertFalse(connectionPoolConfig.getNettyAutoRead());
}

@Test
void testGetNettyAutoReadOverride() {
clientConfig.set(ConnectionPoolConfigImpl.AUTO_READ, true);
assertTrue(connectionPoolConfig.getNettyAutoRead());
}

@Test
void testIsSecure() {
assertFalse(connectionPoolConfig.isSecure());
}

@Test
void testIsSecureOverride() {
clientConfig.set(IClientConfigKey.Keys.IsSecure, true);
assertTrue(connectionPoolConfig.isSecure());
}

@Test
void testUseIPAddrForServer() {
assertTrue(connectionPoolConfig.useIPAddrForServer());
}

@Test
void testUseIPAddrForServerOverride() {
clientConfig.set(IClientConfigKey.Keys.UseIPAddrForServer, false);
assertFalse(connectionPoolConfig.useIPAddrForServer());
}

@Test
void testIsCloseOnCircuitBreakerEnabled() {
assertTrue(connectionPoolConfig.isCloseOnCircuitBreakerEnabled());
}

@Test
void testIsCloseOnCircuitBreakerEnabledOverride() {
clientConfig.set(ConnectionPoolConfigImpl.CLOSE_ON_CIRCUIT_BREAKER, false);
assertFalse(connectionPoolConfig.isCloseOnCircuitBreakerEnabled());
}
}