apple / swift-distributed-actors

Peer-to-peer cluster implementation for Swift Distributed Actors
https://apple.github.io/swift-distributed-actors/
Apache License 2.0
591 stars 55 forks source link

When merging gossips, also potentially apply/emit reachability change. #413

Open ktoso opened 4 years ago

ktoso commented 4 years ago

We should consider this:

I would like to discuss this one some more, rather than just do what "we're used to" -- as our more smart failure detector does the spreading the info, so perhaps we should NOT merge this information on the high-level?

ktoso commented 4 years ago

Today:

we do NOT merge unreachability in high level, we ignore it and rely on local FD to detect these states.

ktoso commented 4 years ago

I wrote some tests but didn't include them as it's a bigger conceptual change (or no change):


    func test_mergeForward_reachability_shouldBeTakenFromMoreRecentIncomingObservation_sameStatus() {
        var membership = Cluster.Membership.parse(
            """
            A.up B.up
            """, nodes: self.allNodes
        )

        var ahead = Cluster.Membership.parse(
            """
            A.up B.up 
            """, nodes: self.allNodes
        )
        ahead.mark(self.nodeA, reachability: .unreachable).shouldNotBeNil()

        let changes = membership.mergeFrom(incoming: ahead, myself: nil)

        changes.count.shouldEqual(0)
        changes.shouldEqual([
            // TODO we'd also want a Reachability change IFF we decide to merge ahead reachability status as well
            // no change to members, just reachability should change
        ])

        membership.shouldEqual(membership)
        membership.uniqueMember(self.nodeA)?.reachability.shouldEqual(.unreachable)
    }

    func test_mergeForward_reachability_shouldBeTakenFromMoreRecentIncomingObservation_diffStatus() {
        var membership = Cluster.Membership.parse(
            """
            A.joining B.up
            """, nodes: self.allNodes
        )

        var ahead = Cluster.Membership.parse(
            """
            A.up B.up 
            """, nodes: self.allNodes
        )
        ahead.mark(self.nodeA, reachability: .unreachable).shouldNotBeNil()

        let changes = membership.mergeFrom(incoming: ahead, myself: nil)

        // TODO: member should have right reachability and it'd be a change (!!!!)
        changes.count.shouldEqual(1)
        changes.shouldEqual([
            // TODO we'd also want a Reachability change IFF we decide to merge ahead reachability status as well
            Cluster.MembershipChange(node: self.nodeA, fromStatus: .joining, toStatus: .up)
        ])

        membership.uniqueMember(self.nodeA)!.reachability.shouldEqual(.unreachable)
        membership.shouldEqual(membership)
    }