You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2016/12/09 19:57:51 UTC
kudu git commit: KUDU-1776: Fix "kudu remote_replica copy" connecting
to wildcard address
Repository: kudu
Updated Branches:
refs/heads/master 8195e5b37 -> 8a7871c47
KUDU-1776: Fix "kudu remote_replica copy" connecting to wildcard address
"remote_replica copy" asks the source server for its bound address,
but that address could be wildcard (0.0.0.0) in a real cluster,
and if same address is stuffed in RPC, it fails the validation
at the source tablet server:
tablet_id: "834e7673f32e4802bdf26f9fff9162ef"
copy_peer_uuid: "893679e1b1dd4342bf1fb8f058ec2789"
copy_peer_addr {
host: "0.0.0.0"
port: 7050
}
UNKNOWN_ERROR: Invalid wildcard address to tablet copy from: 0.0.0.0 (resolved to 0.0.0.0)
Fix: Tool remote_replica calls GetStatus RPC on a remote server which is
expected to return host/port information belonging to remote RPC server.
However, if the remote RPC was bound to wildcard ip address, the returned
host ip contains 0.0.0.0. This patch adds HostPortFromSockaddrReplaceWildcard
which helps to replace the wildcard with hostname of the remote server.
Testing: Patch makes few changes to external_mini_cluster such that it
supports binding to wildcard ip now. This is coupled with few consolidations
around bind_rpc_address_ fields which are shared between masters and
tablet servers, hence moved that up in the class hierarchy.
Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f
Reviewed-on: http://gerrit.cloudera.org:8080/5378
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/8a7871c4
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/8a7871c4
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/8a7871c4
Branch: refs/heads/master
Commit: 8a7871c47897d801d7be2697a28c2ff006a696ee
Parents: 8195e5b
Author: Dinesh Bhat <di...@cloudera.com>
Authored: Tue Dec 6 14:41:08 2016 +0530
Committer: Mike Percy <mp...@apache.org>
Committed: Fri Dec 9 19:57:27 2016 +0000
----------------------------------------------------------------------
.../integration-tests/external_mini_cluster.cc | 67 +++++++++++++-------
.../integration-tests/external_mini_cluster.h | 34 ++++++++--
src/kudu/integration-tests/log-rolling-itest.cc | 2 +-
src/kudu/server/server_base.cc | 12 ++--
src/kudu/tools/kudu-tool-test.cc | 15 +++--
5 files changed, 89 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a7871c4/src/kudu/integration-tests/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.cc b/src/kudu/integration-tests/external_mini_cluster.cc
index 7665ca7..f4be5ad 100644
--- a/src/kudu/integration-tests/external_mini_cluster.cc
+++ b/src/kudu/integration-tests/external_mini_cluster.cc
@@ -71,20 +71,24 @@ namespace kudu {
static const char* const kMasterBinaryName = "kudu-master";
static const char* const kTabletServerBinaryName = "kudu-tserver";
+static const char* const kWildcardIpAddr = "0.0.0.0";
+static const char* const kLoopbackIpAddr = "127.0.0.1";
static double kProcessStartTimeoutSeconds = 30.0;
static double kTabletServerRegistrationTimeoutSeconds = 15.0;
static double kMasterCatalogManagerTimeoutSeconds = 30.0;
#if defined(__APPLE__)
-static bool kBindToUniqueLoopbackAddress = false;
+static ExternalMiniClusterOptions::BindMode kBindMode =
+ ExternalMiniClusterOptions::BindMode::LOOPBACK;
#else
-static bool kBindToUniqueLoopbackAddress = true;
+static ExternalMiniClusterOptions::BindMode kBindMode =
+ ExternalMiniClusterOptions::BindMode::UNIQUE_LOOPBACK;
#endif
ExternalMiniClusterOptions::ExternalMiniClusterOptions()
: num_masters(1),
num_tablet_servers(1),
- bind_to_unique_loopback_addresses(kBindToUniqueLoopbackAddress),
+ bind_mode(kBindMode),
enable_kerberos(false),
logtostderr(true) {
}
@@ -257,7 +261,7 @@ Status ExternalMiniCluster::StartSingleMaster() {
GetLogPath(daemon_id),
SubstituteInFlags(opts_.extra_master_flags, 0));
if (opts_.enable_kerberos) {
- RETURN_NOT_OK_PREPEND(master->EnableKerberos(kdc_.get(), "127.0.0.1"),
+ RETURN_NOT_OK_PREPEND(master->EnableKerberos(kdc_.get(), Substitute("$0", kLoopbackIpAddr)),
"could not enable Kerberos");
}
@@ -276,7 +280,7 @@ Status ExternalMiniCluster::StartDistributedMasters() {
vector<string> peer_addrs;
for (int i = 0; i < num_masters; i++) {
- string addr = Substitute("127.0.0.1:$0", opts_.master_rpc_ports[i]);
+ string addr = Substitute("$0:$1", kLoopbackIpAddr, opts_.master_rpc_ports[i]);
peer_addrs.push_back(addr);
}
vector<string> flags = opts_.extra_master_flags;
@@ -294,7 +298,7 @@ Status ExternalMiniCluster::StartDistributedMasters() {
peer_addrs[i],
SubstituteInFlags(flags, i));
if (opts_.enable_kerberos) {
- RETURN_NOT_OK_PREPEND(peer->EnableKerberos(kdc_.get(), "127.0.0.1"),
+ RETURN_NOT_OK_PREPEND(peer->EnableKerberos(kdc_.get(), Substitute("$0", kLoopbackIpAddr)),
"could not enable Kerberos");
}
RETURN_NOT_OK_PREPEND(peer->Start(),
@@ -306,13 +310,17 @@ Status ExternalMiniCluster::StartDistributedMasters() {
}
string ExternalMiniCluster::GetBindIpForTabletServer(int index) const {
- if (opts_.bind_to_unique_loopback_addresses) {
+ string bind_ip;
+ if (opts_.bind_mode == ExternalMiniClusterOptions::UNIQUE_LOOPBACK) {
pid_t p = getpid();
CHECK_LE(p, MathLimits<uint16_t>::kMax) << "Cannot run on systems with >16-bit pid";
- return Substitute("127.$0.$1.$2", p >> 8, p & 0xff, index);
+ bind_ip = Substitute("127.$0.$1.$2", p >> 8, p & 0xff, index);
+ } else if (opts_.bind_mode == ExternalMiniClusterOptions::WILDCARD) {
+ bind_ip = Substitute("$0", kWildcardIpAddr);
} else {
- return "127.0.0.1";
+ bind_ip = Substitute("$0", kLoopbackIpAddr);
}
+ return bind_ip;
}
Status ExternalMiniCluster::AddTabletServer() {
@@ -789,9 +797,18 @@ void ExternalDaemon::Shutdown() {
if (!process_) return;
// Before we kill the process, store the addresses. If we're told to
- // start again we'll reuse these.
- bound_rpc_ = bound_rpc_hostport();
- bound_http_ = bound_http_hostport();
+ // start again we'll reuse these. Store only the port if the
+ // daemons were using wildcard address for binding.
+ const string& wildcard_ip = Substitute("$0", kWildcardIpAddr);
+ if (get_rpc_bind_address() != wildcard_ip) {
+ bound_rpc_ = bound_rpc_hostport();
+ bound_http_ = bound_http_hostport();
+ } else {
+ bound_rpc_.set_host(wildcard_ip);
+ bound_rpc_.set_port(bound_rpc_hostport().port());
+ bound_http_.set_host(wildcard_ip);
+ bound_http_.set_port(bound_http_hostport().port());
+ }
if (IsProcessAlive()) {
// In coverage builds, ask the process nicely to flush coverage info
@@ -946,8 +963,8 @@ ExternalMaster::ExternalMaster(std::shared_ptr<rpc::Messenger> messenger,
std::move(exe),
std::move(data_dir),
std::move(log_dir),
- std::move(extra_flags)),
- rpc_bind_address_("127.0.0.1:0") {
+ std::move(extra_flags)) {
+ set_rpc_bind_address(Substitute("$0:0", kLoopbackIpAddr));
}
ExternalMaster::ExternalMaster(std::shared_ptr<rpc::Messenger> messenger,
@@ -960,8 +977,9 @@ ExternalMaster::ExternalMaster(std::shared_ptr<rpc::Messenger> messenger,
std::move(exe),
std::move(data_dir),
std::move(log_dir),
- std::move(extra_flags)),
- rpc_bind_address_(std::move(rpc_bind_address)) {}
+ std::move(extra_flags)) {
+ set_rpc_bind_address(std::move(rpc_bind_address));
+}
ExternalMaster::~ExternalMaster() {
}
@@ -970,7 +988,7 @@ Status ExternalMaster::Start() {
vector<string> flags;
flags.push_back("--fs_wal_dir=" + data_dir_);
flags.push_back("--fs_data_dirs=" + data_dir_);
- flags.push_back("--rpc_bind_addresses=" + rpc_bind_address_);
+ flags.push_back("--rpc_bind_addresses=" + get_rpc_bind_address());
flags.push_back("--webserver_interface=localhost");
flags.push_back("--webserver_port=0");
RETURN_NOT_OK(StartProcess(flags));
@@ -1051,8 +1069,9 @@ ExternalTabletServer::ExternalTabletServer(std::shared_ptr<rpc::Messenger> messe
std::move(data_dir),
std::move(log_dir),
std::move(extra_flags)),
- master_addrs_(HostPort::ToCommaSeparatedString(master_addrs)),
- bind_host_(std::move(bind_host)) {}
+ master_addrs_(HostPort::ToCommaSeparatedString(master_addrs)) {
+ set_rpc_bind_address(std::move(bind_host));
+}
ExternalTabletServer::~ExternalTabletServer() {
}
@@ -1062,11 +1081,11 @@ Status ExternalTabletServer::Start() {
flags.push_back("--fs_wal_dir=" + data_dir_);
flags.push_back("--fs_data_dirs=" + data_dir_);
flags.push_back(Substitute("--rpc_bind_addresses=$0:0",
- bind_host_));
+ get_rpc_bind_address()));
flags.push_back(Substitute("--local_ip_for_outbound_sockets=$0",
- bind_host_));
+ get_rpc_bind_address()));
flags.push_back(Substitute("--webserver_interface=$0",
- bind_host_));
+ get_rpc_bind_address()));
flags.push_back("--webserver_port=0");
flags.push_back("--tserver_master_addrs=" + master_addrs_);
RETURN_NOT_OK(StartProcess(flags));
@@ -1083,10 +1102,10 @@ Status ExternalTabletServer::Restart() {
flags.push_back("--fs_data_dirs=" + data_dir_);
flags.push_back("--rpc_bind_addresses=" + bound_rpc_.ToString());
flags.push_back(Substitute("--local_ip_for_outbound_sockets=$0",
- bind_host_));
+ get_rpc_bind_address()));
flags.push_back(Substitute("--webserver_port=$0", bound_http_.port()));
flags.push_back(Substitute("--webserver_interface=$0",
- bind_host_));
+ bound_http_.host()));
flags.push_back("--tserver_master_addrs=" + master_addrs_);
RETURN_NOT_OK(StartProcess(flags));
return Status::OK();
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a7871c4/src/kudu/integration-tests/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.h b/src/kudu/integration-tests/external_mini_cluster.h
index f4049ce..48979f0 100644
--- a/src/kudu/integration-tests/external_mini_cluster.h
+++ b/src/kudu/integration-tests/external_mini_cluster.h
@@ -76,13 +76,22 @@ struct ExternalMiniClusterOptions {
// Default: "", which auto-generates a unique path for this cluster.
std::string data_root;
- // If true, binds each tablet server to a different loopback address.
+ // BindMode lets you specify the socket binding mode for RPC and/or HTTP server.
+ // A) LOOPBACK binds each server to loopback ip address "127.0.0.1".
+ //
+ // B) WILDCARD specifies "0.0.0.0" as the ip to bind to, which means sockets
+ // can be bound to any interface on the local host.
+ // For example, if a host has two interfaces with addresses
+ // 192.168.0.10 and 192.168.0.11, the server process can accept connection
+ // requests addressed to 192.168.0.10 or 192.168.0.11.
+ //
+ // C) UNIQUE_LOOPBACK binds each tablet server to a different loopback address.
// This affects the server's RPC server, and also forces the server to
// only use this IP address for outgoing socket connections as well.
// This allows the use of iptables on the localhost to simulate network
// partitions.
//
- // The addressed used are 127.<A>.<B>.<C> where:
+ // The addresses used are 127.<A>.<B>.<C> where:
// - <A,B> are the high and low bytes of the pid of the process running the
// minicluster (not the daemon itself).
// - <C> is the index of the server within this minicluster.
@@ -97,8 +106,13 @@ struct ExternalMiniClusterOptions {
//
// NOTE: this does not currently affect the HTTP server.
//
- // Default: true
- bool bind_to_unique_loopback_addresses;
+ // Default: UNIQUE_LOOPBACK on Linux, LOOPBACK on macOS.
+ enum BindMode {
+ UNIQUE_LOOPBACK,
+ WILDCARD,
+ LOOPBACK
+ };
+ BindMode bind_mode;
// The path where the kudu daemons should be run from.
// Default: "", which uses the same path as the currently running executable.
@@ -422,6 +436,14 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> {
// In a non-coverage build, this does nothing.
void FlushCoverage();
+ // Get/Set rpc_bind_addresses for daemon.
+ virtual const std::string& get_rpc_bind_address() const {
+ return rpc_bind_address_;
+ }
+ virtual void set_rpc_bind_address(std::string host) {
+ rpc_bind_address_ = host;
+ }
+
const std::shared_ptr<rpc::Messenger> messenger_;
const std::string data_dir_;
const boost::optional<std::string> log_dir_;
@@ -432,6 +454,7 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> {
gscoped_ptr<Subprocess> process_;
gscoped_ptr<server::ServerStatusPB> status_;
+ std::string rpc_bind_address_;
// These capture the daemons parameters and running ports and
// are used to Restart() the daemon with the same parameters.
@@ -487,8 +510,6 @@ class ExternalMaster : public ExternalDaemon {
private:
friend class RefCountedThreadSafe<ExternalMaster>;
virtual ~ExternalMaster();
-
- const std::string rpc_bind_address_;
};
class ExternalTabletServer : public ExternalDaemon {
@@ -509,7 +530,6 @@ class ExternalTabletServer : public ExternalDaemon {
private:
const std::string master_addrs_;
- const std::string bind_host_;
friend class RefCountedThreadSafe<ExternalTabletServer>;
virtual ~ExternalTabletServer();
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a7871c4/src/kudu/integration-tests/log-rolling-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/log-rolling-itest.cc b/src/kudu/integration-tests/log-rolling-itest.cc
index 899ca28..2b00014 100644
--- a/src/kudu/integration-tests/log-rolling-itest.cc
+++ b/src/kudu/integration-tests/log-rolling-itest.cc
@@ -49,7 +49,7 @@ TEST(LogRollingITest, TestLogCleanupOnStartup) {
opts.num_tablet_servers = 0;
opts.extra_master_flags = { "--max_log_files=3", };
opts.logtostderr = false;
- opts.bind_to_unique_loopback_addresses = true;
+ opts.bind_mode = ExternalMiniClusterOptions::UNIQUE_LOOPBACK;
ExternalMiniCluster cluster(opts);
ASSERT_OK(cluster.Start());
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a7871c4/src/kudu/server/server_base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index 1508c1f..2831855 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -24,6 +24,7 @@
#include <gflags/gflags.h>
#include "kudu/codegen/compilation_manager.h"
+#include "kudu/common/wire_protocol.h"
#include "kudu/common/wire_protocol.pb.h"
#include "kudu/fs/fs_manager.h"
#include "kudu/gutil/strings/strcat.h"
@@ -51,6 +52,7 @@
#include "kudu/util/mem_tracker.h"
#include "kudu/util/metrics.h"
#include "kudu/util/monotime.h"
+#include "kudu/util/net/net_util.h"
#include "kudu/util/net/sockaddr.h"
#include "kudu/util/pb_util.h"
#include "kudu/util/rolling_log.h"
@@ -213,9 +215,10 @@ void ServerBase::GetStatusPB(ServerStatusPB* status) const {
vector<Sockaddr> addrs;
CHECK_OK(rpc_server_->GetBoundAddresses(&addrs));
for (const Sockaddr& addr : addrs) {
+ HostPort hp;
+ CHECK_OK(HostPortFromSockaddrReplaceWildcard(addr, &hp));
HostPortPB* pb = status->add_bound_rpc_addresses();
- pb->set_host(addr.host());
- pb->set_port(addr.port());
+ CHECK_OK(HostPortToPB(hp, pb));
}
}
@@ -224,9 +227,10 @@ void ServerBase::GetStatusPB(ServerStatusPB* status) const {
vector<Sockaddr> addrs;
CHECK_OK(web_server_->GetBoundAddresses(&addrs));
for (const Sockaddr& addr : addrs) {
+ HostPort hp;
+ CHECK_OK(HostPortFromSockaddrReplaceWildcard(addr, &hp));
HostPortPB* pb = status->add_bound_http_addresses();
- pb->set_host(addr.host());
- pb->set_port(addr.port());
+ CHECK_OK(HostPortToPB(hp, pb));
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a7871c4/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index ea01c66..163ef63 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -229,17 +229,17 @@ class ToolTest : public KuduTest {
unique_ptr<ExternalMiniClusterFsInspector> inspect_;
unordered_map<string, TServerDetails*> ts_map_;
unique_ptr<MiniCluster> mini_cluster_;
+ ExternalMiniClusterOptions cluster_opts_;
string tool_path_;
};
void ToolTest::StartExternalMiniCluster(const vector<string>& extra_master_flags,
const vector<string>& extra_tserver_flags,
int num_tablet_servers) {
- ExternalMiniClusterOptions opts;
- opts.extra_master_flags = extra_master_flags;
- opts.extra_tserver_flags = extra_tserver_flags;
- opts.num_tablet_servers = num_tablet_servers;
- cluster_.reset(new ExternalMiniCluster(opts));
+ cluster_opts_.extra_master_flags = extra_master_flags;
+ cluster_opts_.extra_tserver_flags = extra_tserver_flags;
+ cluster_opts_.num_tablet_servers = num_tablet_servers;
+ cluster_.reset(new ExternalMiniCluster(cluster_opts_));
ASSERT_OK(cluster_->Start());
inspect_.reset(new ExternalMiniClusterFsInspector(cluster_.get()));
ASSERT_OK(CreateTabletServerMap(cluster_->master_proxy().get(),
@@ -961,6 +961,11 @@ TEST_F(ToolTest, TestRemoteReplicaCopy) {
const int kDstTsIndex = 1;
const int kNumTservers = 3;
const int kNumTablets = 3;
+ // This lets us specify wildcard ip addresses for rpc servers
+ // on the tablet servers in the cluster giving us the test coverage
+ // for KUDU-1776. With this, 'kudu remote_replica copy' can be used to
+ // connect to tablet servers bound to wildcard ip addresses.
+ cluster_opts_.bind_mode = ExternalMiniClusterOptions::WILDCARD;
NO_FATALS(StartExternalMiniCluster(
{"--catalog_manager_wait_for_new_tablets_to_elect_leader=false"},
{"--enable_leader_failure_detection=false"}, kNumTservers));