Repository: incubator-impala Updated Branches: refs/heads/master e2532a96c -> 567814b4c
IMPALA-5499: avoid ephemeral port conflicts We should not select the same port twice, which could happen because multiple ports were selected consecutively without actually binding to any of the ports. Change-Id: Id02c83e6f946a14b83f5e6561957d7ad81442835 Reviewed-on: http://gerrit.cloudera.org:8080/7171 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/df2b5a93 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/df2b5a93 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/df2b5a93 Branch: refs/heads/master Commit: df2b5a938c64799a1264540c349e14593aec1544 Parents: e2532a9 Author: Tim Armstrong <[email protected]> Authored: Tue Jun 13 11:44:08 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Jun 15 04:26:48 2017 +0000 ---------------------------------------------------------------------- be/src/rpc/thrift-server-test.cc | 2 +- be/src/statestore/statestore-test.cc | 7 ++++--- be/src/testutil/in-process-servers.cc | 16 +++++++++------- be/src/util/network-util.cc | 12 +++++++++--- be/src/util/network-util.h | 7 ++++--- 5 files changed, 27 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/df2b5a93/be/src/rpc/thrift-server-test.cc ---------------------------------------------------------------------- diff --git a/be/src/rpc/thrift-server-test.cc b/be/src/rpc/thrift-server-test.cc index 7be286e..21e4e81 100644 --- a/be/src/rpc/thrift-server-test.cc +++ b/be/src/rpc/thrift-server-test.cc @@ -64,7 +64,7 @@ boost::shared_ptr<TProcessor> MakeProcessor() { } int GetServerPort() { - int port = FindUnusedEphemeralPort(); + int port = FindUnusedEphemeralPort(nullptr); EXPECT_FALSE(port == -1); return port; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/df2b5a93/be/src/statestore/statestore-test.cc ---------------------------------------------------------------------- diff --git a/be/src/statestore/statestore-test.cc b/be/src/statestore/statestore-test.cc index eb3a540..7f8467f 100644 --- a/be/src/statestore/statestore-test.cc +++ b/be/src/statestore/statestore-test.cc @@ -44,7 +44,7 @@ TEST(StatestoreTest, SmokeTest) { new InProcessStatestore(ips->port(), ips->port() + 10); ASSERT_FALSE(statestore_wont_start->Start().ok()); - int subscriber_port = FindUnusedEphemeralPort(); + int subscriber_port = FindUnusedEphemeralPort(nullptr); ASSERT_NE(subscriber_port, -1) << "Could not find unused port"; StatestoreSubscriber* sub_will_start = new StatestoreSubscriber("sub1", @@ -72,7 +72,8 @@ TEST(StatestoreSslTest, SmokeTest) { InProcessStatestore* statestore = InProcessStatestore::StartWithEphemeralPorts(); if (statestore == NULL) FAIL() << "Unable to start Statestore"; - int subscriber_port = FindUnusedEphemeralPort(); + vector<int> used_ports; + int subscriber_port = FindUnusedEphemeralPort(&used_ports); ASSERT_NE(subscriber_port, -1) << "Could not find unused port"; StatestoreSubscriber* sub_will_start = new StatestoreSubscriber("smoke_sub1", @@ -83,7 +84,7 @@ TEST(StatestoreSslTest, SmokeTest) { stringstream invalid_server_cert; invalid_server_cert << impala_home << "/be/src/testutil/invalid-server-cert.pem"; FLAGS_ssl_client_ca_certificate = invalid_server_cert.str(); - int another_subscriber_port = FindUnusedEphemeralPort(); + int another_subscriber_port = FindUnusedEphemeralPort(&used_ports); ASSERT_NE(another_subscriber_port, -1) << "Could not find unused port"; StatestoreSubscriber* sub_will_not_start = new StatestoreSubscriber("smoke_sub2", http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/df2b5a93/be/src/testutil/in-process-servers.cc ---------------------------------------------------------------------- diff --git a/be/src/testutil/in-process-servers.cc b/be/src/testutil/in-process-servers.cc index b28f7fc..365e610 100644 --- a/be/src/testutil/in-process-servers.cc +++ b/be/src/testutil/in-process-servers.cc @@ -42,22 +42,23 @@ using namespace impala; InProcessImpalaServer* InProcessImpalaServer::StartWithEphemeralPorts( const string& statestore_host, int statestore_port) { for (int tries = 0; tries < 10; ++tries) { - int backend_port = FindUnusedEphemeralPort(); + vector<int> used_ports; + int backend_port = FindUnusedEphemeralPort(&used_ports); if (backend_port == -1) continue; // This flag is read directly in several places to find the address of the local // backend interface. FLAGS_be_port = backend_port; - int subscriber_port = FindUnusedEphemeralPort(); + int subscriber_port = FindUnusedEphemeralPort(&used_ports); if (subscriber_port == -1) continue; - int webserver_port = FindUnusedEphemeralPort(); + int webserver_port = FindUnusedEphemeralPort(&used_ports); if (webserver_port == -1) continue; - int beeswax_port = FindUnusedEphemeralPort(); + int beeswax_port = FindUnusedEphemeralPort(&used_ports); if (beeswax_port == -1) continue; - int hs2_port = FindUnusedEphemeralPort(); + int hs2_port = FindUnusedEphemeralPort(&used_ports); if (hs2_port == -1) continue; InProcessImpalaServer* impala = @@ -134,10 +135,11 @@ Status InProcessImpalaServer::Join() { InProcessStatestore* InProcessStatestore::StartWithEphemeralPorts() { for (int tries = 0; tries < 10; ++tries) { - int statestore_port = FindUnusedEphemeralPort(); + vector<int> used_ports; + int statestore_port = FindUnusedEphemeralPort(&used_ports); if (statestore_port == -1) continue; - int webserver_port = FindUnusedEphemeralPort(); + int webserver_port = FindUnusedEphemeralPort(&used_ports); if (webserver_port == -1) continue; InProcessStatestore* ips = new InProcessStatestore(statestore_port, webserver_port); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/df2b5a93/be/src/util/network-util.cc ---------------------------------------------------------------------- diff --git a/be/src/util/network-util.cc b/be/src/util/network-util.cc index 4382e99..6228a0b 100644 --- a/be/src/util/network-util.cc +++ b/be/src/util/network-util.cc @@ -22,6 +22,7 @@ #include <netdb.h> #include <arpa/inet.h> #include <limits.h> +#include <algorithm> #include <sstream> #include <random> #include <vector> @@ -35,6 +36,7 @@ using boost::algorithm::is_any_of; using boost::algorithm::split; +using std::find; using std::random_device; #ifdef __APPLE__ @@ -176,7 +178,7 @@ ostream& operator<<(ostream& out, const TNetworkAddress& hostport) { /// Pick a random port in the range of ephemeral ports /// https://tools.ietf.org/html/rfc6335 -int FindUnusedEphemeralPort() { +int FindUnusedEphemeralPort(vector<int>* used_ports) { static uint32_t LOWER = 49152, UPPER = 65000; random_device rd; srand(rd()); @@ -187,17 +189,21 @@ int FindUnusedEphemeralPort() { bzero(reinterpret_cast<char*>(&server_address), sizeof(server_address)); server_address.sin_family = AF_INET; server_address.sin_addr.s_addr = INADDR_ANY; - for (uint32_t tries = 0; tries < 10; ++tries) { + for (int tries = 0; tries < 100; ++tries) { int port = LOWER + rand() % (UPPER - LOWER); + if (used_ports != nullptr + && find(used_ports->begin(), used_ports->end(), port) != used_ports->end()) { + continue; + } server_address.sin_port = htons(port); if (bind(sockfd, reinterpret_cast<struct sockaddr*>(&server_address), sizeof(server_address)) == 0) { close(sockfd); + if (used_ports != nullptr) used_ports->push_back(port); return port; } } close(sockfd); return -1; } - } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/df2b5a93/be/src/util/network-util.h ---------------------------------------------------------------------- diff --git a/be/src/util/network-util.h b/be/src/util/network-util.h index 315d451..2615184 100644 --- a/be/src/util/network-util.h +++ b/be/src/util/network-util.h @@ -64,7 +64,8 @@ std::string TNetworkAddressToString(const TNetworkAddress& address); /// Prints a hostport as ipaddress:port std::ostream& operator<<(std::ostream& out, const TNetworkAddress& hostport); -/// Returns a ephemeral port that is unused when this function executes. Returns -1 on an -/// error or if a free ephemeral port can't be found after 10 tries. -int FindUnusedEphemeralPort(); +/// Returns a ephemeral port that is currently unused. Returns -1 on an error or if +/// a free ephemeral port can't be found after 100 tries. If 'used_ports' is non-NULL, +/// does not select those ports and adds the selected port to 'used_ports'. +int FindUnusedEphemeralPort(std::vector<int>* used_ports); }
