From 6601835ba1c9b9b785ad1579409711a491ec3feb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kuba=20Podg=C3=B3rski?= <kuba--@users.noreply.github.com>
Date: Wed, 3 Feb 2021 19:37:54 +0100
Subject: [PATCH] Remove public mutex from Node

---
 cluster.go              | 26 ++++++++------------------
 encoding/proto/proto.go |  2 +-
 server.go               |  7 -------
 topology/node.go        |  9 ---------
 4 files changed, 9 insertions(+), 35 deletions(-)

diff --git a/cluster.go b/cluster.go
index 0e4ffb356..c6b0c75e5 100644
--- a/cluster.go
+++ b/cluster.go
@@ -353,25 +353,21 @@ func (c *cluster) addNodeBasicSorted(node *topology.Node) bool {
 	n := c.unprotectedNodeByID(node.ID)
 
 	if n != nil {
-		// prevent race on node.URI read against http/client.go:1929
-		n.Mu.Lock()
-		defer n.Mu.Unlock()
-
+		nn := &topology.Node{
+			ID:        node.ID,
+			URI:       node.URI,
+			GRPCURI:   node.GRPCURI,
+			IsPrimary: node.IsPrimary,
+			State:     node.State,
+		}
 		if n.State != node.State || n.IsPrimary != node.IsPrimary || n.URI != node.URI {
-			n.State = node.State
-			n.IsPrimary = node.IsPrimary
-			n.URI = node.URI
-			n.GRPCURI = node.GRPCURI
+			*n = *nn
 			return true
 		}
 		return false
 	}
 
 	c.noder.AppendNode(node)
-
-	// All hosts must be merged in the same order on all nodes in the cluster.
-	// sort.Sort(topology.ByID(c.nodes)) // TODO: this should no longer apply
-
 	return true
 }
 
@@ -1859,12 +1855,6 @@ func (c *cluster) ReceiveEvent(e *NodeEvent) (err error) {
 	}
 	switch e.Event {
 	case NodeJoin:
-		e.Node.Mu.Lock()
-		c.Node.Mu.Lock()
-		c.logger.Debugf("nodeJoin of %s on %s", e.Node.URI, c.Node.URI)
-		c.Node.Mu.Unlock()
-		e.Node.Mu.Unlock()
-
 		// Ignore the event if this is not the coordinator.
 		if !c.isCoordinator() {
 			return nil
diff --git a/encoding/proto/proto.go b/encoding/proto/proto.go
index 35c6565fa..99e7e84b5 100644
--- a/encoding/proto/proto.go
+++ b/encoding/proto/proto.go
@@ -671,7 +671,7 @@ func (s Serializer) encodeNodes(a []*topology.Node) []*internal.Node {
 
 // s.encodeNode converts a Node into its internal representation.
 func (s Serializer) encodeNode(m *topology.Node) *internal.Node {
-	n := m.ProtectedClone()
+	n := m.Clone()
 	return &internal.Node{
 		ID:      n.ID,
 		URI:     s.encodeURI(n.URI),
diff --git a/server.go b/server.go
index 9442c93a5..dbd66bf19 100644
--- a/server.go
+++ b/server.go
@@ -940,11 +940,7 @@ func (s *Server) SendSync(m Message) error {
 
 	for _, node := range s.cluster.Nodes() {
 		node := node
-
-		// prevent race against cluster.addNodeBasicSorted() in cluster.go
-		node.Mu.Lock()
 		uri := node.URI // URI is a struct value
-		node.Mu.Unlock()
 
 		// Don't forward the message to ourselves.
 		if s.uri == uri {
@@ -972,10 +968,7 @@ func (s *Server) SendTo(node *topology.Node, m Message) error {
 	}
 	msg = append([]byte{getMessageType(m)}, msg...)
 
-	// prevent race against cluster.addNodeBasicSorted() in cluster.go
-	node.Mu.Lock()
 	uri := node.URI // URI is a struct value
-	node.Mu.Unlock()
 
 	return s.defaultClient.SendMessage(context.Background(), &uri, msg)
 }
diff --git a/topology/node.go b/topology/node.go
index 3cdcd829b..c691c18a3 100644
--- a/topology/node.go
+++ b/topology/node.go
@@ -16,15 +16,12 @@ package topology
 
 import (
 	"fmt"
-	"sync"
 
 	"github.com/pilosa/pilosa/v2/net"
 )
 
 // Node represents a node in the cluster.
 type Node struct {
-	Mu sync.Mutex `json:"-"` // TODO: we really need to get rid of this
-
 	ID        string  `json:"id"`
 	URI       net.URI `json:"uri"`
 	GRPCURI   net.URI `json:"grpc-uri"`
@@ -32,12 +29,6 @@ type Node struct {
 	State     string  `json:"state"`
 }
 
-func (n *Node) ProtectedClone() *Node {
-	n.Mu.Lock()
-	defer n.Mu.Unlock()
-	return n.Clone()
-}
-
 func (n *Node) Clone() *Node {
 	if n == nil {
 		return nil