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 2020/11/04 05:36:20 UTC
[kudu] 01/03: [client] KUDU-2966 configure connection negotiation
timeout
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
commit 62ec1e474167d79de298cd14d9e1b975bcdbdd8a
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Sat Oct 31 00:48:48 2020 -0700
[client] KUDU-2966 configure connection negotiation timeout
This patch addresses KUDU-2966 for the Kudu C++ client, making
the timeout for the connection negotiation configurable. This patch
also contains a unit test for the newly introduced functionality.
As for the motivation for this patch: we have observed a few clusters
in the wild where connection negotiations were a bit slow when servers
are under heavy load. It would be nice to control the connection
negotiation timeout at the client side as well, so at least kudu CLI
tool could use this functionality.
The newly introduced functionality is to be used at least in kudu CLI
(will be posted in a separate follow-up patch).
Change-Id: I8573a572887be041637e28e518a7cd3a6d1f1693
Reviewed-on: http://gerrit.cloudera.org:8080/16687
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/client/client-internal.cc | 56 +++++++++++++++----------------
src/kudu/client/client-internal.h | 1 +
src/kudu/client/client-test.cc | 36 ++++++++++++++++++++
src/kudu/client/client.cc | 16 +++++++++
src/kudu/client/client.h | 20 +++++++++++
src/kudu/client/client_builder-internal.h | 6 ++--
src/kudu/rpc/messenger.cc | 4 ++-
src/kudu/rpc/messenger.h | 6 +++-
8 files changed, 111 insertions(+), 34 deletions(-)
diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index 1f23ccd..26fd9c9 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -73,7 +73,34 @@ DECLARE_int32(dns_resolver_max_threads_num);
DECLARE_uint32(dns_resolver_cache_capacity_mb);
DECLARE_uint32(dns_resolver_cache_ttl_sec);
+
using boost::container::small_vector;
+using kudu::client::internal::AsyncLeaderMasterRpc;
+using kudu::client::internal::ConnectToClusterRpc;
+using kudu::client::internal::RemoteTablet;
+using kudu::client::internal::RemoteTabletServer;
+using kudu::master::AlterTableRequestPB;
+using kudu::master::AlterTableResponsePB;
+using kudu::master::ConnectToMasterResponsePB;
+using kudu::master::CreateTableRequestPB;
+using kudu::master::CreateTableResponsePB;
+using kudu::master::DeleteTableRequestPB;
+using kudu::master::DeleteTableResponsePB;
+using kudu::master::GetTableSchemaRequestPB;
+using kudu::master::GetTableSchemaResponsePB;
+using kudu::master::IsAlterTableDoneRequestPB;
+using kudu::master::IsAlterTableDoneResponsePB;
+using kudu::master::IsCreateTableDoneRequestPB;
+using kudu::master::IsCreateTableDoneResponsePB;
+using kudu::master::ListTabletServersRequestPB;
+using kudu::master::ListTabletServersResponsePB;
+using kudu::master::MasterFeatures;
+using kudu::master::MasterServiceProxy;
+using kudu::master::TableIdentifierPB;
+using kudu::rpc::BackoffType;
+using kudu::rpc::CredentialsPolicy;
+using kudu::rpc::MessengerBuilder;
+using kudu::security::SignedTokenPB;
using std::map;
using std::pair;
using std::set;
@@ -81,6 +108,7 @@ using std::shared_ptr;
using std::string;
using std::unique_ptr;
using std::vector;
+using strings::Substitute;
namespace kudu {
@@ -88,36 +116,8 @@ namespace security {
class SignedTokenPB;
} // namespace security
-using master::AlterTableRequestPB;
-using master::AlterTableResponsePB;
-using master::ConnectToMasterResponsePB;
-using master::CreateTableRequestPB;
-using master::CreateTableResponsePB;
-using master::DeleteTableRequestPB;
-using master::DeleteTableResponsePB;
-using master::GetTableSchemaRequestPB;
-using master::GetTableSchemaResponsePB;
-using master::IsAlterTableDoneRequestPB;
-using master::IsAlterTableDoneResponsePB;
-using master::IsCreateTableDoneRequestPB;
-using master::IsCreateTableDoneResponsePB;
-using master::ListTabletServersResponsePB;
-using master::ListTabletServersRequestPB;
-using master::MasterFeatures;
-using master::MasterServiceProxy;
-using master::TableIdentifierPB;
-using rpc::BackoffType;
-using rpc::CredentialsPolicy;
-using security::SignedTokenPB;
-using strings::Substitute;
-
namespace client {
-using internal::AsyncLeaderMasterRpc;
-using internal::ConnectToClusterRpc;
-using internal::RemoteTablet;
-using internal::RemoteTabletServer;
-
Status RetryFunc(const MonoTime& deadline,
const string& retry_msg,
const string& timeout_msg,
diff --git a/src/kudu/client/client-internal.h b/src/kudu/client/client-internal.h
index 3adeb62..dc7dade 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -279,6 +279,7 @@ class KuduClient::Data {
std::vector<std::string> master_server_addrs_;
MonoDelta default_admin_operation_timeout_;
MonoDelta default_rpc_timeout_;
+ MonoDelta connection_negotiation_timeout_;
// The host port of the leader master. This is set in
// ConnectedToClusterCb, which is invoked as a callback by
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 4445a49..6849c6a 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -183,6 +183,7 @@ using kudu::itest::GetClusterId;
using kudu::master::CatalogManager;
using kudu::master::GetTableLocationsRequestPB;
using kudu::master::GetTableLocationsResponsePB;
+using kudu::rpc::MessengerBuilder;
using kudu::security::SignedTokenPB;
using kudu::client::sp::shared_ptr;
using kudu::tablet::TabletReplica;
@@ -884,6 +885,41 @@ TEST_F(ClientTest, ConnectToClusterNoMasterAddressSpecified) {
ASSERT_STR_CONTAINS(s.ToString(), "no master address specified");
}
+// Verify setting of the connection negotiation timeout (KUDU-2966).
+TEST_F(ClientTest, ConnectionNegotiationTimeout) {
+ const auto master_addr = cluster_->mini_master()->bound_rpc_addr().ToString();
+
+ // The connection negotiation timeout isn't set explicitly: it should be
+ // equal to the default setting for the RPC messenger.
+ {
+ KuduClientBuilder b;
+ b.add_master_server_addr(master_addr);
+ shared_ptr<KuduClient> c;
+ ASSERT_OK(b.Build(&c));
+ auto t = c->connection_negotiation_timeout();
+ ASSERT_TRUE(t.Initialized());
+ ASSERT_EQ(MessengerBuilder::kRpcNegotiationTimeoutMs, t.ToMilliseconds());
+ auto t_ms = c->data_->messenger_->rpc_negotiation_timeout_ms();
+ ASSERT_EQ(t_ms, t.ToMilliseconds());
+ }
+
+ // The connection negotiation timeout is set explicitly via KuduBuilder. It
+ // should be propagated to the RPC messenger and reported back correspondingly
+ // by the KuduClient::connection_negotiaton_timeout() method.
+ {
+ const MonoDelta kNegotiationTimeout = MonoDelta::FromMilliseconds(12321);
+ KuduClientBuilder b;
+ b.add_master_server_addr(master_addr);
+ b.connection_negotiation_timeout(kNegotiationTimeout);
+ shared_ptr<KuduClient> c;
+ ASSERT_OK(b.Build(&c));
+ auto t = c->connection_negotiation_timeout();
+ ASSERT_EQ(kNegotiationTimeout, t);
+ auto t_ms = c->data_->messenger_->rpc_negotiation_timeout_ms();
+ ASSERT_EQ(t_ms, t.ToMilliseconds());
+ }
+}
+
// Test that, if the master is down, we experience a network error talking
// to it (no "find the new leader master" since there's only one master).
TEST_F(ClientTest, TestMasterDown) {
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 181db49..ebc4fc1 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -304,6 +304,12 @@ KuduClientBuilder& KuduClientBuilder::default_rpc_timeout(const MonoDelta& timeo
return *this;
}
+KuduClientBuilder& KuduClientBuilder::connection_negotiation_timeout(
+ const MonoDelta& timeout) {
+ data_->connection_negotiation_timeout_ = timeout;
+ return *this;
+}
+
KuduClientBuilder& KuduClientBuilder::import_authentication_credentials(string authn_creds) {
data_->authn_creds_ = std::move(authn_creds);
return *this;
@@ -350,6 +356,10 @@ Status KuduClientBuilder::Build(shared_ptr<KuduClient>* client) {
// Init messenger.
MessengerBuilder builder("client");
+ if (data_->connection_negotiation_timeout_.Initialized()) {
+ builder.set_rpc_negotiation_timeout_ms(
+ data_->connection_negotiation_timeout_.ToMilliseconds());
+ }
if (data_->num_reactors_) {
builder.set_num_reactors(data_->num_reactors_.get());
}
@@ -632,6 +642,12 @@ const MonoDelta& KuduClient::default_rpc_timeout() const {
return data_->default_rpc_timeout_;
}
+MonoDelta KuduClient::connection_negotiation_timeout() const {
+ DCHECK(data_->messenger_);
+ return MonoDelta::FromMilliseconds(
+ data_->messenger_->rpc_negotiation_timeout_ms());
+}
+
const uint64_t KuduClient::kNoTimestamp = 0;
uint64_t KuduClient::GetLatestObservedTimestamp() const {
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 16193d3..5001543 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -263,6 +263,22 @@ class KUDU_EXPORT KuduClientBuilder {
/// @return Reference to the updated object.
KuduClientBuilder& default_rpc_timeout(const MonoDelta& timeout);
+ /// Set the timeout for negotiating a connection to a remote server.
+ ///
+ /// If not provided, the underlying messenger is created with reasonable
+ /// default. The result value could be retrieved using
+ /// @c KuduClient.connection_negotiation_timeout() after an instance of
+ /// @c KuduClient is created. Sometimes it makes sense to customize the
+ /// timeout for connection negotiation, e.g. when running on a cluster with
+ /// heavily loaded tablet servers. For details on the connection negotiation,
+ /// see ../../../docs/design-docs/rpc.md#negotiation.
+ ///
+ /// @param [in] timeout
+ /// Timeout value to set.
+ /// @return Reference to the updated object.
+ KuduClientBuilder& connection_negotiation_timeout(const MonoDelta& timeout);
+
+
/// Import serialized authentication credentials from another client.
///
/// @param [in] authn_creds
@@ -518,6 +534,9 @@ class KUDU_EXPORT KuduClient : public sp::enable_shared_from_this<KuduClient> {
/// @return Default timeout for RPCs.
const MonoDelta& default_rpc_timeout() const;
+ /// @return Timeout for connection negotiation to a remote server.
+ MonoDelta connection_negotiation_timeout() const;
+
/// Value for the latest observed timestamp when none has been observed
/// or set.
static const uint64_t kNoTimestamp;
@@ -637,6 +656,7 @@ class KUDU_EXPORT KuduClient : public sp::enable_shared_from_this<KuduClient> {
friend class tools::RemoteKsckCluster;
FRIEND_TEST(kudu::ClientStressTest, TestUniqueClientIds);
+ FRIEND_TEST(ClientTest, ConnectionNegotiationTimeout);
FRIEND_TEST(ClientTest, TestCacheAuthzTokens);
FRIEND_TEST(ClientTest, TestGetSecurityInfoFromMaster);
FRIEND_TEST(ClientTest, TestGetTabletServerBlacklist);
diff --git a/src/kudu/client/client_builder-internal.h b/src/kudu/client/client_builder-internal.h
index 329c41f..f30cc7b 100644
--- a/src/kudu/client/client_builder-internal.h
+++ b/src/kudu/client/client_builder-internal.h
@@ -14,8 +14,7 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_CLIENT_CLIENT_BUILDER_INTERNAL_H
-#define KUDU_CLIENT_CLIENT_BUILDER_INTERNAL_H
+#pragma once
#include <string>
#include <vector>
@@ -39,6 +38,7 @@ class KuduClientBuilder::Data {
std::vector<std::string> master_server_addrs_;
MonoDelta default_admin_operation_timeout_;
MonoDelta default_rpc_timeout_;
+ MonoDelta connection_negotiation_timeout_;
std::string authn_creds_;
internal::ReplicaController::Visibility replica_visibility_;
boost::optional<int> num_reactors_;
@@ -48,5 +48,3 @@ class KuduClientBuilder::Data {
} // namespace client
} // namespace kudu
-
-#endif
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index c000303..784188f 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -62,6 +62,8 @@ using strings::Substitute;
namespace kudu {
namespace rpc {
+const int64_t MessengerBuilder::kRpcNegotiationTimeoutMs = 3000;
+
MessengerBuilder::MessengerBuilder(std::string name)
: name_(std::move(name)),
connection_keepalive_time_(MonoDelta::FromMilliseconds(65000)),
@@ -69,7 +71,7 @@ MessengerBuilder::MessengerBuilder(std::string name)
min_negotiation_threads_(0),
max_negotiation_threads_(4),
coarse_timer_granularity_(MonoDelta::FromMilliseconds(100)),
- rpc_negotiation_timeout_ms_(3000),
+ rpc_negotiation_timeout_ms_(kRpcNegotiationTimeoutMs),
sasl_proto_name_("kudu"),
rpc_authentication_("optional"),
rpc_encryption_("optional"),
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index 2cb1ecd..10b94f8 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -82,6 +82,8 @@ class MessengerBuilder {
friend class Messenger;
friend class ReactorThread;
+ static const int64_t kRpcNegotiationTimeoutMs;
+
explicit MessengerBuilder(std::string name);
// Set the length of time we will keep a TCP connection will alive with no traffic.
@@ -308,7 +310,9 @@ class Messenger {
scoped_refptr<MetricEntity> metric_entity() const { return metric_entity_; }
- const int64_t rpc_negotiation_timeout_ms() const { return rpc_negotiation_timeout_ms_; }
+ int64_t rpc_negotiation_timeout_ms() const {
+ return rpc_negotiation_timeout_ms_;
+ }
const std::string& sasl_proto_name() const {
return sasl_proto_name_;