=cluster efficient Membership Gossip serialization (proto + smart UniqueNode repetition avoidance) #676

ktoso · 2020-06-16T16:55:39Z

Motivation:

The membership gossip is sent around "constantly", it should be as small as possible.
It should scale O(n) and NOT worse (like it did today) with number of nodes.

Modifications:

use protobuf all the way
encode the membership fully with unique nodes
the seen table and other fields never encode unique nodes, but refer to the ones present in membership by using their unique IDs saving a ton of repetition in the rendered data.
- this is not fully generalized but could be if we need to -- it would also help CRDTs after all perhaps.

Result:

smaller messages and less load from the gossip subsystem on the system
more stable system

ktoso · 2020-06-16T16:56:04Z

Protos/Clocks/VersionVector.proto

        ActorAddress actorAddress = 1;
        UniqueNode uniqueNode = 2;
+        uint32 uniqueNodeIdentifier = 3;
    }


this is new, we allow carrying just the node id

number ok? don't need to be string?

is this the same as ClusterMembershipSeenTableRow.uniqueNodeID? maybe use either *ID or *Identifier in all the names?

Yeah good point, I couldn't decide what to go with, I guess ID

Protos/Cluster/Membership.proto

ktoso · 2020-06-16T16:56:22Z

Protos/Cluster/Membership.proto

+    // During deserialization the fields can be resolved against the membership to obtain full UniqueNode values if necessary.
+    uint32 ownerUniqueNodeID = 2;
+    ClusterMembershipSeenTable seenTable = 3;
 }


comments explain what we do here

ktoso · 2020-06-16T16:56:40Z

Sources/DistributedActors/ActorAddress.swift

+
    public var node: Node
-    public let nid: NodeID
+    public let nid: UniqueNodeID


that rename dawned on me during this work, much more correct i guess

ktoso · 2020-06-16T16:57:01Z

Sources/DistributedActors/Clocks/Protobuf/VersionVector+Serialization.swift


+    /// Serialize using uniqueNodeIdentifier specifically (or crash);
+    /// Used in situations where an enclosing message already has the unique nodes serialized and we can save space by avoiding to serialize them again.
+    public func toCompactReplicaNodeIDProto(context: Serialization.Context) throws -> ProtoVersionVector {


this is a bit manual but good enough for now... we could generalize this

TODO: make a ticket for it

The ticket #677

ktoso · 2020-06-16T16:57:14Z

Sources/DistributedActors/Cluster/Cluster+Membership.swift

+        public func member(byUniqueNodeID nid: UniqueNode.ID) -> Cluster.Member? {
+            // TODO: make this O(1) by allowing wrapper type to equality check only on NodeID
+            self._members.first(where: { $0.key.nid == nid })?.value
+        }


TODO: make a ticket

#678 Make Membership.member(byUniqueNodeID) O(1)

ktoso · 2020-06-16T16:57:43Z

Sources/DistributedActorsTestKit/ActorSystemXCTestCase.swift

 /// Base class to handle the repetitive setUp/tearDown code involved in most ActorSystem requiring tests.
 // TODO: Document and API guarantees
-open class ActorSystemTestBase: ClusteredNodesTestBase {
+open class ActorSystemXCTestCase: ClusteredActorSystemsXCTestCase {


should have split this out into other changeset... can do tomorrow

ktoso · 2020-06-16T16:58:21Z

Tests/DistributedActorsTests/Cluster/Protobuf/Membership+SerializationTests.swift

+        let back = try system.serialization.deserialize(as: Cluster.Gossip.self, from: serialized)
+        "\(pretty: back)".shouldStartWith(prefix: "\(pretty: gossip)") // nicer human readable error
+        back.shouldEqual(gossip) // the actual sanity check
+    }


the test showcasing the new thing. The "naive" impl ends up at 8000+ bytes here, and that's pretty bad; our new impl scales much better with cluster size as well.

yim-lee

A couple of non-blocking suggestions/questions

yim-lee · 2020-06-16T17:20:05Z

Protos/Clocks/VersionVector.proto

        ActorAddress actorAddress = 1;
        UniqueNode uniqueNode = 2;
+        uint32 uniqueNodeIdentifier = 3;
    }


number ok? don't need to be string?

is this the same as ClusterMembershipSeenTableRow.uniqueNodeID? maybe use either *ID or *Identifier in all the names?

yim-lee · 2020-06-16T17:25:36Z

Sources/DistributedActors/ActorAddress.swift

 }

-public struct NodeID: Hashable {
+// TODO: move as UniqueNode.Identifier?


yim-lee · 2020-06-16T17:27:50Z

Sources/DistributedActors/Clocks/Protobuf/VersionVector+Serialization.swift

+            case .uniqueNodeID(let nid):
+                replicaVersion.replicaID.uniqueNodeIdentifier = nid.value
+            case .actorAddress:
+                throw SerializationError.unableToSerialize(hint: "Can't serialize using unique node identifier as replica id! Was: \(replicaID)")


Is the error message correct?

Suggested change

throw SerializationError.unableToSerialize(hint: "Can't serialize using unique node identifier as replica id! Was: \(replicaID)")

throw SerializationError.unableToSerialize(hint: "Can't serialize using actor address as replica id! Was: \(replicaID)")

yim-lee · 2020-06-16T17:30:25Z

Sources/DistributedActorsTestKit/ByteBuffer+Testing.swift

            return data.stringDebugDescription()
-        case .nioByteBuffer(let bufffer):
-            return bufffer.stringDebugDescription()
+        case .nioByteBuffer(let buffer):


ktoso · 2020-06-17T03:59:01Z

Could not answer in line for some reason:

uint32 uniqueNodeIdentifier = 3;

number ok? don't need to be string?

is this the same as ClusterMembershipSeenTableRow.uniqueNodeID? maybe use either *ID or *Identifier in all the names?

Yes, that's integer, the same as the ClusterMembershipSeenTableRow.uniqueNodeID now using uniqueNodeID everywhere

ktoso · 2020-06-17T04:48:09Z

Ci failure is a bit mysterious: #680 FAILED: weird build timeouts

ktoso commented Jun 16, 2020

View reviewed changes

Protos/Cluster/Membership.proto Show resolved Hide resolved

ktoso commented Jun 16, 2020

View reviewed changes

yim-lee approved these changes Jun 16, 2020

View reviewed changes

=cluster,membership compressed gossip messages by node ID in seen tables

731f373

ktoso added this to the 0.5.0 - Cluster Hardening milestone Jun 17, 2020

ktoso added ga:cluster t:cluster:all t:serialization labels Jun 17, 2020

ktoso merged commit c6c137f into apple:master Jun 17, 2020

ktoso deleted the wip-compression-wire-format-tables branch June 17, 2020 04:48

	throw SerializationError.unableToSerialize(hint: "Can't serialize using unique node identifier as replica id! Was: \(replicaID)")
	throw SerializationError.unableToSerialize(hint: "Can't serialize using actor address as replica id! Was: \(replicaID)")

=cluster efficient Membership Gossip serialization (proto + smart UniqueNode repetition avoidance) #676

=cluster efficient Membership Gossip serialization (proto + smart UniqueNode repetition avoidance) #676

Uh oh!

Conversation

ktoso commented Jun 16, 2020

Motivation:

Modifications:

Result:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yim-lee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso commented Jun 17, 2020

Uh oh!

ktoso commented Jun 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants