Move Hostname to its own files. (#6759)

* Change DNS cache to use std::map.

Revert commit 90c259d84e95dd35e01149c0a86bd18e82e33930, because if we use unordered_map, toString() can be inconsistent.

* Move ClientKnob::COORDINATOR_HOSTNAME_RESOLVE_DELAY to FlowKnob::HOSTNAME_RESOLVE_DELAY.

* Move Hostname to its own files.

Also, add resolve-related variables and functions in Hostname.

fdbclient/ClientKnobs.cpp

@@ -50,7 +50,6 @@ void ClientKnobs::initialize(Randomize randomize) {
 	init( MAX_GENERATIONS_OVERRIDE,                  0 );
 	init( MAX_GENERATIONS_SIM,                      50 ); //Disable network connections after this many generations in simulation, should be less than RECOVERY_DELAY_START_GENERATION
 
-	init( COORDINATOR_HOSTNAME_RESOLVE_DELAY,     0.05 );
 	init( COORDINATOR_RECONNECTION_DELAY,          1.0 );
 	init( CLIENT_EXAMPLE_AMOUNT,                    20 );
 	init( MAX_CLIENT_STATUS_AGE,                   1.0 );

fdbclient/ClientKnobs.h

@@ -49,7 +49,6 @@ public:
 	double MAX_GENERATIONS_OVERRIDE;
 	double MAX_GENERATIONS_SIM;
 
-	double COORDINATOR_HOSTNAME_RESOLVE_DELAY;
 	double COORDINATOR_RECONNECTION_DELAY;
 	int CLIENT_EXAMPLE_AMOUNT;
 	double MAX_CLIENT_STATUS_AGE;

fdbclient/CoordinationInterface.h

@@ -28,6 +28,7 @@
 #include "fdbclient/CommitProxyInterface.h"
 #include "fdbclient/ClusterInterface.h"
 #include "fdbclient/WellKnownEndpoints.h"
+#include "flow/Hostname.h"
 
 const int MAX_CLUSTER_FILE_BYTES = 60000;
 

fdbclient/MonitorLeader.actor.cpp

@@ -559,7 +559,7 @@ ACTOR Future<Void> monitorNominee(Key key,
 				    .detail("OldAddr", coord.getLeader.getEndpoint().getPrimaryAddress().toString());
 				if (rep.getError().code() == error_code_request_maybe_delivered) {
 					// Delay to prevent tight resolving loop due to outdated DNS cache
-					wait(delay(CLIENT_KNOBS->COORDINATOR_HOSTNAME_RESOLVE_DELAY));
+					wait(delay(FLOW_KNOBS->HOSTNAME_RESOLVE_DELAY));
 					throw coordinators_changed();
 				} else {
 					throw rep.getError();

fdbclient/S3BlobStore.actor.cpp

@@ -32,6 +32,7 @@
 #include <boost/algorithm/string/classification.hpp>
 #include <boost/algorithm/string.hpp>
 #include "fdbrpc/IAsyncFile.h"
+#include "flow/Hostname.h"
 #include "flow/UnitTest.h"
 #include "fdbclient/rapidxml/rapidxml.hpp"
 #include "fdbclient/FDBAWSCredentialsProvider.h"

fdbserver/LeaderElection.actor.cpp

@@ -51,7 +51,7 @@ ACTOR Future<Void> submitCandidacy(Key key,
 				    .detail("OldAddr", coord.candidacy.getEndpoint().getPrimaryAddress().toString());
 				if (rep.getError().code() == error_code_request_maybe_delivered) {
 					// Delay to prevent tight resolving loop due to outdated DNS cache
-					wait(delay(CLIENT_KNOBS->COORDINATOR_HOSTNAME_RESOLVE_DELAY));
+					wait(delay(FLOW_KNOBS->HOSTNAME_RESOLVE_DELAY));
 					throw coordinators_changed();
 				} else {
 					throw rep.getError();

flow/CMakeLists.txt

@@ -30,6 +30,8 @@ set(FLOW_SRCS
   Hash3.h
   Histogram.cpp
   Histogram.h
+  Hostname.actor.cpp
+  Hostname.h
   IDispatched.h
   IRandom.h
   IThreadPoolTest.actor.cpp

flow/Hostname.actor.cpp

@@ -0,0 +1,194 @@
+/*
+ * Hostname.actor.cpp
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "flow/Hostname.h"
+#include "flow/UnitTest.h"
+#include "flow/actorcompiler.h" // has to be last include
+
+Hostname Hostname::parse(const std::string& s) {
+	if (s.empty() || !Hostname::isHostname(s)) {
+		throw connection_string_invalid();
+	}
+
+	bool isTLS = false;
+	std::string f;
+	if (s.size() > 4 && strcmp(s.c_str() + s.size() - 4, ":tls") == 0) {
+		isTLS = true;
+		f = s.substr(0, s.size() - 4);
+	} else {
+		f = s;
+	}
+	auto colonPos = f.find_first_of(":");
+	return Hostname(f.substr(0, colonPos), f.substr(colonPos + 1), isTLS);
+}
+
+void Hostname::resetToUnresolved() {
+	if (status == Hostname::RESOLVED) {
+		status = UNRESOLVED;
+		resolvedAddress = Optional<NetworkAddress>();
+	}
+}
+
+ACTOR Future<Void> resolveImpl(Hostname* self) {
+	loop {
+		if (self->status == Hostname::UNRESOLVED) {
+			self->status = Hostname::RESOLVING;
+			try {
+				std::vector<NetworkAddress> addresses =
+				    wait(INetworkConnections::net()->resolveTCPEndpointWithDNSCache(self->host, self->service));
+				NetworkAddress address = addresses[deterministicRandom()->randomInt(0, addresses.size())];
+				address.flags = 0; // Reset the parsed address to public
+				address.fromHostname = NetworkAddressFromHostname::True;
+				if (self->isTLS) {
+					address.flags |= NetworkAddress::FLAG_TLS;
+				}
+				self->resolvedAddress = address;
+				self->status = Hostname::RESOLVED;
+				break;
+			} catch (...) {
+				self->status = Hostname::UNRESOLVED;
+				self->resolveFinish.trigger();
+				self->resolvedAddress = Optional<NetworkAddress>();
+				wait(delay(FLOW_KNOBS->HOSTNAME_RESOLVE_DELAY));
+			}
+		} else if (self->status == Hostname::RESOLVING) {
+			wait(self->resolveFinish.onTrigger());
+			if (self->status == Hostname::RESOLVED) {
+				break;
+			}
+			// Otherwise, this means other threads failed on resolve, so here we go back to the loop and try to resolve
+			// again.
+		} else {
+			// status is RESOLVED, nothing to do.
+			break;
+		}
+	}
+	return Void();
+}
+
+Future<Void> Hostname::resolve() {
+	return resolveImpl(this);
+}
+
+void Hostname::resolveBlocking() {
+	if (status != RESOLVED) {
+		try {
+			std::vector<NetworkAddress> addresses =
+			    INetworkConnections::net()->resolveTCPEndpointBlockingWithDNSCache(host, service);
+			NetworkAddress address = addresses[deterministicRandom()->randomInt(0, addresses.size())];
+			address.flags = 0; // Reset the parsed address to public
+			address.fromHostname = NetworkAddressFromHostname::True;
+			if (isTLS) {
+				address.flags |= NetworkAddress::FLAG_TLS;
+			}
+			resolvedAddress = address;
+			status = RESOLVED;
+		} catch (...) {
+			status = UNRESOLVED;
+			resolvedAddress = Optional<NetworkAddress>();
+			throw lookup_failed();
+		}
+	}
+}
+
+TEST_CASE("/flow/Hostname/hostname") {
+	std::string hn1s = "localhost:1234";
+	std::string hn2s = "host-name:1234";
+	std::string hn3s = "host.name:1234";
+	std::string hn4s = "host-name_part1.host-name_part2:1234:tls";
+
+	std::string hn5s = "127.0.0.1:1234";
+	std::string hn6s = "127.0.0.1:1234:tls";
+	std::string hn7s = "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:4800";
+	std::string hn8s = "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:4800:tls";
+	std::string hn9s = "2001:0db8:85a3:0000:0000:8a2e:0370:7334";
+	std::string hn10s = "2001:0db8:85a3:0000:0000:8a2e:0370:7334:tls";
+	std::string hn11s = "[::1]:4800";
+	std::string hn12s = "[::1]:4800:tls";
+	std::string hn13s = "1234";
+
+	auto hn1 = Hostname::parse(hn1s);
+	ASSERT(hn1.toString() == hn1s);
+	ASSERT(hn1.host == "localhost");
+	ASSERT(hn1.service == "1234");
+	ASSERT(!hn1.isTLS);
+
+	state Hostname hn2 = Hostname::parse(hn2s);
+	ASSERT(hn2.toString() == hn2s);
+	ASSERT(hn2.host == "host-name");
+	ASSERT(hn2.service == "1234");
+	ASSERT(!hn2.isTLS);
+
+	auto hn3 = Hostname::parse(hn3s);
+	ASSERT(hn3.toString() == hn3s);
+	ASSERT(hn3.host == "host.name");
+	ASSERT(hn3.service == "1234");
+	ASSERT(!hn3.isTLS);
+
+	auto hn4 = Hostname::parse(hn4s);
+	ASSERT(hn4.toString() == hn4s);
+	ASSERT(hn4.host == "host-name_part1.host-name_part2");
+	ASSERT(hn4.service == "1234");
+	ASSERT(hn4.isTLS);
+
+	ASSERT(!Hostname::isHostname(hn5s));
+	ASSERT(!Hostname::isHostname(hn6s));
+	ASSERT(!Hostname::isHostname(hn7s));
+	ASSERT(!Hostname::isHostname(hn8s));
+	ASSERT(!Hostname::isHostname(hn9s));
+	ASSERT(!Hostname::isHostname(hn10s));
+	ASSERT(!Hostname::isHostname(hn11s));
+	ASSERT(!Hostname::isHostname(hn12s));
+	ASSERT(!Hostname::isHostname(hn13s));
+
+	ASSERT(hn1.status == Hostname::UNRESOLVED && !hn1.resolvedAddress.present());
+	ASSERT(hn2.status == Hostname::UNRESOLVED && !hn2.resolvedAddress.present());
+	ASSERT(hn3.status == Hostname::UNRESOLVED && !hn3.resolvedAddress.present());
+	ASSERT(hn4.status == Hostname::UNRESOLVED && !hn4.resolvedAddress.present());
+
+	try {
+		wait(timeoutError(hn2.resolve(), 1));
+	} catch (Error& e) {
+		ASSERT(e.code() == error_code_timed_out);
+	}
+	ASSERT(hn2.status == Hostname::UNRESOLVED && !hn2.resolvedAddress.present());
+
+	try {
+		hn2.resolveBlocking();
+	} catch (Error& e) {
+		ASSERT(e.code() == error_code_lookup_failed);
+	}
+	ASSERT(hn2.status == Hostname::UNRESOLVED && !hn2.resolvedAddress.present());
+
+	state NetworkAddress address = NetworkAddress::parse("127.0.0.0:1234");
+	INetworkConnections::net()->addMockTCPEndpoint("host-name", "1234", { address });
+	wait(hn2.resolve());
+	ASSERT(hn2.status == Hostname::RESOLVED);
+	ASSERT(hn2.resolvedAddress.present() && hn2.resolvedAddress.get() == address);
+
+	hn2.resetToUnresolved();
+	ASSERT(hn2.status == Hostname::UNRESOLVED && !hn2.resolvedAddress.present());
+
+	hn2.resolveBlocking();
+	ASSERT(hn2.status == Hostname::RESOLVED);
+	ASSERT(hn2.resolvedAddress.present() && hn2.resolvedAddress.get() == address);
+
+	return Void();
+}

flow/Hostname.h

@@ -0,0 +1,85 @@
+/*
+ * Hostname.h
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2013-2022 Apple Inc. and the FoundationDB project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef FLOW_HOSTNAME_H
+#define FLOW_HOSTNAME_H
+#pragma once
+
+#include "flow/network.h"
+#include "flow/genericactors.actor.h"
+
+struct Hostname {
+	std::string host;
+	std::string service; // decimal port number
+	bool isTLS;
+
+	Hostname(const std::string& host, const std::string& service, bool isTLS)
+	  : host(host), service(service), isTLS(isTLS) {}
+	Hostname() : host(""), service(""), isTLS(false) {}
+	Hostname(const Hostname& rhs) { operator=(rhs); }
+	Hostname& operator=(const Hostname& rhs) {
+		// Copy everything except AsyncTrigger resolveFinish.
+		host = rhs.host;
+		service = rhs.service;
+		isTLS = rhs.isTLS;
+		resolvedAddress = rhs.resolvedAddress;
+		status = rhs.status;
+		return *this;
+	}
+
+	bool operator==(const Hostname& r) const { return host == r.host && service == r.service && isTLS == r.isTLS; }
+	bool operator!=(const Hostname& r) const { return !(*this == r); }
+	bool operator<(const Hostname& r) const {
+		if (isTLS != r.isTLS)
+			return isTLS < r.isTLS;
+		else if (host != r.host)
+			return host < r.host;
+		return service < r.service;
+	}
+	bool operator>(const Hostname& r) const { return r < *this; }
+	bool operator<=(const Hostname& r) const { return !(*this > r); }
+	bool operator>=(const Hostname& r) const { return !(*this < r); }
+
+	// Allow hostnames in forms like following:
+	//    hostname:1234
+	//    host.name:1234
+	//    host-name:1234
+	//    host-name_part1.host-name_part2:1234:tls
+	static bool isHostname(const std::string& s) {
+		std::regex validation("^([\\w\\-]+\\.?)+:([\\d]+){1,}(:tls)?$");
+		std::regex ipv4Validation("^([\\d]{1,3}\\.?){4,}:([\\d]+){1,}(:tls)?$");
+		return !std::regex_match(s, ipv4Validation) && std::regex_match(s, validation);
+	}
+
+	static Hostname parse(const std::string& s);
+
+	std::string toString() const { return host + ":" + service + (isTLS ? ":tls" : ""); }
+
+	Optional<NetworkAddress> resolvedAddress;
+	enum HostnameStatus { UNRESOLVED, RESOLVING, RESOLVED };
+	Future<Void> resolve();
+	void resolveBlocking(); // This one should only be used when resolving asynchronously is impossible.
+	                        // For all other cases, resolve() should be preferred.
+	void resetToUnresolved();
+	HostnameStatus status = UNRESOLVED;
+	AsyncTrigger resolveFinish;
+};
+
+#endif

flow/Knobs.cpp

@@ -40,6 +40,7 @@ FlowKnobs const* FLOW_KNOBS = &bootstrapGlobalFlowKnobs;
 void FlowKnobs::initialize(Randomize randomize, IsSimulated isSimulated) {
 	init( AUTOMATIC_TRACE_DUMP,                                  1 );
 	init( PREVENT_FAST_SPIN_DELAY,                             .01 );
+	init( HOSTNAME_RESOLVE_DELAY,                              .05 );
 	init( CACHE_REFRESH_INTERVAL_WHEN_ALL_ALTERNATIVES_FAILED, 1.0 );
 
 	init( DELAY_JITTER_OFFSET,                                 0.9 );

flow/Knobs.h

@@ -113,6 +113,7 @@ class FlowKnobs : public KnobsImpl<FlowKnobs> {
 public:
 	int AUTOMATIC_TRACE_DUMP;
 	double PREVENT_FAST_SPIN_DELAY;
+	double HOSTNAME_RESOLVE_DELAY;
 	double CACHE_REFRESH_INTERVAL_WHEN_ALL_ALTERNATIVES_FAILED;
 
 	double DELAY_JITTER_OFFSET;

flow/network.cpp

@@ -63,23 +63,6 @@ bool IPAddress::isValid() const {
 	return std::get<uint32_t>(addr) != 0;
 }
 
-Hostname Hostname::parse(const std::string& s) {
-	if (s.empty()) {
-		throw connection_string_invalid();
-	}
-
-	bool isTLS = false;
-	std::string f;
-	if (s.size() > 4 && strcmp(s.c_str() + s.size() - 4, ":tls") == 0) {
-		isTLS = true;
-		f = s.substr(0, s.size() - 4);
-	} else {
-		f = s;
-	}
-	auto colonPos = f.find_first_of(":");
-	return Hostname(f.substr(0, colonPos), f.substr(colonPos + 1), isTLS);
-}
-
 FDB_DEFINE_BOOLEAN_PARAM(NetworkAddressFromHostname);
 
 NetworkAddress NetworkAddress::parse(std::string const& s) {
@@ -220,7 +203,7 @@ std::string DNSCache::toString() {
 }
 
 DNSCache DNSCache::parseFromString(const std::string& s) {
-	std::unordered_map<std::string, std::vector<NetworkAddress>> dnsCache;
+	std::map<std::string, std::vector<NetworkAddress>> dnsCache;
 
 	for (int p = 0; p < s.length();) {
 		int pSemiColumn = s.find_first_of(';', p);
@@ -364,62 +347,4 @@ TEST_CASE("/flow/network/ipaddress") {
 	return Void();
 }
 
-TEST_CASE("/flow/network/hostname") {
-	std::string hn1s = "localhost:1234";
-	std::string hn2s = "host-name:1234";
-	std::string hn3s = "host.name:1234";
-	std::string hn4s = "host-name_part1.host-name_part2:1234:tls";
-
-	std::string hn5s = "127.0.0.1:1234";
-	std::string hn6s = "127.0.0.1:1234:tls";
-	std::string hn7s = "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:4800";
-	std::string hn8s = "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:4800:tls";
-	std::string hn9s = "2001:0db8:85a3:0000:0000:8a2e:0370:7334";
-	std::string hn10s = "2001:0db8:85a3:0000:0000:8a2e:0370:7334:tls";
-	std::string hn11s = "[::1]:4800";
-	std::string hn12s = "[::1]:4800:tls";
-	std::string hn13s = "1234";
-
-	auto hn1 = Hostname::parse(hn1s);
-	ASSERT(hn1.toString() == hn1s);
-	ASSERT(hn1.host == "localhost");
-	ASSERT(hn1.service == "1234");
-	ASSERT(!hn1.isTLS);
-
-	auto hn2 = Hostname::parse(hn2s);
-	ASSERT(hn2.toString() == hn2s);
-	ASSERT(hn2.host == "host-name");
-	ASSERT(hn2.service == "1234");
-	ASSERT(!hn2.isTLS);
-
-	auto hn3 = Hostname::parse(hn3s);
-	ASSERT(hn3.toString() == hn3s);
-	ASSERT(hn3.host == "host.name");
-	ASSERT(hn3.service == "1234");
-	ASSERT(!hn3.isTLS);
-
-	auto hn4 = Hostname::parse(hn4s);
-	ASSERT(hn4.toString() == hn4s);
-	ASSERT(hn4.host == "host-name_part1.host-name_part2");
-	ASSERT(hn4.service == "1234");
-	ASSERT(hn4.isTLS);
-
-	ASSERT(Hostname::isHostname(hn1s));
-	ASSERT(Hostname::isHostname(hn2s));
-	ASSERT(Hostname::isHostname(hn3s));
-	ASSERT(Hostname::isHostname(hn4s));
-
-	ASSERT(!Hostname::isHostname(hn5s));
-	ASSERT(!Hostname::isHostname(hn6s));
-	ASSERT(!Hostname::isHostname(hn7s));
-	ASSERT(!Hostname::isHostname(hn8s));
-	ASSERT(!Hostname::isHostname(hn9s));
-	ASSERT(!Hostname::isHostname(hn10s));
-	ASSERT(!Hostname::isHostname(hn11s));
-	ASSERT(!Hostname::isHostname(hn12s));
-	ASSERT(!Hostname::isHostname(hn13s));
-
-	return Void();
-}
-
 NetworkInfo::NetworkInfo() : handshakeLock(new FlowLock(FLOW_KNOBS->TLS_HANDSHAKE_LIMIT)) {}

flow/network.h

@@ -135,43 +135,6 @@ inline TaskPriority incrementPriorityIfEven(TaskPriority p) {
 
 class Void;
 
-struct Hostname {
-	std::string host;
-	std::string service; // decimal port number
-	bool isTLS;
-
-	Hostname(std::string host, std::string service, bool isTLS) : host(host), service(service), isTLS(isTLS) {}
-	Hostname() : host(""), service(""), isTLS(false) {}
-
-	bool operator==(const Hostname& r) const { return host == r.host && service == r.service && isTLS == r.isTLS; }
-	bool operator!=(const Hostname& r) const { return !(*this == r); }
-	bool operator<(const Hostname& r) const {
-		if (isTLS != r.isTLS)
-			return isTLS < r.isTLS;
-		else if (host != r.host)
-			return host < r.host;
-		return service < r.service;
-	}
-	bool operator>(const Hostname& r) const { return r < *this; }
-	bool operator<=(const Hostname& r) const { return !(*this > r); }
-	bool operator>=(const Hostname& r) const { return !(*this < r); }
-
-	// Allow hostnames in forms like following:
-	//    hostname:1234
-	//    host.name:1234
-	//    host-name:1234
-	//    host-name_part1.host-name_part2:1234:tls
-	static bool isHostname(const std::string& s) {
-		std::regex validation("^([\\w\\-]+\\.?)+:([\\d]+){1,}(:tls)?$");
-		std::regex ipv4Validation("^([\\d]{1,3}\\.?){4,}:([\\d]+){1,}(:tls)?$");
-		return !std::regex_match(s, ipv4Validation) && std::regex_match(s, validation);
-	}
-
-	static Hostname parse(const std::string& s);
-
-	std::string toString() const { return host + ":" + service + (isTLS ? ":tls" : ""); }
-};
-
 struct IPAddress {
 	typedef boost::asio::ip::address_v6::bytes_type IPAddressStore;
 	static_assert(std::is_same<IPAddressStore, std::array<uint8_t, 16>>::value,
@@ -701,7 +664,7 @@ public:
 class DNSCache {
 public:
 	DNSCache() = default;
-	explicit DNSCache(const std::unordered_map<std::string, std::vector<NetworkAddress>>& dnsCache)
+	explicit DNSCache(const std::map<std::string, std::vector<NetworkAddress>>& dnsCache)
 	  : hostnameToAddresses(dnsCache) {}
 
 	Optional<std::vector<NetworkAddress>> find(const std::string& host, const std::string& service);
@@ -715,7 +678,7 @@ public:
 	static DNSCache parseFromString(const std::string& s);
 
 private:
-	std::unordered_map<std::string, std::vector<NetworkAddress>> hostnameToAddresses;
+	std::map<std::string, std::vector<NetworkAddress>> hostnameToAddresses;
 };
 
 class INetworkConnections {