This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push: new ee5e856 [java] KUDU-3240 Make connection negotiation timeout configurable for Java client ee5e856 is described below commit ee5e856b93e6efff415a81955071465dce0e2ded Author: yejiabao <yejia...@huawei.com> AuthorDate: Sun Jan 30 20:22:01 2022 +0800 [java] KUDU-3240 Make connection negotiation timeout configurable for Java client This patch addresses KUDU-3240 for Kudu Java client, making the timeout for the connection negotiation configurable. Change-Id: Iac1fbc2df3118cbab2f87751c5e21346478f72f9 Reviewed-on: http://gerrit.cloudera.org:8080/18183 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <aser...@cloudera.com> --- .../java/org/apache/kudu/client/AsyncKuduClient.java | 16 +++++++++++++++- .../src/main/java/org/apache/kudu/client/Connection.java | 8 +++++--- .../java/org/apache/kudu/client/ConnectionCache.java | 9 +++++++-- .../src/main/java/org/apache/kudu/client/KuduClient.java | 12 ++++++++++++ .../test/java/org/apache/kudu/client/TestTimeouts.java | 10 ++-------- 5 files changed, 41 insertions(+), 14 deletions(-) diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java index 3e2c466..3a0780c 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java @@ -303,6 +303,7 @@ public class AsyncKuduClient implements AutoCloseable { public static final long INVALID_TXN_ID = -1; public static final long DEFAULT_OPERATION_TIMEOUT_MS = 30000; public static final long DEFAULT_KEEP_ALIVE_PERIOD_MS = 15000; // 25% of the default scanner ttl. + public static final long DEFAULT_NEGOTIATION_TIMEOUT_MS = 10000; private static final long MAX_RPC_ATTEMPTS = 100; /** @@ -427,7 +428,7 @@ public class AsyncKuduClient implements AutoCloseable { this.securityContext = new SecurityContext(); this.connectionCache = new ConnectionCache(securityContext, bootstrap, b.saslProtocolName, b.requireAuthentication, !b.encryptionPolicy.equals(EncryptionPolicy.OPTIONAL), - b.encryptionPolicy.equals(EncryptionPolicy.REQUIRED)); + b.encryptionPolicy.equals(EncryptionPolicy.REQUIRED), b.defaultNegotiationTimeoutMs); this.tokenReacquirer = new AuthnTokenReacquirer(this); this.authzTokenCache = new AuthzTokenCache(this); } @@ -2764,6 +2765,7 @@ public class AsyncKuduClient implements AutoCloseable { private final List<HostAndPort> masterAddresses; private long defaultAdminOperationTimeoutMs = DEFAULT_OPERATION_TIMEOUT_MS; private long defaultOperationTimeoutMs = DEFAULT_OPERATION_TIMEOUT_MS; + private long defaultNegotiationTimeoutMs = DEFAULT_NEGOTIATION_TIMEOUT_MS; private final HashedWheelTimer timer = new HashedWheelTimer( new ThreadFactoryBuilder().setDaemon(true).build(), 20, MILLISECONDS); @@ -2834,6 +2836,18 @@ public class AsyncKuduClient implements AutoCloseable { } /** + * Sets the default timeout used for connection negotiation. + * Optional. + * If not provided, defaults to 10s. + * @param timeoutMs a timeout in milliseconds + * @return this builder + */ + public AsyncKuduClientBuilder connectionNegotiationTimeoutMs(long timeoutMs) { + this.defaultNegotiationTimeoutMs = timeoutMs; + return this; + } + + /** * Socket read timeouts are no longer used in the Java client and have no effect. * Setting this has no effect. * @param timeoutMs a timeout in milliseconds diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java index 0cc7900..cd8ad8c 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java @@ -138,7 +138,7 @@ class Connection extends SimpleChannelInboundHandler<Object> { }; private static final String NEGOTIATION_TIMEOUT_HANDLER = "negotiation-timeout-handler"; - private static final long NEGOTIATION_TIMEOUT_MS = 10000; + private final long negotiationTimeoutMs; /** Lock to guard access to some of the fields below. */ private final ReentrantLock lock = new ReentrantLock(); @@ -196,7 +196,8 @@ class Connection extends SimpleChannelInboundHandler<Object> { String saslProtocolName, boolean requireAuthentication, boolean requireEncryption, - boolean encryptLoopback) { + boolean encryptLoopback, + long negotiationTimeoutMs) { this.serverInfo = serverInfo; this.securityContext = securityContext; this.saslProtocolName = saslProtocolName; @@ -207,6 +208,7 @@ class Connection extends SimpleChannelInboundHandler<Object> { this.requireAuthentication = requireAuthentication; this.requireEncryption = requireEncryption; this.encryptLoopback = encryptLoopback; + this.negotiationTimeoutMs = negotiationTimeoutMs; } /** {@inheritDoc} */ @@ -837,7 +839,7 @@ class Connection extends SimpleChannelInboundHandler<Object> { // Add a socket read timeout handler to function as a timeout for negotiation. // The handler will be removed once the connection is negotiated. pipeline.addLast(NEGOTIATION_TIMEOUT_HANDLER, - new ReadTimeoutHandler(NEGOTIATION_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + new ReadTimeoutHandler(negotiationTimeoutMs, TimeUnit.MILLISECONDS)); pipeline.addLast("kudu-handler", Connection.this); } } diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java index 706bc98..17fac96 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java @@ -64,6 +64,8 @@ class ConnectionCache { private boolean encryptLoopback; + private long negotiationTimeoutMs; + /** * Container mapping server IP/port into the established connection from the client to the * server. It may be up to two connections per server: one established with secondary @@ -79,13 +81,15 @@ class ConnectionCache { String saslProtocolName, boolean requireAuthentication, boolean requireEncryption, - boolean encryptLoopback) { + boolean encryptLoopback, + long negotiationTimeoutMs) { this.securityContext = securityContext; this.bootstrap = bootstrap; this.saslProtocolName = saslProtocolName; this.requireAuthentication = requireAuthentication; this.requireEncryption = requireEncryption; this.encryptLoopback = encryptLoopback; + this.negotiationTimeoutMs = negotiationTimeoutMs; } /** @@ -142,7 +146,8 @@ class ConnectionCache { saslProtocolName, requireAuthentication, requireEncryption, - encryptLoopback); + encryptLoopback, + negotiationTimeoutMs); connections.add(result); // There can be at most 2 connections to the same destination: one with primary and another // with secondary credentials. diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java index 3cc1d31..5631815 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java @@ -536,6 +536,18 @@ public class KuduClient implements AutoCloseable { } /** + * Sets the default timeout used for connection negotiation. + * Optional. + * If not provided, defaults to 10s. + * @param timeoutMs a timeout in milliseconds + * @return this builder + */ + public KuduClientBuilder connectionNegotiationTimeoutMs(long timeoutMs) { + clientBuilder.connectionNegotiationTimeoutMs(timeoutMs); + return this; + } + + /** * Socket read timeouts are no longer used in the Java client and have no effect. * Setting this has no effect. * @param timeoutMs a timeout in milliseconds diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java index 5e1289b..99ef32e 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; @@ -124,21 +123,16 @@ public class TestTimeouts { } /** - * KUDU-1868: This tests that negotiation can time out on the client side. It passes if the - * hardcoded negotiation timeout is lowered to 500ms. In general it is hard to get it to work - * right because injecting latency to negotiation server side affects all client connections, - * including the harness's Java client, the kudu tool used to create the test cluster, and the - * other members of the test cluster. There isn't a way to configure the kudu tool's - * negotiation timeout within a Java test, presently. + * KUDU-1868: This tests that negotiation can time out on the client side. */ @Test(timeout = 100000) - @Ignore @MasterServerConfig(flags = { "--rpc_negotiation_inject_delay_ms=1000" }) public void testClientNegotiationTimeout() throws Exception { // Make a new client so we can turn down the operation timeout-- otherwise this test takes 50s! try (KuduClient lowTimeoutsClient = new KuduClient.KuduClientBuilder(harness.getMasterAddressesAsString()) .defaultAdminOperationTimeoutMs(5000) + .connectionNegotiationTimeoutMs(500) .build()) { try { lowTimeoutsClient.getTablesList();