Undo some changes that aren't needed

bindings/c/fdb_c.cpp

@@ -24,7 +24,6 @@
 #define FDB_API_VERSION 720
 #define FDB_INCLUDE_LEGACY_TYPES
 
-#include "fdbclient/CoordinationInterface.h"
 #include "fdbclient/MultiVersionTransaction.h"
 #include "fdbclient/MultiVersionAssignmentVars.h"
 #include "foundationdb/fdb_c.h"
@@ -361,7 +360,8 @@ void fdb_cluster_destroy_v609(FDBCluster* c) {
 // If it does and this is an external client loaded though the multi-version API, then it may inadvertently call
 // the version of the function in the primary library if it was loaded into the global symbols.
 fdb_error_t fdb_create_database_impl(const char* cluster_file_path, FDBDatabase** out_database) {
-	CATCH_AND_RETURN(*out_database = (FDBDatabase*)API->createDatabase(cluster_file_path).extractPtr(););
+	CATCH_AND_RETURN(*out_database =
+	                     (FDBDatabase*)API->createDatabase(cluster_file_path ? cluster_file_path : "").extractPtr(););
 }
 
 FDBFuture* fdb_cluster_create_database_v609(FDBCluster* c, uint8_t const* db_name, int db_name_length) {

documentation/sphinx/source/trace-logging.rst

@@ -1,77 +0,0 @@
-#############
-Trace Logging
-#############
-
-Overview
-========
-
-FoundationDB processes generate log files known as trace logs that include details about the sequence of events that a process performed and record a variety of metrics about that process. These log files are useful for observing the state of a cluster over time and for debugging issues that occur in the cluster, a client, or various tools.
-
-Each file contains a sequence of events for a single process, ordered by time. Clients that use the multi-version or multi-threaded client features will generate :ref:`multiple log files simultaneously <mvc-logging>`. Each entry in the log file will be either an XML event or a JSON object that containts several mandatory fields and zero or more arbitrary fields. For example, the following is an event generated in the XML format::
-
-<Event Severity="10" Time="1579736072.656689" Type="Net2Starting" ID="0000000000000000" Machine="1.1.1.1:4000" LogGroup="default"/>
-
-Most FoundationDB processes generate trace logs in the format described in this document. This includes server and client processes, fdbcli, and backup and DR tooling.
-
-Configuration
-=============
-
-File Format
------------
-
-FoundationDB trace logging
-
-* Parameters
-* Format
-
-Trace Files
-===========
-
-* Filenames
-* Rolling
-
-Mandatory Fields
-================
-
-* Severity
-* Time
-* Type
-* Machine
-* LogGroup
-* ID
-
-Common Fields
-=============
-
-* Roles
-
-Event Suppression
------------------
-
-* SuppressedEventCount
-
-Errors
-------
-
-* Error
-* ErrorDescription
-* ErrorCode
-
-Clients
--------
-
-*
-
-Rolled Events
-=============
-
-Counters
-========
-
-PeriodicEvents
-==============
-
-.. mvc-logging::
-
-Multi-Version and Multi-Threaded Client Logging
-===============================================

fdbcli/fdbcli.actor.cpp

@@ -1095,7 +1095,7 @@ ACTOR Future<int> cli(CLIOptions opt, LineNoise* plinenoise) {
 		if (!opt.exec.present()) {
 			printf("Using cluster file `%s'.\n", ccf->getLocation().c_str());
 		}
-		db = API->createDatabase(resolvedClusterFile.first.c_str());
+		db = API->createDatabase(opt.clusterFile.c_str());
 	} catch (Error& e) {
 		fprintf(stderr, "ERROR: %s (%d)\n", e.what(), e.code());
 		printf("Unable to connect to cluster from `%s'\n", ccf->getLocation().c_str());

fdbclient/CMakeLists.txt

@@ -23,13 +23,12 @@ vexillographer_compile(TARGET fdboptions_c LANG c OUT ${CMAKE_CURRENT_BINARY_DIR
 add_custom_target(fdboptions DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/include/fdbclient/FDBOptions.g.h)
 add_dependencies(fdboptions fdboptions_c)
 
-# ###############################################################################
+################################################################################
 # Build information
-# ###############################################################################
+################################################################################
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/BuildFlags.h.in ${CMAKE_CURRENT_BINARY_DIR}/include/fdbclient/BuildFlags.h)
 
 set(BUILD_AZURE_BACKUP OFF CACHE BOOL "Build Azure backup client")
-
 if(BUILD_AZURE_BACKUP)
   add_compile_definitions(BUILD_AZURE_BACKUP)
   set(FDBCLIENT_SRCS
@@ -64,6 +63,7 @@ if(BUILD_AZURE_BACKUP)
   )
 endif()
 
+
 if(WITH_AWS_BACKUP)
   add_compile_definitions(BUILD_AWS_BACKUP)
   include(awssdk)
@@ -83,7 +83,6 @@ target_include_directories(fdbclient_sampling PUBLIC "${CMAKE_CURRENT_SOURCE_DIR
 add_dependencies(fdbclient_sampling fdboptions)
 target_link_libraries(fdbclient_sampling PUBLIC fdbrpc_sampling msgpack PRIVATE rapidxml)
 target_compile_definitions(fdbclient_sampling PRIVATE -DENABLE_SAMPLING)
-
 if(WIN32)
   add_dependencies(fdbclient_sampling_actors fdbclient_actors)
 endif()

fdbclient/ClusterConnectionFile.actor.cpp

@@ -103,16 +103,6 @@ std::string ClusterConnectionFile::toString() const {
 	return "file://" + filename;
 }
 
-// Return the specified path of the cluster file
-Optional<std::string> ClusterConnectionFile::getFilename() const {
-	return filename;
-}
-
-// Returns true because cluster files are supported through the C API
-bool ClusterConnectionFile::supportedExternally() const {
-	return true;
-}
-
 // returns <resolved name, was default file>
 std::pair<std::string, bool> ClusterConnectionFile::lookupClusterFileName(std::string const& filename) {
 	if (filename.length())

fdbclient/ClusterConnectionKey.actor.cpp

@@ -0,0 +1,167 @@
+/*
+ * ClusterConnectionKey.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 "fdbclient/ClusterConnectionKey.actor.h"
+#include "flow/actorcompiler.h" // has to be last include
+
+// Creates a cluster connection record with a given connection string and saves it to the specified key. Needs to be
+// persisted should be set to true unless this ClusterConnectionKey is being created with the value read from the
+// key.
+ClusterConnectionKey::ClusterConnectionKey(Database db,
+                                           Key connectionStringKey,
+                                           ClusterConnectionString const& contents,
+                                           ConnectionStringNeedsPersisted needsToBePersisted)
+  : IClusterConnectionRecord(needsToBePersisted), db(db), connectionStringKey(connectionStringKey) {
+	if (!needsToBePersisted) {
+		lastPersistedConnectionString = ValueRef(contents.toString());
+	}
+	cs = contents;
+}
+
+// Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the
+// format is invalid.
+ACTOR Future<Reference<ClusterConnectionKey>> ClusterConnectionKey::loadClusterConnectionKey(Database db,
+                                                                                             Key connectionStringKey) {
+	state Transaction tr(db);
+	loop {
+		try {
+			Optional<Value> v = wait(tr.get(connectionStringKey));
+			if (!v.present()) {
+				throw connection_string_invalid();
+			}
+			return makeReference<ClusterConnectionKey>(db,
+			                                           connectionStringKey,
+			                                           ClusterConnectionString(v.get().toString()),
+			                                           ConnectionStringNeedsPersisted::False);
+		} catch (Error& e) {
+			wait(tr.onError(e));
+		}
+	}
+}
+
+// Sets the connections string held by this object and persists it.
+Future<Void> ClusterConnectionKey::setAndPersistConnectionString(ClusterConnectionString const& connectionString) {
+	cs = connectionString;
+	return success(persist());
+}
+
+// Get the connection string stored in the database.
+ACTOR Future<ClusterConnectionString> ClusterConnectionKey::getStoredConnectionStringImpl(
+    Reference<ClusterConnectionKey> self) {
+	Reference<ClusterConnectionKey> cck =
+	    wait(ClusterConnectionKey::loadClusterConnectionKey(self->db, self->connectionStringKey));
+	return cck->cs;
+}
+
+Future<ClusterConnectionString> ClusterConnectionKey::getStoredConnectionString() {
+	return getStoredConnectionStringImpl(Reference<ClusterConnectionKey>::addRef(this));
+}
+
+ACTOR Future<bool> ClusterConnectionKey::upToDateImpl(Reference<ClusterConnectionKey> self,
+                                                      ClusterConnectionString* connectionString) {
+	try {
+		// the cluster file hasn't been created yet so there's nothing to check
+		if (self->needsToBePersisted())
+			return true;
+
+		Reference<ClusterConnectionKey> temp =
+		    wait(ClusterConnectionKey::loadClusterConnectionKey(self->db, self->connectionStringKey));
+		*connectionString = temp->getConnectionString();
+		return connectionString->toString() == self->cs.toString();
+	} catch (Error& e) {
+		TraceEvent(SevWarnAlways, "ClusterKeyError").error(e).detail("Key", self->connectionStringKey);
+		return false; // Swallow the error and report that the file is out of date
+	}
+}
+
+// Checks whether the connection string in the database matches the connection string stored in memory. The cluster
+// string stored in the database is returned via the reference parameter connectionString.
+Future<bool> ClusterConnectionKey::upToDate(ClusterConnectionString& connectionString) {
+	return upToDateImpl(Reference<ClusterConnectionKey>::addRef(this), &connectionString);
+}
+
+// Returns the key where the connection string is stored.
+std::string ClusterConnectionKey::getLocation() const {
+	return printable(connectionStringKey);
+}
+
+// Creates a copy of this object with a modified connection string but that isn't persisted.
+Reference<IClusterConnectionRecord> ClusterConnectionKey::makeIntermediateRecord(
+    ClusterConnectionString const& connectionString) const {
+	return makeReference<ClusterConnectionKey>(db, connectionStringKey, connectionString);
+}
+
+// Returns a string representation of this cluster connection record. This will include the type of record and the
+// key where the record is stored.
+std::string ClusterConnectionKey::toString() const {
+	return "fdbkey://" + printable(connectionStringKey);
+}
+
+ACTOR Future<bool> ClusterConnectionKey::persistImpl(Reference<ClusterConnectionKey> self) {
+	self->setPersisted();
+	state Value newConnectionString = ValueRef(self->cs.toString());
+
+	try {
+		state Transaction tr(self->db);
+		loop {
+			try {
+				Optional<Value> existingConnectionString = wait(tr.get(self->connectionStringKey));
+				// Someone has already updated the connection string to what we want
+				if (existingConnectionString.present() && existingConnectionString.get() == newConnectionString) {
+					self->lastPersistedConnectionString = newConnectionString;
+					return true;
+				}
+				// Someone has updated the connection string to something we didn't expect, in which case we leave it
+				// alone. It's possible this could result in the stored string getting stuck if the connection string
+				// changes twice and only the first change is recorded. If the process that wrote the first change dies
+				// and no other process attempts to write the intermediate state, then only a newly opened connection
+				// key would be able to update the state.
+				else if (existingConnectionString.present() &&
+				         existingConnectionString != self->lastPersistedConnectionString) {
+					TraceEvent(SevWarnAlways, "UnableToChangeConnectionKeyDueToMismatch")
+					    .detail("ConnectionKey", self->connectionStringKey)
+					    .detail("NewConnectionString", newConnectionString)
+					    .detail("ExpectedStoredConnectionString", self->lastPersistedConnectionString)
+					    .detail("ActualStoredConnectionString", existingConnectionString);
+					return false;
+				}
+				tr.set(self->connectionStringKey, newConnectionString);
+				wait(tr.commit());
+
+				self->lastPersistedConnectionString = newConnectionString;
+				return true;
+			} catch (Error& e) {
+				wait(tr.onError(e));
+			}
+		}
+	} catch (Error& e) {
+		TraceEvent(SevWarnAlways, "UnableToChangeConnectionKey")
+		    .error(e)
+		    .detail("ConnectionKey", self->connectionStringKey)
+		    .detail("ConnectionString", self->cs.toString());
+	}
+
+	return false;
+};
+
+// Writes the connection string to the database
+Future<bool> ClusterConnectionKey::persist() {
+	return persistImpl(Reference<ClusterConnectionKey>::addRef(this));
+}

fdbclient/ClusterConnectionMemoryRecord.actor.cpp

@@ -57,11 +57,6 @@ std::string ClusterConnectionMemoryRecord::toString() const {
 	return "memory://" + id.toString();
 }
 
-// Returns true because memory connection records are supported through the C API
-bool ClusterConnectionMemoryRecord::supportedExternally() const {
-	return true;
-}
-
 // This is a no-op for memory records. Returns true to indicate success.
 Future<bool> ClusterConnectionMemoryRecord::persist() {
 	return true;

fdbclient/SpecialKeySpace.actor.cpp

@@ -36,7 +36,6 @@
 #include "flow/Arena.h"
 #include "flow/UnitTest.h"
 #include "fdbclient/ManagementAPI.actor.h"
-#include "fdbclient/MetaclusterManagement.actor.h"
 #include "fdbclient/StatusClient.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 

fdbclient/include/fdbclient/ClusterConnectionFile.h

@@ -25,9 +25,7 @@
 #include "fdbclient/CoordinationInterface.h"
 
 // An implementation of IClusterConnectionRecord backed by a file.
-class ClusterConnectionFile : public IClusterConnectionRecord,
+class ClusterConnectionFile : public IClusterConnectionRecord, ReferenceCounted<ClusterConnectionFile>, NonCopyable {
-                              ThreadSafeReferenceCounted<ClusterConnectionFile>,
-                              NonCopyable {
 public:
 	// Loads and parses the file at 'filename', throwing errors if the file cannot be read or the format is invalid.
 	explicit ClusterConnectionFile(std::string const& filename);
@@ -61,14 +59,8 @@ public:
 	// filename of the cluster file.
 	std::string toString() const override;
 
-	// Return the specified path of the cluster file
+	void addref() override { ReferenceCounted<ClusterConnectionFile>::addref(); }
-	virtual Optional<std::string> getFilename() const override;
+	void delref() override { ReferenceCounted<ClusterConnectionFile>::delref(); }
-
-	// Returns true because cluster files are supported through the C API
-	bool supportedExternally() const override;
-
-	void addref() override { ThreadSafeReferenceCounted<ClusterConnectionFile>::addref(); }
-	void delref() override { ThreadSafeReferenceCounted<ClusterConnectionFile>::delref(); }
 
 	// returns <resolved name, was default file>
 	static std::pair<std::string, bool> lookupClusterFileName(std::string const& filename);

fdbclient/include/fdbclient/ClusterConnectionKey.actor.h

@@ -17,9 +17,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #pragma once
-
 // When actually compiled (NO_INTELLISENSE), include the generated version of this file.  In intellisense use the source
 // version.
 #if defined(NO_INTELLISENSE) && !defined(FDBCLIENT_CLUSTERCONNECTIONKEY_ACTOR_G_H)
@@ -29,170 +27,63 @@
 #define FDBCLIENT_CLUSTERCONNECTIONKEY_ACTOR_H
 
 #include "fdbclient/CoordinationInterface.h"
-#include "fdbclient/GenericTransactionHelper.h"
 #include "fdbclient/NativeAPI.actor.h"
 #include "flow/actorcompiler.h" // has to be last include
 
 // An implementation of IClusterConnectionRecord backed by a key in a FoundationDB database.
-template <class DB>
+class ClusterConnectionKey : public IClusterConnectionRecord, ReferenceCounted<ClusterConnectionKey>, NonCopyable {
-class ClusterConnectionKey : public IClusterConnectionRecord,
-                             ThreadSafeReferenceCounted<ClusterConnectionKey<DB>>,
-                             NonCopyable {
 public:
 	// Creates a cluster connection record with a given connection string and saves it to the specified key. Needs to be
 	// persisted should be set to true unless this ClusterConnectionKey is being created with the value read from the
 	// key.
-	ClusterConnectionKey(DB db,
+	ClusterConnectionKey(Database db,
 	                     Key connectionStringKey,
 	                     ClusterConnectionString const& contents,
-	                     ConnectionStringNeedsPersisted needsToBePersisted = ConnectionStringNeedsPersisted::True)
+	                     ConnectionStringNeedsPersisted needsToBePersisted = ConnectionStringNeedsPersisted::True);
-	  : IClusterConnectionRecord(needsToBePersisted), db(db), connectionStringKey(connectionStringKey) {
-		if (!needsToBePersisted) {
-			lastPersistedConnectionString = ValueRef(contents.toString());
-		}
-		cs = contents;
-	}
 
-	// Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or
+	// Loads and parses the connection string at the specified key, throwing errors if the file cannot be read or the
-	// the format is invalid.
+	// format is invalid.
-	ACTOR static Future<Reference<ClusterConnectionKey>> loadClusterConnectionKey(DB db, Key connectionStringKey) {
+	ACTOR static Future<Reference<ClusterConnectionKey>> loadClusterConnectionKey(Database db, Key connectionStringKey);
-		state Reference<typename DB::TransactionT> tr = db->createTransaction();
-		loop {
-			try {
-				typename transaction_future_type<Transaction, Optional<Value>>::type f = tr->get(connectionStringKey);
-				Optional<Value> v = wait(safeThreadFutureToFuture(f));
-				if (!v.present()) {
-					throw connection_string_invalid();
-				}
-				return makeReference<ClusterConnectionKey>(db,
-				                                           connectionStringKey,
-				                                           ClusterConnectionString(v.get().toString()),
-				                                           ConnectionStringNeedsPersisted::False);
-			} catch (Error& e) {
-				wait(safeThreadFutureToFuture(tr->onError(e)));
-			}
-		}
-	}
 
 	// Sets the connections string held by this object and persists it.
-	Future<Void> setAndPersistConnectionString(ClusterConnectionString const& connectionString) override {
+	Future<Void> setAndPersistConnectionString(ClusterConnectionString const&) override;
-		cs = connectionString;
-		return success(persist());
-	}
 
 	// Get the connection string stored in the database.
-	Future<ClusterConnectionString> getStoredConnectionString() override {
+	Future<ClusterConnectionString> getStoredConnectionString() override;
-		return getStoredConnectionStringImpl(Reference<ClusterConnectionKey>::addRef(this));
-	}
 
-	// Checks whether the connection string in the database matches the connection string stored in memory. The
+	// Checks whether the connection string in the database matches the connection string stored in memory. The cluster
-	// cluster string stored in the database is returned via the reference parameter connectionString.
+	// string stored in the database is returned via the reference parameter connectionString.
-	Future<bool> upToDate(ClusterConnectionString& connectionString) override {
+	Future<bool> upToDate(ClusterConnectionString& connectionString) override;
-		return upToDateImpl(Reference<ClusterConnectionKey>::addRef(this), &connectionString);
-	}
 
 	// Returns the key where the connection string is stored.
-	std::string getLocation() const override { return printable(connectionStringKey); }
+	std::string getLocation() const override;
 
 	// Creates a copy of this object with a modified connection string but that isn't persisted.
 	Reference<IClusterConnectionRecord> makeIntermediateRecord(
-	    ClusterConnectionString const& connectionString) const override {
+	    ClusterConnectionString const& connectionString) const override;
-		return makeReference<ClusterConnectionKey>(db, connectionStringKey, connectionString);
-	}
 
-	// Returns a string representation of this cluster connection record. This will include the type of record and
+	// Returns a string representation of this cluster connection record. This will include the type of record and the
-	// the key where the record is stored.
+	// key where the record is stored.
-	std::string toString() const override { return "fdbkey://" + printable(connectionStringKey); }
+	std::string toString() const override;
 
-	// Returns false because cluster connection keys are not supported through the C API
+	void addref() override { ReferenceCounted<ClusterConnectionKey>::addref(); }
-	bool supportedExternally() const { return false; }
+	void delref() override { ReferenceCounted<ClusterConnectionKey>::delref(); }
-
-	void addref() override { ThreadSafeReferenceCounted<ClusterConnectionKey>::addref(); }
-	void delref() override { ThreadSafeReferenceCounted<ClusterConnectionKey>::delref(); }
 
 protected:
 	// Writes the connection string to the database
-	Future<bool> persist() override { return persistImpl(Reference<ClusterConnectionKey>::addRef(this)); }
+	Future<bool> persist() override;
 
 private:
-	ACTOR static Future<ClusterConnectionString> getStoredConnectionStringImpl(Reference<ClusterConnectionKey> self) {
+	ACTOR static Future<ClusterConnectionString> getStoredConnectionStringImpl(Reference<ClusterConnectionKey> self);
-		Reference<ClusterConnectionKey> cck =
-		    wait(ClusterConnectionKey::loadClusterConnectionKey(self->db, self->connectionStringKey));
-		return cck->cs;
-	}
-
 	ACTOR static Future<bool> upToDateImpl(Reference<ClusterConnectionKey> self,
-	                                       ClusterConnectionString* connectionString) {
+	                                       ClusterConnectionString* connectionString);
-		try {
+	ACTOR static Future<bool> persistImpl(Reference<ClusterConnectionKey> self);
-			// the cluster file hasn't been created yet so there's nothing to check
-			if (self->needsToBePersisted())
-				return true;
 
-			Reference<ClusterConnectionKey> temp =
+	// The database where the connection key is stored. Note that this does not need to be the same database as the one
-			    wait(ClusterConnectionKey::loadClusterConnectionKey(self->db, self->connectionStringKey));
+	// that the connection string would connect to.
-			*connectionString = temp->getConnectionString();
+	Database db;
-			return connectionString->toString() == self->cs.toString();
-		} catch (Error& e) {
-			TraceEvent(SevWarnAlways, "ClusterKeyError").error(e).detail("Key", self->connectionStringKey);
-			return false; // Swallow the error and report that the file is out of date
-		}
-	}
-
-	ACTOR static Future<bool> persistImpl(Reference<ClusterConnectionKey> self) {
-		self->setPersisted();
-		state Value newConnectionString = ValueRef(self->cs.toString());
-
-		try {
-			state Reference<typename DB::TransactionT> tr = self->db->createTransaction();
-			loop {
-				try {
-					typename transaction_future_type<Transaction, Optional<Value>>::type f =
-					    tr->get(self->connectionStringKey);
-					Optional<Value> existingConnectionString = wait(safeThreadFutureToFuture(f));
-					// Someone has already updated the connection string to what we want
-					if (existingConnectionString.present() && existingConnectionString.get() == newConnectionString) {
-						self->lastPersistedConnectionString = newConnectionString;
-						return true;
-					}
-					// Someone has updated the connection string to something we didn't expect, in which case we leave
-					// it alone. It's possible this could result in the stored string getting stuck if the connection
-					// string changes twice and only the first change is recorded. If the process that wrote the first
-					// change dies and no other process attempts to write the intermediate state, then only a newly
-					// opened connection key would be able to update the state.
-					else if (existingConnectionString.present() &&
-					         existingConnectionString != self->lastPersistedConnectionString) {
-						TraceEvent(SevWarnAlways, "UnableToChangeConnectionKeyDueToMismatch")
-						    .detail("ConnectionKey", self->connectionStringKey)
-						    .detail("NewConnectionString", newConnectionString)
-						    .detail("ExpectedStoredConnectionString", self->lastPersistedConnectionString)
-						    .detail("ActualStoredConnectionString", existingConnectionString);
-						return false;
-					}
-					tr->set(self->connectionStringKey, newConnectionString);
-					wait(safeThreadFutureToFuture(tr->commit()));
-
-					self->lastPersistedConnectionString = newConnectionString;
-					return true;
-				} catch (Error& e) {
-					wait(safeThreadFutureToFuture(tr->onError(e)));
-				}
-			}
-		} catch (Error& e) {
-			TraceEvent(SevWarnAlways, "UnableToChangeConnectionKey")
-			    .error(e)
-			    .detail("ConnectionKey", self->connectionStringKey)
-			    .detail("ConnectionString", self->cs.toString());
-		}
-
-		return false;
-	}
-
-	// The database where the connection key is stored. Note that this does not need to be the same database as the
-	// one that the connection string would connect to.
-	DB db;
 	Key connectionStringKey;
 	Optional<Value> lastPersistedConnectionString;
 };
-
 #include "flow/unactorcompiler.h"
 #endif

fdbclient/include/fdbclient/ClusterConnectionMemoryRecord.h

@@ -26,7 +26,7 @@
 
 // An implementation of IClusterConnectionRecord that is stored in memory only and not persisted.
 class ClusterConnectionMemoryRecord : public IClusterConnectionRecord,
-                                      ThreadSafeReferenceCounted<ClusterConnectionMemoryRecord>,
+                                      ReferenceCounted<ClusterConnectionMemoryRecord>,
                                       NonCopyable {
 public:
 	// Creates a cluster file with a given connection string.
@@ -56,11 +56,8 @@ public:
 	// record.
 	std::string toString() const override;
 
-	// Returns true because memory connection records are supported through the C API
+	void addref() override { ReferenceCounted<ClusterConnectionMemoryRecord>::addref(); }
-	bool supportedExternally() const override;
+	void delref() override { ReferenceCounted<ClusterConnectionMemoryRecord>::delref(); }
-
-	void addref() override { ThreadSafeReferenceCounted<ClusterConnectionMemoryRecord>::addref(); }
-	void delref() override { ThreadSafeReferenceCounted<ClusterConnectionMemoryRecord>::delref(); }
 
 protected:
 	// This is a no-op for memory records. Returns true to indicate success.

fdbclient/include/fdbclient/CoordinationInterface.h

@@ -149,13 +149,6 @@ public:
 	// Signals to the connection record that it was successfully used to connect to a cluster.
 	void notifyConnected();
 
-	// If the connection string is backed by a file, returns the filename. Otherwise, returns an empty optional.
-	// The interface provides a default implementation that assumes no file-backing.
-	virtual Optional<std::string> getFilename() const { return Optional<std::string>(); }
-
-	// Returns true if this type of cluster connection record is exposed through the C API
-	virtual bool supportedExternally() const = 0;
-
 	virtual void addref() = 0;
 	virtual void delref() = 0;
 

fdbclient/include/fdbclient/GenericManagementAPI.actor.h

@@ -36,13 +36,11 @@ the contents of the system key space.
 #include <map>
 #include "fdbclient/ClientBooleanParams.h"
 #include "fdbclient/DatabaseConfiguration.h"
-#include "fdbclient/GenericTransactionHelper.h"
 #include "fdbclient/Status.h"
 #include "fdbclient/Subspace.h"
 #include "fdbclient/DatabaseConfiguration.h"
 #include "fdbclient/Status.h"
 #include "fdbclient/SystemData.h"
-#include "fdbclient/TenantManagement.actor.h"
 #include "flow/actorcompiler.h" // has to be last include
 
 // ConfigurationResult enumerates normal outcomes of changeConfig() and various error
@@ -206,11 +204,10 @@ Future<Void> removeCachedRange(Reference<DB> db, KeyRangeRef range) {
 	return changeCachedRange(db, range, false);
 }
 
-ACTOR template <class Transaction>
+ACTOR template <class Tr>
-Future<std::vector<ProcessData>> getWorkers(
+Future<std::vector<ProcessData>> getWorkers(Reference<Tr> tr,
-    Transaction tr,
+                                            typename Tr::template FutureT<RangeResult> processClassesF,
-    typename transaction_future_type<Transaction, RangeResult>::type processClassesF,
+                                            typename Tr::template FutureT<RangeResult> processDataF) {
-    typename transaction_future_type<Transaction, RangeResult>::type processDataF) {
 	// processClassesF and processDataF are used to hold standalone memory
 	processClassesF = tr->getRange(processClassKeys, CLIENT_KNOBS->TOO_MANY);
 	processDataF = tr->getRange(workerListKeys, CLIENT_KNOBS->TOO_MANY);
@@ -248,31 +245,31 @@ Future<std::vector<ProcessData>> getWorkers(
 // ConfigurationResult (or error).
 
 // Accepts a full configuration in key/value format (from buildConfiguration)
-ACTOR template <class Transaction>
+ACTOR template <class DB>
-Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
+Future<ConfigurationResult> changeConfig(Reference<DB> db, std::map<std::string, std::string> m, bool force) {
-                                                    std::map<std::string, std::string> m,
+	state StringRef initIdKey = LiteralStringRef("\xff/init_id");
-                                                    bool force,
+	state Reference<typename DB::TransactionT> tr = db->createTransaction();
-                                                    Optional<UID> createId) {
-	state StringRef initIdKey = "\xff/init_id"_sr;
 
 	if (!m.size()) {
 		return ConfigurationResult::NO_OPTIONS_PROVIDED;
 	}
 
 	// make sure we have essential configuration options
+	std::string initKey = configKeysPrefix.toString() + "initialized";
+	state bool creating = m.count(initKey) != 0;
 	state Optional<UID> locked;
 	{
 		auto iter = m.find(databaseLockedKey.toString());
 		if (iter != m.end()) {
-			if (!createId.present()) {
+			if (!creating) {
 				return ConfigurationResult::LOCKED_NOT_NEW;
 			}
 			locked = UID::fromString(iter->second);
 			m.erase(iter);
 		}
 	}
-	if (createId.present()) {
+	if (creating) {
-		m[initIdKey.toString()] = createId.get().toString();
+		m[initIdKey.toString()] = deterministicRandom()->randomUniqueID().toString();
 		if (!isCompleteConfiguration(m)) {
 			return ConfigurationResult::INCOMPLETE_CONFIGURATION;
 		}
@@ -286,17 +283,19 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 	state bool warnRocksDBIsExperimental = false;
 	state bool warnShardedRocksDBIsExperimental = false;
 
+	loop {
+		try {
 			tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 			tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE);
 			tr->setOption(FDBTransactionOptions::LOCK_AWARE);
 			tr->setOption(FDBTransactionOptions::USE_PROVISIONAL_PROXIES);
 
-	if (!createId.present() && !force) {
+			if (!creating && !force) {
-		state typename transaction_future_type<Transaction, RangeResult>::type fConfigF =
+				state typename DB::TransactionT::template FutureT<RangeResult> fConfigF =
 				    tr->getRange(configKeys, CLIENT_KNOBS->TOO_MANY);
 				state Future<RangeResult> fConfig = safeThreadFutureToFuture(fConfigF);
-		state typename transaction_future_type<Transaction, RangeResult>::type processClassesF;
+				state typename DB::TransactionT::template FutureT<RangeResult> processClassesF;
-		state typename transaction_future_type<Transaction, RangeResult>::type processDataF;
+				state typename DB::TransactionT::template FutureT<RangeResult> processDataF;
 				state Future<std::vector<ProcessData>> fWorkers = getWorkers(tr, processClassesF, processDataF);
 				wait(success(fConfig) || tooLong);
 
@@ -320,7 +319,8 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 						return ConfigurationResult::REGION_REPLICATION_MISMATCH;
 					}
 
-			oldReplicationUsesDcId = oldReplicationUsesDcId || oldConfig.tLogPolicy->attributeKeys().count("dcid");
+					oldReplicationUsesDcId =
+					    oldReplicationUsesDcId || oldConfig.tLogPolicy->attributeKeys().count("dcid");
 
 					if (oldConfig.usableRegions != newConfig.usableRegions) {
 						// cannot change region configuration
@@ -346,14 +346,14 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 						}
 					}
 
-			state typename transaction_future_type<Transaction, RangeResult>::type fServerListF =
+					state typename DB::TransactionT::template FutureT<RangeResult> fServerListF =
 					    tr->getRange(serverListKeys, CLIENT_KNOBS->TOO_MANY);
 					state Future<RangeResult> fServerList =
 					    (newConfig.regions.size()) ? safeThreadFutureToFuture(fServerListF) : Future<RangeResult>();
 
 					if (newConfig.usableRegions == 2) {
 						if (oldReplicationUsesDcId) {
-					state typename transaction_future_type<Transaction, RangeResult>::type fLocalityListF =
+							state typename DB::TransactionT::template FutureT<RangeResult> fLocalityListF =
 							    tr->getRange(tagLocalityListKeys, CLIENT_KNOBS->TOO_MANY);
 							state Future<RangeResult> fLocalityList = safeThreadFutureToFuture(fLocalityListF);
 							wait(success(fLocalityList) || tooLong);
@@ -378,7 +378,7 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 							}
 						} else {
 							// all regions with priority >= 0 must be fully replicated
-					state std::vector<typename transaction_future_type<Transaction, Optional<Value>>::type>
+							state std::vector<typename DB::TransactionT::template FutureT<Optional<Value>>>
 							    replicasFuturesF;
 							state std::vector<Future<Optional<Value>>> replicasFutures;
 							for (auto& it : newConfig.regions) {
@@ -479,18 +479,18 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 					}
 
 					if (newConfig.tenantMode != oldConfig.tenantMode) {
-				state
+						state typename DB::TransactionT::template FutureT<Optional<Value>>
-				    typename transaction_future_type<Transaction, Optional<Value>>::type metaclusterRegistrationFuture =
+						    metaclusterRegistrationFuture = tr->get(metaclusterRegistrationKey);
-				        tr->get(metaclusterRegistrationKey);
 
-				Optional<Value> metaclusterRegistration = wait(safeThreadFutureToFuture(metaclusterRegistrationFuture));
+						Optional<Value> metaclusterRegistration =
+						    wait(safeThreadFutureToFuture(metaclusterRegistrationFuture));
 						if (metaclusterRegistration.present()) {
 							return ConfigurationResult::DATABASE_IS_REGISTERED;
 						}
 					}
 				}
 			}
-	if (createId.present()) {
+			if (creating) {
 				tr->setOption(FDBTransactionOptions::INITIALIZE_NEW_DATABASE);
 				tr->addReadConflictRange(singleKeyRange(initIdKey));
 			} else if (m.size()) {
@@ -501,7 +501,7 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 			}
 
 			if (locked.present()) {
-		ASSERT(createId.present());
+				ASSERT(creating);
 				tr->atomicOp(databaseLockedKey,
 				             BinaryWriter::toValue(locked.get(), Unversioned())
 				                 .withPrefix(LiteralStringRef("0123456789"))
@@ -515,38 +515,10 @@ Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
 
 			tr->addReadConflictRange(singleKeyRange(moveKeysLockOwnerKey));
 			tr->set(moveKeysLockOwnerKey, versionKey);
-
+			break;
-	if (warnPPWGradual) {
-		return ConfigurationResult::SUCCESS_WARN_PPW_GRADUAL;
-	} else if (warnRocksDBIsExperimental) {
-		return ConfigurationResult::SUCCESS_WARN_ROCKSDB_EXPERIMENTAL;
-	} else if (warnShardedRocksDBIsExperimental) {
-		return ConfigurationResult::SUCCESS_WARN_SHARDED_ROCKSDB_EXPERIMENTAL;
-	} else {
-		return ConfigurationResult::SUCCESS;
-	}
-}
-
-// Accepts a full configuration in key/value format (from buildConfiguration)
-ACTOR template <class DB>
-Future<ConfigurationResult> changeConfig(Reference<DB> db, std::map<std::string, std::string> m, bool force) {
-	state StringRef initIdKey = "\xff/init_id"_sr;
-	state Reference<typename DB::TransactionT> tr = db->createTransaction();
-
-	state Optional<UID> createId;
-	if (m.count(configKeysPrefix.toString() + "initialized") != 0) {
-		createId = deterministicRandom()->randomUniqueID();
-	}
-
-	loop {
-		try {
-			state ConfigurationResult result = wait(changeConfigTransaction(tr, m, force, createId));
-			wait(safeThreadFutureToFuture(tr->commit()));
-			return result;
 		} catch (Error& e) {
 			state Error e1(e);
-			if ((e.code() == error_code_not_committed || e.code() == error_code_transaction_too_old) &&
+			if ((e.code() == error_code_not_committed || e.code() == error_code_transaction_too_old) && creating) {
-			    createId.present()) {
 				// The database now exists.  Determine whether we created it or it was already existing/created by
 				// someone else.  The latter is an error.
 				tr->reset();
@@ -559,17 +531,16 @@ Future<ConfigurationResult> changeConfig(Reference<DB> db, std::map<std::string,
 
 						state typename DB::TransactionT::template FutureT<Optional<Value>> vF = tr->get(initIdKey);
 						Optional<Value> v = wait(safeThreadFutureToFuture(vF));
-						if (v != createId.get().toString()) {
+						if (v != m[initIdKey.toString()])
 							return ConfigurationResult::DATABASE_ALREADY_CREATED;
-						} else if (m[configKeysPrefix.toString() + "storage_engine"] ==
+						else if (m[configKeysPrefix.toString() + "storage_engine"] ==
-						           std::to_string(KeyValueStoreType::SSD_ROCKSDB_V1)) {
+						         std::to_string(KeyValueStoreType::SSD_ROCKSDB_V1))
 							return ConfigurationResult::DATABASE_CREATED_WARN_ROCKSDB_EXPERIMENTAL;
-						} else if (m[configKeysPrefix.toString() + "storage_engine"] ==
+						else if (m[configKeysPrefix.toString() + "storage_engine"] ==
-						           std::to_string(KeyValueStoreType::SSD_SHARDED_ROCKSDB)) {
+						         std::to_string(KeyValueStoreType::SSD_SHARDED_ROCKSDB))
 							return ConfigurationResult::DATABASE_CREATED_WARN_SHARDED_ROCKSDB_EXPERIMENTAL;
-						} else {
+						else
 							return ConfigurationResult::DATABASE_CREATED;
-						}
 					} catch (Error& e2) {
 						wait(safeThreadFutureToFuture(tr->onError(e2)));
 					}
@@ -578,22 +549,35 @@ Future<ConfigurationResult> changeConfig(Reference<DB> db, std::map<std::string,
 			wait(safeThreadFutureToFuture(tr->onError(e1)));
 		}
 	}
+
+	if (warnPPWGradual) {
+		return ConfigurationResult::SUCCESS_WARN_PPW_GRADUAL;
+	} else if (warnRocksDBIsExperimental) {
+		return ConfigurationResult::SUCCESS_WARN_ROCKSDB_EXPERIMENTAL;
+	} else if (warnShardedRocksDBIsExperimental) {
+		return ConfigurationResult::SUCCESS_WARN_SHARDED_ROCKSDB_EXPERIMENTAL;
+	} else {
+		return ConfigurationResult::SUCCESS;
+	}
 }
 
-ACTOR template <class Transaction>
+ACTOR template <class DB>
-Future<ConfigurationResult> autoConfigTransaction(Transaction tr, ConfigureAutoResult conf) {
+Future<ConfigurationResult> autoConfig(Reference<DB> db, ConfigureAutoResult conf) {
+	state Reference<typename DB::TransactionT> tr = db->createTransaction();
 	state Key versionKey = BinaryWriter::toValue(deterministicRandom()->randomUniqueID(), Unversioned());
 
 	if (!conf.address_class.size())
 		return ConfigurationResult::INCOMPLETE_CONFIGURATION; // FIXME: correct return type
 
+	loop {
+		try {
 			tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
 			tr->setOption(FDBTransactionOptions::PRIORITY_SYSTEM_IMMEDIATE);
 			tr->setOption(FDBTransactionOptions::LOCK_AWARE);
 			tr->setOption(FDBTransactionOptions::USE_PROVISIONAL_PROXIES);
 
-	state typename transaction_future_type<Transaction, RangeResult>::type processClassesF;
+			state typename DB::TransactionT::template FutureT<RangeResult> processClassesF;
-	state typename transaction_future_type<Transaction, RangeResult>::type processDataF;
+			state typename DB::TransactionT::template FutureT<RangeResult> processDataF;
 			std::vector<ProcessData> workers = wait(getWorkers(tr, processClassesF, processDataF));
 			std::map<NetworkAddress, Optional<Standalone<StringRef>>> address_processId;
 			for (auto& w : workers) {
@@ -639,16 +623,6 @@ Future<ConfigurationResult> autoConfigTransaction(Transaction tr, ConfigureAutoR
 			tr->set(moveKeysLockOwnerKey, versionKey);
 
 			return ConfigurationResult::SUCCESS;
-}
-
-ACTOR template <class DB>
-Future<ConfigurationResult> autoConfig(Reference<DB> db, ConfigureAutoResult conf) {
-	state Reference<typename DB::TransactionT> tr = db->createTransaction();
-	loop {
-		try {
-			state ConfigurationResult result = wait(autoConfigTransaction(tr, conf));
-			wait(safeThreadFutureToFuture(tr->commit()));
-			return result;
 		} catch (Error& e) {
 			wait(safeThreadFutureToFuture(tr->onError(e)));
 		}
@@ -666,23 +640,6 @@ Future<ConfigurationResult> changeConfig(Reference<DB> db, std::string const& mo
 	return changeConfig(db, m, force);
 }
 
-template <class Transaction>
-Future<ConfigurationResult> changeConfigTransaction(Transaction tr,
-                                                    std::vector<StringRef> const& modes,
-                                                    Optional<ConfigureAutoResult> const& conf,
-                                                    bool force,
-                                                    Optional<UID> createId) {
-	if (modes.size() && modes[0] == LiteralStringRef("auto") && conf.present()) {
-		return autoConfigTransaction(tr, conf.get());
-	}
-
-	std::map<std::string, std::string> m;
-	auto r = buildConfiguration(modes, m);
-	if (r != ConfigurationResult::SUCCESS)
-		return r;
-	return changeConfigTransaction(tr, m, force, createId);
-}
-
 // Accepts a vector of configuration tokens
 template <class DB>
 Future<ConfigurationResult> changeConfig(Reference<DB> db,

fdbclient/include/fdbclient/IClientApi.h

@@ -22,7 +22,6 @@
 #define FDBCLIENT_ICLIENTAPI_H
 #pragma once
 
-#include "fdbclient/CoordinationInterface.h"
 #include "fdbclient/FDBOptions.g.h"
 #include "fdbclient/FDBTypes.h"
 #include "fdbclient/Tenant.h"

fdbclient/include/fdbclient/MultiVersionTransaction.h

@@ -522,8 +522,8 @@ class MultiVersionDatabase;
 class MultiVersionTenant;
 
 // An implementation of ITransaction that wraps a transaction created either locally or through a dynamically loaded
-// external client. When needed (e.g on cluster version change), the MultiVersionTransaction can automatically
+// external client. When needed (e.g on cluster version change), the MultiVersionTransaction can automatically replace
-// replace its wrapped transaction with one from another client.
+// its wrapped transaction with one from another client.
 class MultiVersionTransaction : public ITransaction, ThreadSafeReferenceCounted<MultiVersionTransaction> {
 public:
 	MultiVersionTransaction(Reference<MultiVersionDatabase> db,
@@ -625,8 +625,8 @@ private:
 	// A lock that needs to be held if using timeoutTsav or currentTimeout
 	ThreadSpinLock timeoutLock;
 
-	// A single assignment var (i.e. promise) that gets set with an error when the timeout elapses or the
+	// A single assignment var (i.e. promise) that gets set with an error when the timeout elapses or the transaction
-	// transaction is reset or destroyed.
+	// is reset or destroyed.
 	Reference<ThreadSingleAssignmentVar<Void>> timeoutTsav;
 
 	// A reference to the current actor waiting for the timeout. This actor will set the timeoutTsav promise.
@@ -793,8 +793,8 @@ public:
 	double getMainThreadBusyness() override;
 
 	// Returns the protocol version reported by the coordinator this client is connected to
-	// If an expected version is given, the future won't return until the protocol version is different than
+	// If an expected version is given, the future won't return until the protocol version is different than expected
-	// expected Note: this will never return if the server is running a protocol from FDB 5.0 or older
+	// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 	ThreadFuture<ProtocolVersion> getServerProtocol(
 	    Optional<ProtocolVersion> expectedVersion = Optional<ProtocolVersion>()) override;
 

fdbclient/include/fdbclient/SpecialKeySpace.actor.h

@@ -258,7 +258,7 @@ private:
 
 	static std::unordered_map<SpecialKeySpace::MODULE, KeyRange> moduleToBoundary;
 
-	// module command to special keys range
+	// management command to its special keys' range
 	static std::unordered_map<std::string, KeyRange> managementApiCommandToRange;
 	static std::unordered_map<std::string, KeyRange> actorLineageApiCommandToRange;
 

fdbclient/include/fdbclient/SystemData.h

@@ -26,7 +26,6 @@
 
 #include "fdbclient/FDBTypes.h"
 #include "fdbclient/BlobWorkerInterface.h" // TODO move the functions that depend on this out of here and into BlobWorkerInterface.h to remove this depdendency
-#include "fdbclient/Metacluster.h"
 #include "fdbclient/StorageServerInterface.h"
 #include "Tenant.h"
 

fdbclient/include/fdbclient/TenantManagement.actor.h

@@ -28,6 +28,7 @@
 #include <string>
 #include <map>
 #include "fdbclient/GenericTransactionHelper.h"
+#include "fdbclient/Metacluster.h"
 #include "fdbclient/SystemData.h"
 #include "flow/actorcompiler.h" // has to be last include
 

fdbserver/ApplyMetadataMutation.cpp

@@ -20,6 +20,7 @@
 
 #include "fdbclient/BackupAgent.actor.h"
 #include "fdbclient/KeyBackedTypes.h" // for key backed map codecs for tss mapping
+#include "fdbclient/Metacluster.h"
 #include "fdbclient/MutationList.h"
 #include "fdbclient/Notified.h"
 #include "fdbclient/SystemData.h"

fdbserver/ClusterRecovery.actor.cpp

@@ -18,6 +18,7 @@
  * limitations under the License.
  */
 
+#include "fdbclient/Metacluster.h"
 #include "fdbrpc/sim_validation.h"
 #include "fdbserver/ApplyMetadataMutation.h"
 #include "fdbserver/BackupProgress.actor.h"