Addressing review comments

fdbclient/DatabaseContext.h

@@ -428,7 +428,12 @@ public:
 	static const std::vector<std::string> debugTransactionTagChoices;
 	std::unordered_map<KeyRef, Reference<WatchMetadata>> watchMap;
 
+        // Adds or updates the specified (SS, TSS) pair in the TSS mapping (if not already present).
+        // Requests to the storage server will be duplicated to the TSS.
 	void addTssMapping(StorageServerInterface const& ssi, StorageServerInterface const& tssi);
+
+        // Removes the storage server and its TSS pair from the TSS mapping (if present).
+        // Requests to the storage server will no longer be duplicated to its pair TSS.
 	void removeTssMapping(StorageServerInterface const& ssi);
 };
 

fdbrpc/QueueModel.h

@@ -109,8 +109,13 @@ public:
 	int laggingRequestCount;
 	int laggingTSSCompareCount;
 
+        // Updates this endpoint data to duplicate requests to the specified TSS endpoint
 	void updateTssEndpoint(uint64_t endpointId, const TSSEndpointData& endpointData);
+
+        // Removes the TSS mapping from this endpoint to stop duplicating requests to a TSS endpoint
 	void removeTssEndpoint(uint64_t endpointId);
+
+        // Retrieves the data for this endpoint's pair TSS endpoint, if present
 	Optional<TSSEndpointData> getTssData(uint64_t endpointId);
 
 	QueueModel() : secondMultiplier(1.0), secondBudget(0), laggingRequestCount(0) {
@@ -147,4 +152,4 @@ private:
 };
 */
 
-#endif
+#endif

fdbserver/CMakeLists.txt

@@ -103,7 +103,7 @@ set(FDBSERVER_SRCS
   TesterInterface.actor.h
   TLogInterface.h
   TLogServer.actor.cpp
-  TSSMappingUtil.h
+  TSSMappingUtil.actor.h
   TSSMappingUtil.actor.cpp
   VersionedBTree.actor.cpp
   VFSAsync.h

fdbserver/CommitProxyServer.actor.cpp

@@ -1430,6 +1430,7 @@ ACTOR Future<Void> commitBatch(ProxyCommitData* self,
 	return Void();
 }
 
+// Add tss mapping data to the reply, if any of the included storage servers have a TSS pair
 void maybeAddTssMapping(GetKeyServerLocationsReply& reply,
                         ProxyCommitData* commitData,
                         std::unordered_set<UID>& included,

fdbserver/MoveKeys.actor.cpp

@@ -24,7 +24,7 @@
 #include "fdbclient/SystemData.h"
 #include "fdbserver/MoveKeys.actor.h"
 #include "fdbserver/Knobs.h"
-#include "fdbserver/TSSMappingUtil.h"
+#include "fdbserver/TSSMappingUtil.actor.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
 using std::max;

fdbserver/TSSMappingUtil.actor.cpp

@@ -20,25 +20,9 @@
 
 #include "fdbclient/SystemData.h"
 #include "fdbclient/KeyBackedTypes.h"
-#include "fdbserver/TSSMappingUtil.h"
+#include "fdbserver/TSSMappingUtil.actor.h"
 #include "flow/actorcompiler.h" // This must be the last #include.
 
-// TODO should I just change back to not use KeyBackedMap at this point?
-
-/*ACTOR Future<std::map<UID, StorageServerInterface>> readTSSMapping(Database cx) {
-    state Reference<ReadYourWritesTransaction> tr = makeReference<ReadYourWritesTransaction>(cx);    
-    loop {
-        try {
-            state std::map<UID, StorageServerInterface> mapping;
-			tr->setOption(FDBTransactionOptions::ACCESS_SYSTEM_KEYS);
-            readTSSMappingRYW(tr, &mapping);
-			return mapping;
-		} catch (Error& e) {
-			wait(tr->onError(e));
-		}
-    }
-}*/
-
 ACTOR Future<Void> readTSSMappingRYW(Reference<ReadYourWritesTransaction> tr, std::map<UID, StorageServerInterface>* tssMapping) {
     KeyBackedMap<UID, UID> tssMapDB = KeyBackedMap<UID, UID>(tssMappingKeys.begin);
     state std::vector<std::pair<UID, UID>> uidMapping = wait(tssMapDB.getRange(tr, UID(), Optional<UID>(), CLIENT_KNOBS->TOO_MANY));
@@ -85,4 +69,4 @@ ACTOR Future<Void> removeTSSPairsFromCluster(Database cx, vector<std::pair<UID,
         }
     }
     return Void();
-}
+}

fdbserver/TSSMappingUtil.actor.h

@@ -0,0 +1,48 @@
+/*
+ * TSSMappingUtil.actor.h
+ *
+ * This source file is part of the FoundationDB open source project
+ *
+ * Copyright 2013-2018 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.
+ */
+
+#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(TSS_MAPPING_UTIL_SERVER_G_H)
+#define TSS_MAPPING_UTIL_SERVER_G_H
+#include "fdbserver/TSSMappingUtil.actor.g.h"
+#elif !defined(TSS_MAPPING_UTIL_SERVER_H)
+#define TSS_MAPPING_UTIL_SERVER_H
+
+#include "fdbclient/StorageServerInterface.h"
+#include "flow/actorcompiler.h" // This must be the last #include.
+
+/*
+ * Collection of utility functions for dealing with the TSS mapping
+ */
+
+// Reads the current cluster TSS mapping as part of the RYW transaction
+ACTOR Future<Void> readTSSMappingRYW(Reference<ReadYourWritesTransaction> tr, std::map<UID, StorageServerInterface>* tssMapping);
+
+// Reads the current cluster TSS mapping as part of the given Transaction
+ACTOR Future<Void> readTSSMapping(Transaction* tr, std::map<UID, StorageServerInterface>* tssMapping);
+
+// Removes the TSS pairs from the cluster
+ACTOR Future<Void> removeTSSPairsFromCluster(Database cx, vector<std::pair<UID, UID>> pairsToRemove);
+
+#include "flow/unactorcompiler.h"
+#endif

fdbserver/TSSMappingUtil.h

@@ -1,36 +0,0 @@
-/*
- * TSSMappingUtil.h
- *
- * This source file is part of the FoundationDB open source project
- *
- * Copyright 2013-2018 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 TSS_MAPPING_UTIL_SERVER_H
-#define TSS_MAPPING_UTIL_SERVER_H
-#pragma once
-
-#include "fdbclient/StorageServerInterface.h"
-
-// TODO unused
-// Future<std::map<UID, StorageServerInterface>> readTSSMapping(Database cx);
-
-Future<Void> readTSSMappingRYW(Reference<ReadYourWritesTransaction> const& tr, std::map<UID, StorageServerInterface>* const& tssMapping);
-
-Future<Void> readTSSMapping(Transaction* const& tr, std::map<UID, StorageServerInterface>* const& tssMapping);
-
-Future<Void> removeTSSPairsFromCluster(Database const& cx, vector<std::pair<UID, UID>> const& pairsToRemove);
-
-#endif

fdbserver/workloads/ConsistencyCheck.actor.cpp

@@ -32,7 +32,7 @@
 #include "fdbserver/StorageMetrics.h"
 #include "fdbserver/DataDistribution.actor.h"
 #include "fdbserver/QuietDatabase.h"
-#include "fdbserver/TSSMappingUtil.h"
+#include "fdbserver/TSSMappingUtil.actor.h"
 #include "flow/DeterministicRandom.h"
 #include "fdbclient/ManagementAPI.actor.h"
 #include "fdbclient/StorageServerInterface.h"