From 8e94207fc79c31a8f5bf047d54dc3bc5a9733a04 Mon Sep 17 00:00:00 2001
From: Youngteac Hong <susukang98@gmail.com>
Date: Mon, 2 Oct 2023 20:22:52 +0900
Subject: [PATCH] Remove lock from ActorID

---
 pkg/document/document.go                  | 14 +++++++-----
 pkg/document/time/actor_id.go             | 27 +++++------------------
 server/backend/sync/coordinator.go        |  3 ---
 server/backend/sync/memory/coordinator.go | 12 +++-------
 server/rpc/yorkie_server.go               |  3 ---
 5 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/pkg/document/document.go b/pkg/document/document.go
index c1548eb1e..d4b022094 100644
--- a/pkg/document/document.go
+++ b/pkg/document/document.go
@@ -33,8 +33,7 @@ import (
 
 var (
 	// ErrUnsupportedPayloadType is returned when the payload is unserializable to JSON.
-	ErrUnsupportedPayloadType = errors.New(
-		"the payload is an unsupported JSON type")
+	ErrUnsupportedPayloadType = errors.New("the payload is an unsupported JSON type")
 )
 
 // DocEvent represents the event that occurred in the document.
@@ -390,7 +389,7 @@ func (d *Document) Broadcast(topic string, payload any) error {
 }
 
 // SubscribeBroadcastEvent subscribes to the given topic and registers
-// an event handler
+// an event handler.
 func (d *Document) SubscribeBroadcastEvent(
 	topic string,
 	handler func(topic, publisher string, payload []byte) error,
@@ -399,7 +398,7 @@ func (d *Document) SubscribeBroadcastEvent(
 }
 
 // UnsubscribeBroadcastEvent unsubscribes to the given topic and deregisters
-// the event handler
+// the event handler.
 func (d *Document) UnsubscribeBroadcastEvent(
 	topic string,
 ) {
@@ -407,8 +406,11 @@ func (d *Document) UnsubscribeBroadcastEvent(
 }
 
 // BroadcastEventHandlers returns the registered handlers for broadcast events.
-func (d *Document) BroadcastEventHandlers() map[string](func(topic string,
-	publisher string, payload []byte) error) {
+func (d *Document) BroadcastEventHandlers() map[string]func(
+	topic string,
+	publisher string,
+	payload []byte,
+) error {
 	return d.broadcastEventHandlers
 }
 
diff --git a/pkg/document/time/actor_id.go b/pkg/document/time/actor_id.go
index 6939369a6..a2bcb61f6 100644
--- a/pkg/document/time/actor_id.go
+++ b/pkg/document/time/actor_id.go
@@ -23,7 +23,6 @@ import (
 	"errors"
 	"fmt"
 	"math"
-	"sync"
 )
 
 const actorIDSize = 12
@@ -57,13 +56,14 @@ var (
 	ErrInvalidActorID = errors.New("invalid actor id")
 )
 
-// ActorID is bytes represented by the hexadecimal string.
-// It should be generated by unique value.
+// ActorID represents the unique ID of the client. It is composed of 12 bytes.
+// It caches the string representation of ActorID to reduce the number of calls
+// to hex.EncodeToString. This causes a multi-routine problem, so it is
+// recommended to use it in a single routine or to use it after locking.
 type ActorID struct {
 	bytes [actorIDSize]byte
 
-	cachedString   string
-	cachedStringMu sync.RWMutex
+	cachedString string
 }
 
 // ActorIDFromHex returns the bytes represented by the hexadecimal string str.
@@ -106,23 +106,6 @@ func ActorIDFromBytes(bytes []byte) (*ActorID, error) {
 // String returns the hexadecimal encoding of ActorID.
 // If the receiver is nil, it would return empty string.
 func (id *ActorID) String() string {
-	id.cachedStringMu.RLock()
-	readLocked := true
-	defer func() {
-		if readLocked {
-			id.cachedStringMu.RUnlock()
-		}
-	}()
-
-	if id.cachedString != "" {
-		return id.cachedString
-	}
-
-	id.cachedStringMu.RUnlock()
-	readLocked = false
-	id.cachedStringMu.Lock()
-	defer id.cachedStringMu.Unlock()
-
 	if id.cachedString == "" {
 		id.cachedString = hex.EncodeToString(id.bytes[:])
 	}
diff --git a/server/backend/sync/coordinator.go b/server/backend/sync/coordinator.go
index deab2105c..764a942d6 100644
--- a/server/backend/sync/coordinator.go
+++ b/server/backend/sync/coordinator.go
@@ -54,9 +54,6 @@ type Coordinator interface {
 	// Publish publishes the given event.
 	Publish(ctx context.Context, publisherID *time.ActorID, event DocEvent)
 
-	// PublishToLocal publishes the given event.
-	PublishToLocal(ctx context.Context, publisherID *time.ActorID, event DocEvent)
-
 	// Members returns the members of this cluster.
 	Members() map[string]*ServerInfo
 
diff --git a/server/backend/sync/memory/coordinator.go b/server/backend/sync/memory/coordinator.go
index 57b6ca909..dfc0e1542 100644
--- a/server/backend/sync/memory/coordinator.go
+++ b/server/backend/sync/memory/coordinator.go
@@ -85,15 +85,9 @@ func (c *Coordinator) Publish(
 	publisherID *time.ActorID,
 	event sync.DocEvent,
 ) {
-	c.pubSub.Publish(ctx, publisherID, event)
-}
-
-// PublishToLocal publishes the given event.
-func (c *Coordinator) PublishToLocal(
-	ctx context.Context,
-	publisherID *time.ActorID,
-	event sync.DocEvent,
-) {
+	// NOTE(hackerwins): String() triggers the cache of ActorID to avoid
+	// race condition of concurrent access to the cache.
+	_ = event.Publisher.String()
 	c.pubSub.Publish(ctx, publisherID, event)
 }
 
diff --git a/server/rpc/yorkie_server.go b/server/rpc/yorkie_server.go
index d41faac7f..f3fa6bc67 100644
--- a/server/rpc/yorkie_server.go
+++ b/server/rpc/yorkie_server.go
@@ -584,9 +584,6 @@ func (s *yorkieServer) Broadcast(
 		return nil, err
 	}
 
-	if err != nil {
-		return nil, err
-	}
 	s.backend.Coordinator.Publish(
 		ctx,
 		clientID,