Avoid colocating remote log routers with satellite TLogs

fdbserver/clustercontroller/ClusterController.actor.cpp

@@ -3566,6 +3566,56 @@ TEST_CASE("/fdbserver/clustercontroller/recoverForExcludedOldTLogLocality") {
 	return Void();
 }
 
+TEST_CASE("/fdbserver/clustercontroller/avoidSatelliteTLogRouterColocation") {
+	ClusterControllerData data(ClusterControllerFullInterface(),
+	                           LocalityData(),
+	                           ServerCoordinators(Reference<IClusterConnectionRecord>(
+	                               new ClusterConnectionMemoryRecord(ClusterConnectionString()))),
+	                           makeReference<AsyncVar<Optional<UID>>>());
+	data.goodRemoteRecruitmentTime = Void();
+
+	DatabaseConfiguration configuration;
+	configuration.tLogReplicationFactor = 1;
+	configuration.tLogPolicy = makeReference<PolicyAcross>(1, "zoneid", makeReference<PolicyOne>());
+	configuration.remoteTLogReplicationFactor = 1;
+	configuration.remoteDesiredTLogCount = 1;
+	configuration.remoteTLogPolicy = makeReference<PolicyAcross>(1, "zoneid", makeReference<PolicyOne>());
+
+	const Key remoteDcId = "remote"_sr;
+	auto addWorker = [&](std::string const& processId, ProcessClass::ClassType classType) {
+		LocalityData locality;
+		locality.set(LocalityData::keyProcessId, Standalone<StringRef>(processId));
+		locality.set(LocalityData::keyZoneId, Standalone<StringRef>(processId));
+		locality.set(LocalityData::keyDcId, remoteDcId);
+		WorkerInterface worker(locality);
+		worker.initEndpoints();
+		auto& workerInfo = data.id_worker[locality.processId()];
+		workerInfo.details.interf = worker;
+		workerInfo.details.processClass = ProcessClass(classType, ProcessClass::CommandLineSource);
+		workerInfo.details.recoveredDiskFiles = true;
+		return worker.id();
+	};
+
+	const UID satelliteA = addWorker("satellite-a", ProcessClass::UnsetClass);
+	const UID satelliteB = addWorker("satellite-b", ProcessClass::UnsetClass);
+	const UID remoteLog = addWorker("remote-log", ProcessClass::LogClass);
+	addWorker("router-a", ProcessClass::UnsetClass);
+	addWorker("router-b", ProcessClass::UnsetClass);
+	const std::vector<UID> satelliteWorkers{ satelliteA, satelliteB };
+
+	for (int attempt = 0; attempt < 16; ++attempt) {
+		RecruitRemoteFromConfigurationRequest req(configuration, remoteDcId, 2, satelliteWorkers);
+		auto result = data.findRemoteWorkersForConfiguration(req);
+		ASSERT_EQ(result.remoteTLogs.size(), 1);
+		ASSERT(result.remoteTLogs[0].id() == remoteLog);
+		ASSERT_EQ(result.logRouters.size(), 2);
+		for (const auto& router : result.logRouters) {
+			ASSERT(std::find(satelliteWorkers.begin(), satelliteWorkers.end(), router.id()) == satelliteWorkers.end());
+		}
+	}
+	return Void();
+}
+
 // Tests `ClusterControllerData::updateWorkerHealth()` can update `ClusterControllerData::workerHealth`
 // based on `UpdateWorkerHealth` request correctly.
 TEST_CASE("/fdbserver/clustercontroller/updateWorkerHealth") {

fdbserver/clustercontroller/ClusterController.h

@@ -1674,6 +1674,14 @@ public:
 		std::map<Optional<Standalone<StringRef>>, int> id_used;
 
 		updateKnownIds(&id_used);
+		// Primary and satellite TLogs can share the remote DC. Account for their workers when placing log routers so
+		// recovery makes the same colocation choice that betterMasterExists() projects.
+		for (const auto& [processId, worker] : id_worker) {
+			if (std::find(req.exclusionWorkerIds.begin(), req.exclusionWorkerIds.end(), worker.details.interf.id()) !=
+			    req.exclusionWorkerIds.end()) {
+				id_used[processId]++;
+			}
+		}
 
 		if (req.dbgId.present()) {
 			TraceEvent(SevDebug, "FindRemoteWorkersForConf", req.dbgId.get())