From e69c3b96642a92f7193a8ad37ffd8e0f63bbdeef Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Thu, 4 Feb 2021 18:21:21 -0500 Subject: [PATCH 1/5] use a client per mongos in txn tests --- .../SpecTestRunner/SpecTest.swift | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift index b65ef50e9..7d570efac 100644 --- a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift +++ b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift @@ -182,9 +182,9 @@ extension SpecTestFile { } } - func terminateOpenTransactions(using client: MongoSwiftSync.MongoClient) throws { - // Using the provided MongoClient, execute the killAllSessions command on either the primary or, if - // connected to a sharded cluster, all mongos servers. + func terminateOpenTransactions(using client: MongoSwiftSync.MongoClient, mongosClients: [MongoSwiftSync.MongoClient]?) throws { + // if connected to a replica set, use the provided client to execute killAllSessions on the primary. + // if connected to a sharded cluster, use the per-mongos clients to execute killAllSessions on each mongos. switch try client.topologyType() { case .single: return @@ -196,12 +196,11 @@ extension SpecTestFile { _ = try client.db("admin").runCommand(["killAllSessions": []], options: opts) } catch let commandError as MongoError.CommandError where commandError.code == 11601 {} case .sharded, .shardedReplicaSet: - for address in MongoSwiftTestCase.getHosts() { + for mongosClient in mongosClients! { do { - _ = try client.db("admin").runCommand(["killAllSessions": []], on: address) - } catch let commandError as MongoError.CommandError where commandError.code == 11601 { - continue - } + _ = try mongosClient.db("admin").runCommand(["killAllSessions": []]) + break + } catch let commandError as MongoError.CommandError where commandError.code == 11601 {} } } } @@ -221,6 +220,18 @@ extension SpecTestFile { setupClientOptions.minHeartbeatFrequencyMS = 50 setupClientOptions.heartbeatFrequencyMS = 50 let setupClient = try MongoClient.makeTestClient(connString, options: setupClientOptions) + let topologyType = try setupClient.topologyType() + + var mongosClients: [MongoClient]? = nil + switch topologyType { + case .sharded, .shardedReplicaSet: + var opts = setupClientOptions + opts.directConnection = true // connect directly to mongoses + mongosClients = try MongoSwiftTestCase.getConnectionStringPerHost() + .map { try MongoClient.makeTestClient($0.toString(), options: opts) } + default: + break + } if let requirements = self.runOn { guard try requirements.contains(where: { try setupClient.getUnmetRequirement($0) == nil }) else { @@ -235,19 +246,17 @@ extension SpecTestFile { print("Skipping test \(test.description) due to matched keyword \"\(keyword)\".") continue } - - try self.terminateOpenTransactions(using: setupClient) + try self.terminateOpenTransactions(using: setupClient, mongosClients: mongosClients) try self.populateData(using: setupClient) // Due to strange behavior in mongos, a "distinct" command needs to be run against each mongos // before the tests run to prevent certain errors from ocurring. (SERVER-39704) - if MongoSwiftTestCase.topologyType == .sharded, + if [.sharded, .shardedReplicaSet].contains(topologyType), let collName = self.collectionName, test.description.contains("distinct") { - for address in MongoSwiftTestCase.getHosts() { - _ = try setupClient.db(self.databaseName) - .runCommand(["distinct": .string(collName), "key": "_id"], on: address) + for client in mongosClients! { + _ = try client.db(self.databaseName).runCommand(["distinct": .string(collName), "key": "_id"]) } } From c832ce749afeef44c812aef09e2a9f527a57d846 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Thu, 4 Feb 2021 18:24:59 -0500 Subject: [PATCH 2/5] format/lint --- .../MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift index 7d570efac..bf053372d 100644 --- a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift +++ b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift @@ -182,7 +182,10 @@ extension SpecTestFile { } } - func terminateOpenTransactions(using client: MongoSwiftSync.MongoClient, mongosClients: [MongoSwiftSync.MongoClient]?) throws { + func terminateOpenTransactions( + using client: MongoSwiftSync.MongoClient, + mongosClients: [MongoSwiftSync.MongoClient]? + ) throws { // if connected to a replica set, use the provided client to execute killAllSessions on the primary. // if connected to a sharded cluster, use the per-mongos clients to execute killAllSessions on each mongos. switch try client.topologyType() { @@ -199,7 +202,7 @@ extension SpecTestFile { for mongosClient in mongosClients! { do { _ = try mongosClient.db("admin").runCommand(["killAllSessions": []]) - break + break } catch let commandError as MongoError.CommandError where commandError.code == 11601 {} } } @@ -222,7 +225,7 @@ extension SpecTestFile { let setupClient = try MongoClient.makeTestClient(connString, options: setupClientOptions) let topologyType = try setupClient.topologyType() - var mongosClients: [MongoClient]? = nil + var mongosClients: [MongoClient]? switch topologyType { case .sharded, .shardedReplicaSet: var opts = setupClientOptions @@ -256,7 +259,7 @@ extension SpecTestFile { test.description.contains("distinct") { for client in mongosClients! { - _ = try client.db(self.databaseName).runCommand(["distinct": .string(collName), "key": "_id"]) + _ = try client.db(self.databaseName).runCommand(["distinct": .string(collName), "key": "_id"]) } } From 122ddc949cb32a2d4bb588cf3756e93af27d28a1 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Thu, 4 Feb 2021 18:27:20 -0500 Subject: [PATCH 3/5] reorder --- .../SpecTestRunner/SpecTest.swift | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift index bf053372d..084a7f337 100644 --- a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift +++ b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift @@ -223,6 +223,14 @@ extension SpecTestFile { setupClientOptions.minHeartbeatFrequencyMS = 50 setupClientOptions.heartbeatFrequencyMS = 50 let setupClient = try MongoClient.makeTestClient(connString, options: setupClientOptions) + + if let requirements = self.runOn { + guard try requirements.contains(where: { try setupClient.getUnmetRequirement($0) == nil }) else { + fileLevelLog("Skipping tests from file \(self.name), deployment requirements not met.") + return + } + } + let topologyType = try setupClient.topologyType() var mongosClients: [MongoClient]? @@ -236,12 +244,6 @@ extension SpecTestFile { break } - if let requirements = self.runOn { - guard try requirements.contains(where: { try setupClient.getUnmetRequirement($0) == nil }) else { - fileLevelLog("Skipping tests from file \(self.name), deployment requirements not met.") - return - } - } fileLevelLog("Executing tests from file \(self.name)...") for var test in self.tests { From 64727d85c109bb520a182bc68626220978d6101c Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Thu, 4 Feb 2021 18:49:19 -0500 Subject: [PATCH 4/5] Update SpecTest.swift --- Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift index 084a7f337..99673892b 100644 --- a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift +++ b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift @@ -244,7 +244,6 @@ extension SpecTestFile { break } - fileLevelLog("Executing tests from file \(self.name)...") for var test in self.tests { if let keyword = Self.TestType.skippedTestKeywords.first(where: { test.description.contains($0) }) { From 4dc6fbe501eb1232822ae348314f1b722c6253b7 Mon Sep 17 00:00:00 2001 From: Kaitlin Mahar Date: Thu, 4 Feb 2021 22:33:10 -0500 Subject: [PATCH 5/5] add TestTopologyConfiguration.isSharded --- Sources/TestsCommon/CommonTestUtils.swift | 9 +++++++++ .../MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift | 10 ++-------- .../UnifiedTestRunner/UnifiedTestRunner.swift | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Sources/TestsCommon/CommonTestUtils.swift b/Sources/TestsCommon/CommonTestUtils.swift index 41b5f3962..ef1f3ebdb 100644 --- a/Sources/TestsCommon/CommonTestUtils.swift +++ b/Sources/TestsCommon/CommonTestUtils.swift @@ -137,11 +137,20 @@ open class MongoSwiftTestCase: XCTestCase { /// Enumerates the different topology configurations that are used throughout the tests public enum TestTopologyConfiguration: String, Decodable { + /// A sharded topology where each shard is a standalone. case sharded + /// A replica set. case replicaSet = "replicaset" + /// A sharded topology where each shard is a replica set. case shardedReplicaSet = "sharded-replicaset" + /// A standalone server. case single + /// Returns a Bool indicating whether this topology is either sharded configuration. + public var isSharded: Bool { + self == .sharded || self == .shardedReplicaSet + } + /// Determines the topologyType of a client based on the reply returned by running an isMaster command and the /// first document in the config.shards collection. public init(isMasterReply: BSONDocument, shards: [BSONDocument]) throws { diff --git a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift index 99673892b..b263f025d 100644 --- a/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift +++ b/Tests/MongoSwiftSyncTests/SpecTestRunner/SpecTest.swift @@ -234,14 +234,11 @@ extension SpecTestFile { let topologyType = try setupClient.topologyType() var mongosClients: [MongoClient]? - switch topologyType { - case .sharded, .shardedReplicaSet: + if topologyType.isSharded { var opts = setupClientOptions opts.directConnection = true // connect directly to mongoses mongosClients = try MongoSwiftTestCase.getConnectionStringPerHost() .map { try MongoClient.makeTestClient($0.toString(), options: opts) } - default: - break } fileLevelLog("Executing tests from file \(self.name)...") @@ -255,10 +252,7 @@ extension SpecTestFile { // Due to strange behavior in mongos, a "distinct" command needs to be run against each mongos // before the tests run to prevent certain errors from ocurring. (SERVER-39704) - if [.sharded, .shardedReplicaSet].contains(topologyType), - let collName = self.collectionName, - test.description.contains("distinct") - { + if topologyType.isSharded, test.description.contains("distinct"), let collName = self.collectionName { for client in mongosClients! { _ = try client.db(self.databaseName).runCommand(["distinct": .string(collName), "key": "_id"]) } diff --git a/Tests/MongoSwiftSyncTests/UnifiedTestRunner/UnifiedTestRunner.swift b/Tests/MongoSwiftSyncTests/UnifiedTestRunner/UnifiedTestRunner.swift index bed147edd..5bfdacfb3 100644 --- a/Tests/MongoSwiftSyncTests/UnifiedTestRunner/UnifiedTestRunner.swift +++ b/Tests/MongoSwiftSyncTests/UnifiedTestRunner/UnifiedTestRunner.swift @@ -152,7 +152,7 @@ struct UnifiedTestRunner { // Workaround for SERVER-39704: a test runners MUST execute a non-transactional distinct command on // each mongos server before running any test that might execute distinct within a transaction. To ease // the implementation, test runners MAY execute distinct before every test. - if self.topologyType == .sharded || self.topologyType == .shardedReplicaSet { + if self.topologyType.isSharded { let collEntities = context.entities.values.compactMap { try? $0.asCollection() } for address in MongoSwiftTestCase.getHosts() { for entity in collEntities {