You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2022/02/01 19:19:49 UTC
[kudu] branch master updated: [java] KUDU-3240 Make connection negotiation timeout configurable for Java client
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 <ye...@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 <as...@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();