Address review comments:

Use nullptr instead of NULL
Use const& for a parameter
Add some comments

bindings/c/fdb_c.cpp

@@ -367,6 +367,7 @@ extern "C" DLLEXPORT double fdb_database_get_main_thread_busyness(FDBDatabase* d
 
 // Returns the protocol version reported by the coordinator this client is connected to
 // If an expected version is non-zero, the future won't return until the protocol version is different than expected
+// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 extern "C" DLLEXPORT FDBFuture* fdb_database_get_server_protocol(FDBDatabase* db, uint64_t expected_version) {
 	Optional<ProtocolVersion> expected;
 	if (expected_version > 0) {

fdbclient/DatabaseContext.h

@@ -201,6 +201,7 @@ public:
 
 	// 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 expected
+	// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 	Future<ProtocolVersion> getClusterProtocol(Optional<ProtocolVersion> expectedVersion = Optional<ProtocolVersion>());
 
 	// Update the watch counter for the database

fdbclient/IClientApi.h

@@ -102,6 +102,7 @@ public:
 
 	// 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 expected
+	// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 	virtual ThreadFuture<ProtocolVersion> getServerProtocol(
 	    Optional<ProtocolVersion> expectedVersion = Optional<ProtocolVersion>()) = 0;
 

fdbclient/MultiVersionTransaction.actor.cpp

@@ -361,6 +361,7 @@ double DLDatabase::getMainThreadBusyness() {
 
 // 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 expected
+// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 ThreadFuture<ProtocolVersion> DLDatabase::getServerProtocol(Optional<ProtocolVersion> expectedVersion) {
 	ASSERT(api->databaseGetServerProtocol != nullptr);
 
@@ -972,13 +973,14 @@ double MultiVersionDatabase::getMainThreadBusyness() {
 
 // 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 expected
+// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 ThreadFuture<ProtocolVersion> MultiVersionDatabase::getServerProtocol(Optional<ProtocolVersion> expectedVersion) {
 	return dbState->versionMonitorDb->getServerProtocol(expectedVersion);
 }
 
 MultiVersionDatabase::DatabaseState::DatabaseState(std::string clusterFilePath, Reference<IDatabase> versionMonitorDb)
   : clusterFilePath(clusterFilePath), versionMonitorDb(versionMonitorDb),
-    dbVar(new ThreadSafeAsyncVar<Reference<IDatabase>>(Reference<IDatabase>(NULL))) {}
+    dbVar(new ThreadSafeAsyncVar<Reference<IDatabase>>(Reference<IDatabase>(nullptr))) {}
 
 // Adds a client (local or externally loaded) that can be used to connect to the cluster
 void MultiVersionDatabase::DatabaseState::addClient(Reference<ClientInfo> client) {

fdbclient/MultiVersionTransaction.h

@@ -273,6 +273,7 @@ public:
 
 	// 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 expected
+	// 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;
 
@@ -448,6 +449,7 @@ public:
 
 	// 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 expected
+	// 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;
 
@@ -510,6 +512,9 @@ public:
 		ThreadFuture<Void> protocolVersionMonitor;
 		std::list<LegacyVersionMonitor> legacyVersionMonitors;
 		Optional<ProtocolVersion> dbProtocolVersion;
+
+		// This maps a normalized protocol version to the client associated with it. This prevents compatible
+		// differences in protocol version not matching each other.
 		std::map<ProtocolVersion, Reference<ClientInfo>> clients;
 
 		std::vector<std::pair<FDBDatabaseOptions::Option, Optional<Standalone<StringRef>>>> options;
@@ -520,7 +525,7 @@ public:
 	// A struct that enables monitoring whether the cluster is running an old version (<= 5.0) that doesn't support
 	// connect packet monitoring.
 	struct LegacyVersionMonitor {
-		LegacyVersionMonitor(Reference<ClientInfo> client) : client(client), monitorRunning(false) {}
+		LegacyVersionMonitor(Reference<ClientInfo> const& client) : client(client), monitorRunning(false) {}
 		~LegacyVersionMonitor() { TraceEvent("DestroyingVersionMonitor"); }
 
 		// Starts the connection monitor by creating a database object at an old version.

fdbclient/NativeAPI.actor.cpp

@@ -4982,6 +4982,7 @@ ACTOR Future<ProtocolVersion> getClusterProtocolImpl(
 
 // Returns the protocol version reported by the coordinator this client is currently connected to
 // If an expected version is given, the future won't return until the protocol version is different than expected
+// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 Future<ProtocolVersion> DatabaseContext::getClusterProtocol(Optional<ProtocolVersion> expectedVersion) {
 	return getClusterProtocolImpl(coordinator, expectedVersion);
 }

fdbclient/ThreadSafeTransaction.cpp

@@ -99,6 +99,7 @@ double ThreadSafeDatabase::getMainThreadBusyness() {
 
 // 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 expected
+// Note: this will never return if the server is running a protocol from FDB 5.0 or older
 ThreadFuture<ProtocolVersion> ThreadSafeDatabase::getServerProtocol(Optional<ProtocolVersion> expectedVersion) {
 	DatabaseContext* db = this->db;
 	return onMainThread(

fdbclient/ThreadSafeTransaction.h

@@ -41,6 +41,7 @@ public:
 
 	// 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 expected
+	// 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;