From 1a86a4d78b5dd80466e7de94f6eb9d55e439585c Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 10:08:51 -0800 Subject: [PATCH 1/8] Update log message https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361275121 --- Sources/ActorSingletonPlugin/ActorSingletonProxy.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift b/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift index 7675ef75d..4353c826b 100644 --- a/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift +++ b/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift @@ -160,7 +160,7 @@ internal class ActorSingletonProxy { } private func updateRef(_ context: ActorContext, _ newRef: ActorRef?) { - context.log.debug("Updating ref from [\(String(describing: self.ref))] to [\(String(describing: newRef))]") + context.log.debug("Updating ref from [\(String(describing: self.ref))] to [\(String(describing: newRef))], flushing \(self.buffer.count) messages.") self.ref = newRef // Unstash messages if we have the singleton From 2f6bde3360dae5dfe844a429c231fe9b6f87f6ed Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 10:09:57 -0800 Subject: [PATCH 2/8] Add TODO for improving handOver on PostStop https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361274076 --- Sources/ActorSingletonPlugin/ActorSingletonProxy.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift b/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift index 4353c826b..2023c83bf 100644 --- a/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift +++ b/Sources/ActorSingletonPlugin/ActorSingletonProxy.swift @@ -79,6 +79,7 @@ internal class ActorSingletonProxy { try self.forwardOrStash(context, message: message) return .same }.receiveSpecificSignal(Signals.PostStop.self) { context, _ in + // TODO: perhaps we can figure out where `to` is next and hand over gracefully? try self.handOver(context, to: nil) return .same } From d323e8e6112d86bf653b4d0255f2f67fec22db05 Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 10:11:59 -0800 Subject: [PATCH 3/8] Don't set settings.cluster.node.host https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361272366 --- .../ActorSingletonPluginTests.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift index a1e2d0b00..9e81ce0df 100644 --- a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift +++ b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift @@ -60,7 +60,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { let first = self.setUpNode("first") { settings in settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.host = "127.0.0.1" settings.cluster.node.port = 7111 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -71,7 +70,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { let second = self.setUpNode("second") { settings in settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.host = "127.0.0.1" settings.cluster.node.port = 8222 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -82,7 +80,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { let third = self.setUpNode("third") { settings in settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.host = "127.0.0.1" settings.cluster.node.port = 9333 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -92,8 +89,7 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { } let fourth = self.setUpNode("fourth") { settings in settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - - settings.cluster.node.host = "127.0.0.1" + settings.cluster.node.port = 7444 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) From 782a2cb5586e02523e843d3eae8a6614d6ffb220 Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 10:13:50 -0800 Subject: [PATCH 4/8] Remove defer shutdown of setUpNode https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361271899 --- .../ActorSingletonPluginTests.swift | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift index 9e81ce0df..e954e8dab 100644 --- a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift +++ b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift @@ -89,7 +89,7 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { } let fourth = self.setUpNode("fourth") { settings in settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - + settings.cluster.node.port = 7444 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -98,14 +98,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { settings.serialization.registerCodable(for: GreeterSingleton.Message.self, underId: 10001) } - defer { - first.shutdown().wait() - second.shutdown().wait() - third.shutdown().wait() - fourth.shutdown().wait() - // self.logCaptureHandler.printLogs() - } - first.cluster.join(node: second.cluster.node.node) third.cluster.join(node: second.cluster.node.node) From b6e1ece3513e49e0e7b2232a0b3bffc345caf05b Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 10:17:06 -0800 Subject: [PATCH 5/8] Add TODO for #344 https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361270647 --- Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift index e954e8dab..fc428d4f9 100644 --- a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift +++ b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift @@ -48,6 +48,8 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { // singleton.actor let actor = try system.singleton.actor(name: "\(GreeterSingleton.name)-other", GreeterSingleton.self) + // TODO: https://github.com/apple/swift-distributed-actors/issues/344 + // let string = try probe.expectReply(actor.greet(name: "Charlie", _replyTo: replyProbe.ref)) actor.ref.tell(.greet(name: "Charlie", _replyTo: replyProbe.ref)) try replyProbe.expectMessage("Hi Charlie!") From 910c98996ae240d290f49cf3e01017f7f848f29f Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 10:20:30 -0800 Subject: [PATCH 6/8] No need to do manual log capture https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361271428 --- .../ActorSingletonPluginTests.swift | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift index fc428d4f9..93087cc43 100644 --- a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift +++ b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift @@ -18,16 +18,6 @@ import DistributedActorsTestKit import XCTest final class ActorSingletonPluginTests: ClusteredNodesTestBase { - var logCaptureHandler: LogCapture! - - override func setUp() { - self.logCaptureHandler = LogCapture() - } - - override func tearDown() { - self.logCaptureHandler.printIfFailed(self.testRun) - } - func test_nonCluster() throws { // Singleton should work just fine without clustering let system = ActorSystem("test") { settings in @@ -60,8 +50,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { singletonSettings.allocationStrategy = .leadership let first = self.setUpNode("first") { settings in - settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.port = 7111 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -70,8 +58,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { settings.serialization.registerCodable(for: GreeterSingleton.Message.self, underId: 10001) } let second = self.setUpNode("second") { settings in - settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.port = 8222 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -80,8 +66,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { settings.serialization.registerCodable(for: GreeterSingleton.Message.self, underId: 10001) } let third = self.setUpNode("third") { settings in - settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.port = 9333 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) @@ -90,8 +74,6 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { settings.serialization.registerCodable(for: GreeterSingleton.Message.self, underId: 10001) } let fourth = self.setUpNode("fourth") { settings in - settings.overrideLogger = self.logCaptureHandler.makeLogger(label: settings.cluster.node.systemName) - settings.cluster.node.port = 7444 settings.cluster.autoLeaderElection = .lowestAddress(minNumberOfMembers: 3) From 4420b14b009fb7b1b1aaca9967725aa9da4a5a12 Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 11:32:51 -0800 Subject: [PATCH 7/8] Fix race? https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361272499 Maybe https://github.com/apple/swift-distributed-actors/issues/346 --- .../Cluster/ClusteredNodesTestBase.swift | 21 +++++++++++++++++++ .../ActorSingletonPluginTests.swift | 10 ++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/Sources/DistributedActorsTestKit/Cluster/ClusteredNodesTestBase.swift b/Sources/DistributedActorsTestKit/Cluster/ClusteredNodesTestBase.swift index 151592b74..e6ef2af24 100644 --- a/Sources/DistributedActorsTestKit/Cluster/ClusteredNodesTestBase.swift +++ b/Sources/DistributedActorsTestKit/Cluster/ClusteredNodesTestBase.swift @@ -278,6 +278,27 @@ extension ClusteredNodesTestBase { throw testKit.error("Expected \(reflecting: foundMember.node) on \(reflecting: system.cluster.node) to be seen as: [\(expectedStatus)], but was [\(foundMember.status)]") } } + + /// Asserts the given node is the leader. + /// + /// An error is thrown but NOT failing the test; use in pair with `testKit.eventually` to achieve the expected behavior. + public func assertLeaderNode( + on system: ActorSystem, is expectedNode: UniqueNode?, + file: StaticString = #file, line: UInt = #line + ) throws { + let testKit = self.testKit(system) + let p = testKit.spawnTestProbe(expecting: Membership.self) + defer { + p.stop() + } + system.cluster.ref.tell(.query(.currentMembership(p.ref))) + + let membership = try p.expectMessage() + let leaderNode = membership.leader?.node + if leaderNode != expectedNode { + throw testKit.error("Expected \(reflecting: expectedNode) to be leader node on \(reflecting: system.cluster.node) but was [\(reflecting: leaderNode)]") + } + } } // ==== ---------------------------------------------------------------------------------------------------------------- diff --git a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift index 93087cc43..a18d0cf1d 100644 --- a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift +++ b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift @@ -107,7 +107,15 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { // Take down the leader first.cluster.down(node: first.cluster.node.node) - try self.assertMemberStatus(on: first, node: first.cluster.node, is: .down) + // Make sure that `second` and `third` see `first` as down and become leader-less + try self.testKit(second).eventually(within: .seconds(10)) { + try self.assertMemberStatus(on: second, node: first.cluster.node, is: .down) + try self.assertLeaderNode(on: second, is: nil) + } + try self.testKit(third).eventually(within: .seconds(10)) { + try self.assertMemberStatus(on: third, node: first.cluster.node, is: .down) + try self.assertLeaderNode(on: third, is: nil) + } // No leader so singleton is not available, messages sent should be stashed ref2.tell(.greet(name: "Charlie-2", _replyTo: replyProbe2.ref)) From 2e0af0c1e7249e4cd3b0d93a280b7581ab79c742 Mon Sep 17 00:00:00 2001 From: Yim Lee Date: Wed, 25 Dec 2019 11:39:12 -0800 Subject: [PATCH 8/8] Use self.testKit instead of ActorTestKit https://github.com/apple/swift-distributed-actors/pull/311#discussion_r361271999 --- .../ActorSingletonPluginTests.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift index a18d0cf1d..216b863eb 100644 --- a/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift +++ b/Tests/ActorSingletonPluginTests/ActorSingletonPluginTests.swift @@ -87,15 +87,15 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { try self.ensureNodes(.up, within: .seconds(10), systems: first, second, third) - let replyProbe1 = ActorTestKit(first).spawnTestProbe(expecting: String.self) + let replyProbe1 = self.testKit(first).spawnTestProbe(expecting: String.self) let ref1 = try first.singleton.ref(name: GreeterSingleton.name, of: GreeterSingleton.Message.self) ref1.tell(.greet(name: "Charlie", _replyTo: replyProbe1.ref)) - let replyProbe2 = ActorTestKit(second).spawnTestProbe(expecting: String.self) + let replyProbe2 = self.testKit(second).spawnTestProbe(expecting: String.self) let ref2 = try second.singleton.ref(name: GreeterSingleton.name, of: GreeterSingleton.Message.self) ref2.tell(.greet(name: "Charlie", _replyTo: replyProbe2.ref)) - let replyProbe3 = ActorTestKit(third).spawnTestProbe(expecting: String.self) + let replyProbe3 = self.testKit(third).spawnTestProbe(expecting: String.self) let ref3 = try third.singleton.ref(name: GreeterSingleton.name, of: GreeterSingleton.Message.self) ref3.tell(.greet(name: "Charlie", _replyTo: replyProbe3.ref)) @@ -119,7 +119,7 @@ final class ActorSingletonPluginTests: ClusteredNodesTestBase { // No leader so singleton is not available, messages sent should be stashed ref2.tell(.greet(name: "Charlie-2", _replyTo: replyProbe2.ref)) - ref3.tell(.greet(name: "Charlie-3", _replyTo: replyProbe2.ref)) + ref3.tell(.greet(name: "Charlie-3", _replyTo: replyProbe3.ref)) // `fourth` will become the new leader and singleton fourth.cluster.join(node: second.cluster.node.node)