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 3932a7780 [rpc] add hostname in Messenger to use it in
ServerNegotiation
3932a7780 is described below
commit 3932a7780b8005b7204b3058302600ea67cd1f2b
Author: Alexey Serbin <[email protected]>
AuthorDate: Fri Oct 20 16:29:44 2023 -0700
[rpc] add hostname in Messenger to use it in ServerNegotiation
This patch introduces a new 'hostname' property for the Messenger class
to use it in ServerNegotiation. By doing so, a call to GetFQDN() is
avoided during RPC connection negotiation at the server side. A call
to GetFQDN() might be quite expensive since sometimes it turns into
a remote call, and if DNS resolver is slow or misconfigured, the
round-trip might take several seconds. By avoiding calls to GetFQDN()
as part of ServerNegotiation::InitSaslServer(), the server-side
negotiation is now more robust at least when using Kerberos credentials
for RPC authentication.
The hostname (usually FQDN) is retrieved as a part of KuduServer::Init()
and stored in the messenger. It's used later on to set the 'hostname'
attribute for the server metrics and also when running server-side
RPC connection negotiation. Of course, this is done under assumption
that the name of a node isn't changing under a running Kudu server.
This patch also contains a few test scenarios to cover the new
functionality. Existing {TabletServerTest,MasterTest}.ServerAttributes
test scenarios have been updated accordingly.
Change-Id: I1c13e0b4c4320b5b3eeaaf8e63df8b0d56f0ee6a
Reviewed-on: http://gerrit.cloudera.org:8080/20609
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <[email protected]>
---
src/kudu/master/master-test.cc | 16 +++++++++++-----
src/kudu/rpc/messenger.cc | 1 +
src/kudu/rpc/messenger.h | 20 +++++++++++++++++++-
src/kudu/rpc/negotiation.cc | 4 +++-
src/kudu/rpc/rpc-test-base.h | 4 ++++
src/kudu/server/server_base.cc | 22 +++++++++++++++++-----
src/kudu/tserver/tablet_server-test.cc | 16 +++++++++++-----
7 files changed, 66 insertions(+), 17 deletions(-)
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 85c111e01..1b080a0bc 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -2384,15 +2384,21 @@ TEST_F(MasterTest, ServerAttributes) {
EasyCurl c;
faststring buf;
ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics?ids=kudu.master";,
- mini_master_->bound_http_addr().ToString()),
- &buf));
- string raw = buf.ToString();
- string server_hostname;
+ mini_master_->bound_http_addr().ToString()),
+ &buf));
+ const auto& raw = buf.ToString();
ASSERT_STR_CONTAINS(raw, "\"uuid\": \"" + mini_master_->permanent_uuid() +
"\"");
- ASSERT_OK(GetFQDN(&server_hostname));
+ const auto& server_hostname =
mini_master_->master()->messenger()->hostname();
ASSERT_STR_CONTAINS(raw, "\"hostname\": \"" + server_hostname + "\"");
}
+// Test that hostname is set properly for Master's Messenger.
+TEST_F(MasterTest, ServerHostname) {
+ string server_hostname;
+ ASSERT_OK(GetFQDN(&server_hostname));
+ ASSERT_EQ(server_hostname, mini_master_->master()->messenger()->hostname());
+}
+
// Run multiple threads calling GetTableSchema() directly to system catalog.
TEST_P(ConcurrentGetTableSchemaTest, DirectMethodCall) {
SKIP_IF_SLOW_NOT_ALLOWED();
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 9c711a6a1..923d85f7f 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -326,6 +326,7 @@ Messenger::Messenger(const MessengerBuilder &bld)
rpcz_store_(new RpczStore),
metric_entity_(bld.metric_entity_),
rpc_negotiation_timeout_ms_(bld.rpc_negotiation_timeout_ms_),
+ hostname_(bld.hostname_),
sasl_proto_name_(bld.sasl_proto_name_),
keytab_file_(bld.keytab_file_),
reuseport_(bld.reuseport_),
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index c352c1e13..d8e37e99c 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -144,6 +144,12 @@ class MessengerBuilder {
return *this;
}
+ // Set the name of the node where the result messenger will be running.
+ MessengerBuilder& set_hostname(const std::string& hostname) {
+ hostname_ = hostname;
+ return *this;
+ }
+
// Set the SASL protocol name that is used for the SASL negotiation.
MessengerBuilder& set_sasl_proto_name(const std::string& sasl_proto_name) {
sasl_proto_name_ = sasl_proto_name;
@@ -270,6 +276,7 @@ class MessengerBuilder {
MonoDelta coarse_timer_granularity_;
scoped_refptr<MetricEntity> metric_entity_;
int64_t rpc_negotiation_timeout_ms_;
+ std::string hostname_;
std::string sasl_proto_name_;
std::string rpc_authentication_;
std::string rpc_encryption_;
@@ -434,6 +441,14 @@ class Messenger {
return rpc_negotiation_timeout_ms_;
}
+ // The name of the node where this Messenger is running. The best case is
+ // FQDN retrieved using getaddrinfo(), but it might be just local hostname
+ // retrived by gethostname(). It can also be empty if Messenger has been
+ // created without setting the hostname.
+ const std::string& hostname() const {
+ return hostname_;
+ }
+
const std::string& sasl_proto_name() const {
return sasl_proto_name_;
}
@@ -535,6 +550,9 @@ class Messenger {
// Timeout in milliseconds after which an incomplete connection negotiation
will timeout.
const int64_t rpc_negotiation_timeout_ms_;
+ // The name of the node where this messenger is running.
+ const std::string hostname_;
+
// The SASL protocol name that is used for the SASL negotiation.
const std::string sasl_proto_name_;
@@ -542,7 +560,7 @@ class Messenger {
const std::string keytab_file_;
// Whether to set SO_REUSEPORT on the listening sockets.
- bool reuseport_;
+ const bool reuseport_;
// The ownership of the Messenger object is somewhat subtle. The pointer
graph
// looks like this:
diff --git a/src/kudu/rpc/negotiation.cc b/src/kudu/rpc/negotiation.cc
index dd7f7005b..2c0775a5f 100644
--- a/src/kudu/rpc/negotiation.cc
+++ b/src/kudu/rpc/negotiation.cc
@@ -273,7 +273,9 @@ static Status DoServerNegotiation(Connection* conn,
encryption,
encrypt_loopback,
messenger->sasl_proto_name());
-
+ if (!messenger->hostname().empty()) {
+ server_negotiation.set_server_fqdn(messenger->hostname());
+ }
if (authentication != RpcAuthentication::DISABLED &&
!messenger->keytab_file().empty()) {
RETURN_NOT_OK(server_negotiation.EnableGSSAPI());
}
diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h
index 97eca8e64..9f8efa11a 100644
--- a/src/kudu/rpc/rpc-test-base.h
+++ b/src/kudu/rpc/rpc-test-base.h
@@ -45,6 +45,7 @@
#include "kudu/util/jwt.h"
#include "kudu/util/mem_tracker.h"
#include "kudu/util/monotime.h"
+#include "kudu/util/net/net_util.h"
#include "kudu/util/net/sockaddr.h"
#include "kudu/util/path_util.h"
#include "kudu/util/pb_util.h"
@@ -469,6 +470,9 @@ class RpcTestBase : public KuduTest {
}
bld.set_metric_entity(metric_entity_);
bld.set_rpc_negotiation_timeout_ms(rpc_negotiation_timeout_ms_);
+ std::string hostname;
+ RETURN_NOT_OK(GetFQDN(&hostname));
+ bld.set_hostname(hostname);
return bld.Build(messenger);
}
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index cb921daa6..0d9bd20fd 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -731,6 +731,19 @@ Status ServerBase::Init() {
InitSpinLockContentionProfiling();
+ // Get the FQDN of the node where the server is running. If fetching of the
+ // FQDN fails, it attempts to set the 'hostname_' field to the local
hostname.
+ string hostname;
+ if (auto s = GetFQDN(&hostname); !s.ok()) {
+ const auto& msg = Substitute("could not determine host FQDN: $0",
s.ToString());
+ if (hostname.empty()) {
+ LOG(ERROR) << msg;
+ return s;
+ }
+ LOG(WARNING) << msg;
+ }
+ DCHECK(!hostname.empty());
+
RETURN_NOT_OK(security::InitKerberosForServer(FLAGS_principal,
FLAGS_keytab_file));
RETURN_NOT_OK(file_cache_->Init());
@@ -816,6 +829,7 @@ Status ServerBase::Init() {
.set_epki_certificate_authority_file(FLAGS_rpc_ca_certificate_file)
.set_epki_private_password_key_cmd(FLAGS_rpc_private_key_password_cmd)
.set_keytab_file(FLAGS_keytab_file)
+ .set_hostname(hostname)
.enable_inbound_tls();
if (options_.rpc_opts.rpc_reuseport) {
@@ -1142,12 +1156,10 @@ std::string ServerBase::FooterHtml() const {
Status ServerBase::Start() {
GenerateInstanceID();
+ DCHECK(!fs_manager_->uuid().empty());
metric_entity_->SetAttribute("uuid", fs_manager_->uuid());
- // Get the FQDN. If that fails server_hostname will have either the local
hostname
- // (if GetHostname() succeeds) or still be empty (if GetHostname() fails)
- string server_hostname = "";
- WARN_NOT_OK(GetFQDN(&server_hostname), "could not determine host FQDN");
- metric_entity_->SetAttribute("hostname", server_hostname);
+ DCHECK(!messenger_->hostname().empty());
+ metric_entity_->SetAttribute("hostname", messenger_->hostname());
RETURN_NOT_OK(RegisterService(
unique_ptr<rpc::ServiceIf>(new GenericServiceImpl(this))));
diff --git a/src/kudu/tserver/tablet_server-test.cc
b/src/kudu/tserver/tablet_server-test.cc
index 2f386eff8..b1b9ac390 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -4155,15 +4155,21 @@ TEST_F(TabletServerTest, ServerAttributes) {
EasyCurl c;
faststring buf;
ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics?ids=kudu.tabletserver";,
- mini_server_->bound_http_addr().ToString()),
- &buf));
- string raw = buf.ToString();
- string server_hostname;
+ mini_server_->bound_http_addr().ToString()),
+ &buf));
+ const auto& raw = buf.ToString();
ASSERT_STR_CONTAINS(raw, "\"uuid\": \"" + mini_server_->uuid() + "\"");
- ASSERT_OK(GetFQDN(&server_hostname));
+ const auto& server_hostname =
mini_server_->server()->messenger()->hostname();
ASSERT_STR_CONTAINS(raw, "\"hostname\": \"" + server_hostname + "\"");
}
+// Test that hostname is set properly for TabletServer's Messenger.
+TEST_F(TabletServerTest, ServerHostname) {
+ string server_hostname;
+ ASSERT_OK(GetFQDN(&server_hostname));
+ ASSERT_EQ(server_hostname, mini_server_->server()->messenger()->hostname());
+}
+
TEST_F(TabletServerTest, TestInsertLatencyMicroBenchmark) {
METRIC_DEFINE_entity(test);
METRIC_DEFINE_histogram(test, insert_latency,
