From 1084b8d209186bc7e6889a2dac499bb570108c44 Mon Sep 17 00:00:00 2001 From: Yourim Cha <81357083+chacha912@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:57:06 +0900 Subject: [PATCH] Replace MaxCreatedAtMapByActor with VersionVector (#1088) Refactored causal and concurrent relationship handling in text and tree operations to use version vectors instead of MaxCreatedAtMapByActor. This change enables more precise tracking of client Lamport times during operations like deletion, improving concurrency management and providing a more robust method for determining node existence. --- api/converter/from_bytes.go | 2 +- pkg/document/change/change.go | 2 +- pkg/document/crdt/rga_tree_split.go | 46 ++++++-- pkg/document/crdt/root_test.go | 18 ++-- pkg/document/crdt/text.go | 23 +++- pkg/document/crdt/text_test.go | 10 +- pkg/document/crdt/tree.go | 100 ++++++++++++++---- pkg/document/crdt/tree_test.go | 10 +- pkg/document/json/text.go | 2 + pkg/document/json/tree.go | 5 +- pkg/document/operations/add.go | 2 +- pkg/document/operations/array_set.go | 3 +- pkg/document/operations/edit.go | 5 +- pkg/document/operations/increase.go | 2 +- pkg/document/operations/move.go | 2 +- pkg/document/operations/operation.go | 2 +- pkg/document/operations/remove.go | 2 +- pkg/document/operations/set.go | 2 +- pkg/document/operations/style.go | 4 +- pkg/document/operations/tree_edit.go | 3 +- pkg/document/operations/tree_style.go | 7 +- pkg/document/time/version_vector.go | 7 ++ test/integration/gc_test.go | 144 ++++++++++++++++++++++++++ 23 files changed, 331 insertions(+), 72 deletions(-) diff --git a/api/converter/from_bytes.go b/api/converter/from_bytes.go index 8e3e41abd..2ba4c24e7 100644 --- a/api/converter/from_bytes.go +++ b/api/converter/from_bytes.go @@ -334,7 +334,7 @@ func fromTextNode( if err != nil { return nil, err } - textNode.Remove(removedAt, time.MaxTicket) + textNode.Remove(removedAt, time.MaxTicket, time.MaxLamport) } return textNode, nil } diff --git a/pkg/document/change/change.go b/pkg/document/change/change.go index 0bdbf02ed..54cb162de 100644 --- a/pkg/document/change/change.go +++ b/pkg/document/change/change.go @@ -54,7 +54,7 @@ func New(id ID, message string, operations []operations.Operation, p *innerprese // Execute applies this change to the given JSON root. func (c *Change) Execute(root *crdt.Root, presences *innerpresence.Map) error { for _, op := range c.operations { - if err := op.Execute(root); err != nil { + if err := op.Execute(root, c.ID().versionVector); err != nil { return err } } diff --git a/pkg/document/crdt/rga_tree_split.go b/pkg/document/crdt/rga_tree_split.go index b2e9448b3..20726f71b 100644 --- a/pkg/document/crdt/rga_tree_split.go +++ b/pkg/document/crdt/rga_tree_split.go @@ -258,10 +258,18 @@ func (s *RGATreeSplitNode[V]) toTestString() string { // Remove removes this node if it created before the time of deletion are // deleted. It only marks the deleted time (tombstone). -func (s *RGATreeSplitNode[V]) Remove(removedAt *time.Ticket, maxCreatedAt *time.Ticket) bool { +func (s *RGATreeSplitNode[V]) Remove(removedAt *time.Ticket, + maxCreatedAt *time.Ticket, clientLamportAtChange int64) bool { justRemoved := s.removedAt == nil - if !s.createdAt().After(maxCreatedAt) && + var nodeExisted bool + if maxCreatedAt == nil { + nodeExisted = s.createdAt().Lamport() <= clientLamportAtChange + } else { + nodeExisted = !s.createdAt().After(maxCreatedAt) + } + + if nodeExisted && (s.removedAt == nil || removedAt.After(s.removedAt)) { s.removedAt = removedAt return justRemoved @@ -271,8 +279,16 @@ func (s *RGATreeSplitNode[V]) Remove(removedAt *time.Ticket, maxCreatedAt *time. } // canStyle checks if node is able to set style. -func (s *RGATreeSplitNode[V]) canStyle(editedAt *time.Ticket, maxCreatedAt *time.Ticket) bool { - return !s.createdAt().After(maxCreatedAt) && +func (s *RGATreeSplitNode[V]) canStyle(editedAt *time.Ticket, + maxCreatedAt *time.Ticket, clientLamportAtChange int64) bool { + var nodeExisted bool + if maxCreatedAt == nil { + nodeExisted = s.createdAt().Lamport() <= clientLamportAtChange + } else { + nodeExisted = !s.createdAt().After(maxCreatedAt) + } + + return nodeExisted && (s.removedAt == nil || editedAt.After(s.removedAt)) } @@ -451,6 +467,7 @@ func (s *RGATreeSplit[V]) edit( maxCreatedAtMapByActor map[string]*time.Ticket, content V, editedAt *time.Ticket, + versionVector time.VersionVector, ) (*RGATreeSplitNodePos, map[string]*time.Ticket, []GCPair, error) { // 01. Split nodes with from and to toLeft, toRight, err := s.findNodeWithSplit(to, editedAt) @@ -464,7 +481,7 @@ func (s *RGATreeSplit[V]) edit( // 02. delete between from and to nodesToDelete := s.findBetween(fromRight, toRight) - maxCreatedAtMap, removedNodes := s.deleteNodes(nodesToDelete, maxCreatedAtMapByActor, editedAt) + maxCreatedAtMap, removedNodes := s.deleteNodes(nodesToDelete, maxCreatedAtMapByActor, editedAt, versionVector) var caretID *RGATreeSplitNodeID if toRight == nil { @@ -506,6 +523,7 @@ func (s *RGATreeSplit[V]) deleteNodes( candidates []*RGATreeSplitNode[V], maxCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket, + versionVector time.VersionVector, ) (map[string]*time.Ticket, map[string]*RGATreeSplitNode[V]) { createdAtMapByActor := make(map[string]*time.Ticket) removedNodeMap := make(map[string]*RGATreeSplitNode[V]) @@ -523,10 +541,20 @@ func (s *RGATreeSplit[V]) deleteNodes( for _, node := range candidates { actorIDHex := node.createdAt().ActorIDHex() + actorID := node.createdAt().ActorID() var maxCreatedAt *time.Ticket - if maxCreatedAtMapByActor == nil { - maxCreatedAt = time.MaxTicket + var clientLamportAtChange int64 + if versionVector == nil && maxCreatedAtMapByActor == nil { + // Local edit - use version vector comparison + clientLamportAtChange = time.MaxLamport + } else if versionVector != nil { + lamport, ok := versionVector.Get(actorID) + if ok { + clientLamportAtChange = lamport + } else { + clientLamportAtChange = 0 + } } else { createdAt, ok := maxCreatedAtMapByActor[actorIDHex] if ok { @@ -536,7 +564,9 @@ func (s *RGATreeSplit[V]) deleteNodes( } } - if node.Remove(editedAt, maxCreatedAt) { + // TODO(chacha912): maxCreatedAt can be removed after all legacy Changes + // (without version vector) are migrated to new Changes with version vector. + if node.Remove(editedAt, maxCreatedAt, clientLamportAtChange) { maxCreatedAt := createdAtMapByActor[actorIDHex] createdAt := node.id.createdAt if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { diff --git a/pkg/document/crdt/root_test.go b/pkg/document/crdt/root_test.go index 18d499434..4311b9434 100644 --- a/pkg/document/crdt/root_test.go +++ b/pkg/document/crdt/root_test.go @@ -65,28 +65,28 @@ func TestRoot(t *testing.T) { text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket()) fromPos, toPos, _ := text.CreateRange(0, 0) - _, _, pairs, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket()) + _, _, pairs, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, "Hello World", text.String()) assert.Equal(t, 0, root.GarbageLen()) fromPos, toPos, _ = text.CreateRange(5, 10) - _, _, pairs, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket()) + _, _, pairs, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, "HelloYorkied", text.String()) assert.Equal(t, 1, root.GarbageLen()) fromPos, toPos, _ = text.CreateRange(0, 5) - _, _, pairs, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket()) + _, _, pairs, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, "Yorkied", text.String()) assert.Equal(t, 2, root.GarbageLen()) fromPos, toPos, _ = text.CreateRange(6, 7) - _, _, pairs, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket()) + _, _, pairs, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, "Yorkie", text.String()) @@ -125,7 +125,7 @@ func TestRoot(t *testing.T) { for _, tc := range steps { fromPos, toPos, _ := text.CreateRange(tc.from, tc.to) - _, _, pairs, err := text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket()) + _, _, pairs, err := text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, tc.want, text.String()) @@ -157,7 +157,7 @@ func TestRoot(t *testing.T) { for _, tc := range steps { fromPos, toPos, _ := text.CreateRange(tc.from, tc.to) - _, _, pairs, err := text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket()) + _, _, pairs, err := text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, tc.want, text.String()) @@ -176,21 +176,21 @@ func TestRoot(t *testing.T) { text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket()) fromPos, toPos, _ := text.CreateRange(0, 0) - _, _, pairs, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket()) + _, _, pairs, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal()) assert.Equal(t, 0, root.GarbageLen()) fromPos, toPos, _ = text.CreateRange(6, 11) - _, _, pairs, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket()) + _, _, pairs, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal()) assert.Equal(t, 1, root.GarbageLen()) fromPos, toPos, _ = text.CreateRange(0, 6) - _, _, pairs, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket()) + _, _, pairs, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) registerGCPairs(root, pairs) assert.Equal(t, `[{"val":"Yorkie"}]`, text.Marshal()) diff --git a/pkg/document/crdt/text.go b/pkg/document/crdt/text.go index 09f3520f3..18653e80e 100644 --- a/pkg/document/crdt/text.go +++ b/pkg/document/crdt/text.go @@ -273,6 +273,7 @@ func (t *Text) Edit( content string, attributes map[string]string, executedAt *time.Ticket, + versionVector time.VersionVector, ) (*RGATreeSplitNodePos, map[string]*time.Ticket, []GCPair, error) { val := NewTextValue(content, NewRHT()) for key, value := range attributes { @@ -285,6 +286,7 @@ func (t *Text) Edit( maxCreatedAtMapByActor, val, executedAt, + versionVector, ) } @@ -295,6 +297,7 @@ func (t *Text) Style( maxCreatedAtMapByActor map[string]*time.Ticket, attributes map[string]string, executedAt *time.Ticket, + versionVector time.VersionVector, ) (map[string]*time.Ticket, []GCPair, error) { // 01. Split nodes with from and to _, toRight, err := t.rgaTreeSplit.findNodeWithSplit(to, executedAt) @@ -313,10 +316,20 @@ func (t *Text) Style( for _, node := range nodes { actorIDHex := node.id.createdAt.ActorIDHex() + actorID := node.id.createdAt.ActorID() var maxCreatedAt *time.Ticket - if len(maxCreatedAtMapByActor) == 0 { - maxCreatedAt = time.MaxTicket + var clientLamportAtChange int64 + if versionVector == nil && maxCreatedAtMapByActor == nil { + // Local edit - use version vector comparison + clientLamportAtChange = time.MaxLamport + } else if versionVector != nil { + lamport, ok := versionVector.Get(actorID) + if ok { + clientLamportAtChange = lamport + } else { + clientLamportAtChange = 0 + } } else { createdAt, ok := maxCreatedAtMapByActor[actorIDHex] if ok { @@ -326,8 +339,10 @@ func (t *Text) Style( } } - if node.canStyle(executedAt, maxCreatedAt) { - maxCreatedAt = createdAtMapByActor[actorIDHex] + // TODO(chacha912): maxCreatedAt can be removed after all legacy Changes + // (without version vector) are migrated to new Changes with version vector. + if node.canStyle(executedAt, maxCreatedAt, clientLamportAtChange) { + maxCreatedAt := createdAtMapByActor[actorIDHex] createdAt := node.id.createdAt if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { createdAtMapByActor[actorIDHex] = createdAt diff --git a/pkg/document/crdt/text_test.go b/pkg/document/crdt/text_test.go index 474cf819b..c36f85285 100644 --- a/pkg/document/crdt/text_test.go +++ b/pkg/document/crdt/text_test.go @@ -32,12 +32,12 @@ func TestText(t *testing.T) { text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket()) fromPos, toPos, _ := text.CreateRange(0, 0) - _, _, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket()) + _, _, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal()) fromPos, toPos, _ = text.CreateRange(6, 11) - _, _, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket()) + _, _, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal()) }) @@ -70,17 +70,17 @@ func TestText(t *testing.T) { text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket()) fromPos, toPos, _ := text.CreateRange(0, 0) - _, _, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket()) + _, _, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal()) fromPos, toPos, _ = text.CreateRange(6, 11) - _, _, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket()) + _, _, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal()) fromPos, toPos, _ = text.CreateRange(0, 1) - _, _, err = text.Style(fromPos, toPos, nil, map[string]string{"b": "1"}, ctx.IssueTimeTicket()) + _, _, err = text.Style(fromPos, toPos, nil, map[string]string{"b": "1"}, ctx.IssueTimeTicket(), nil) assert.NoError(t, err) assert.Equal( t, diff --git a/pkg/document/crdt/tree.go b/pkg/document/crdt/tree.go index 42454818b..3b84d82fa 100644 --- a/pkg/document/crdt/tree.go +++ b/pkg/document/crdt/tree.go @@ -381,20 +381,40 @@ func (n *TreeNode) remove(removedAt *time.Ticket) bool { return false } -func (n *TreeNode) canDelete(removedAt *time.Ticket, maxCreatedAt *time.Ticket) bool { - if !n.id.CreatedAt.After(maxCreatedAt) && +// TODO(chacha912): maxCreatedAt can be removed after all legacy Changes +// (without version vector) are migrated to new Changes with version vector. +func (n *TreeNode) canDelete(removedAt *time.Ticket, + maxCreatedAt *time.Ticket, clientLamportAtChange int64) bool { + var nodeExisted bool + if maxCreatedAt == nil { + nodeExisted = n.id.CreatedAt.Lamport() <= clientLamportAtChange + } else { + nodeExisted = !n.id.CreatedAt.After(maxCreatedAt) + } + + if nodeExisted && (n.removedAt == nil || n.removedAt.Compare(removedAt) > 0) { return true } return false } -func (n *TreeNode) canStyle(editedAt *time.Ticket, maxCreatedAt *time.Ticket) bool { +// TODO(chacha912): maxCreatedAt can be removed after all legacy Changes +// (without version vector) are migrated to new Changes with version vector. +func (n *TreeNode) canStyle(editedAt *time.Ticket, + maxCreatedAt *time.Ticket, clientLamportAtChange int64) bool { if n.IsText() { return false } - return !n.id.CreatedAt.After(maxCreatedAt) && + var nodeExisted bool + if maxCreatedAt == nil { + nodeExisted = n.id.CreatedAt.Lamport() <= clientLamportAtChange + } else { + nodeExisted = !n.id.CreatedAt.After(maxCreatedAt) + } + + return nodeExisted && (n.removedAt == nil || editedAt.After(n.removedAt)) } @@ -669,7 +689,7 @@ func (t *Tree) EditT( return err } - _, _, err = t.Edit(fromPos, toPos, contents, splitLevel, editedAt, issueTimeTicket, nil) + _, _, err = t.Edit(fromPos, toPos, contents, splitLevel, editedAt, issueTimeTicket, nil, nil) return err } @@ -717,6 +737,7 @@ func (t *Tree) Edit( editedAt *time.Ticket, issueTimeTicket func() *time.Ticket, maxCreatedAtMapByActor map[string]*time.Ticket, + versionVector time.VersionVector, ) (map[string]*time.Ticket, []GCPair, error) { // 01. find nodes from the given range and split nodes. fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt) @@ -730,7 +751,7 @@ func (t *Tree) Edit( toBeRemoveds, toBeMovedToFromParents, maxCreatedAtMap, err := t.collectBetween( fromParent, fromLeft, toParent, toLeft, - maxCreatedAtMapByActor, editedAt, + maxCreatedAtMapByActor, editedAt, versionVector, ) if err != nil { return nil, nil, err @@ -816,6 +837,7 @@ func (t *Tree) collectBetween( fromParent *TreeNode, fromLeft *TreeNode, toParent *TreeNode, toLeft *TreeNode, maxCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket, + versionVector time.VersionVector, ) ([]*TreeNode, []*TreeNode, map[string]*time.Ticket, error) { var toBeRemoveds []*TreeNode var toBeMovedToFromParents []*TreeNode @@ -843,10 +865,20 @@ func (t *Tree) collectBetween( } actorIDHex := node.id.CreatedAt.ActorIDHex() + actorID := node.id.CreatedAt.ActorID() var maxCreatedAt *time.Ticket - if maxCreatedAtMapByActor == nil { - maxCreatedAt = time.MaxTicket + var clientLamportAtChange int64 + if versionVector == nil && maxCreatedAtMapByActor == nil { + // Local edit - use version vector comparison + clientLamportAtChange = time.MaxLamport + } else if versionVector != nil { + lamport, ok := versionVector.Get(actorID) + if ok { + clientLamportAtChange = lamport + } else { + clientLamportAtChange = 0 + } } else { createdAt, ok := maxCreatedAtMapByActor[actorIDHex] if ok { @@ -858,8 +890,9 @@ func (t *Tree) collectBetween( // NOTE(sejongk): If the node is removable or its parent is going to // be removed, then this node should be removed. - if node.canDelete(editedAt, maxCreatedAt) || slices.Contains(toBeRemoveds, node.Index.Parent.Value) { - maxCreatedAt = createdAtMapByActor[actorIDHex] + if node.canDelete(editedAt, maxCreatedAt, clientLamportAtChange) || + slices.Contains(toBeRemoveds, node.Index.Parent.Value) { + maxCreatedAt := createdAtMapByActor[actorIDHex] createdAt := node.id.CreatedAt if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { createdAtMapByActor[actorIDHex] = createdAt @@ -935,6 +968,7 @@ func (t *Tree) StyleByIndex( attributes map[string]string, editedAt *time.Ticket, maxCreatedAtMapByActor map[string]*time.Ticket, + versionVector time.VersionVector, ) (map[string]*time.Ticket, []GCPair, error) { fromPos, err := t.FindPos(start) if err != nil { @@ -946,7 +980,7 @@ func (t *Tree) StyleByIndex( return nil, nil, err } - return t.Style(fromPos, toPos, attributes, editedAt, maxCreatedAtMapByActor) + return t.Style(fromPos, toPos, attributes, editedAt, maxCreatedAtMapByActor, versionVector) } // Style applies the given attributes of the given range. @@ -955,6 +989,7 @@ func (t *Tree) Style( attrs map[string]string, editedAt *time.Ticket, maxCreatedAtMapByActor map[string]*time.Ticket, + versionVector time.VersionVector, ) (map[string]*time.Ticket, []GCPair, error) { fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt) if err != nil { @@ -970,20 +1005,31 @@ func (t *Tree) Style( if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) { node := token.Node actorIDHex := node.id.CreatedAt.ActorIDHex() + actorID := node.id.CreatedAt.ActorID() var maxCreatedAt *time.Ticket - if maxCreatedAtMapByActor == nil { - maxCreatedAt = time.MaxTicket + var clientLamportAtChange int64 + if versionVector == nil && maxCreatedAtMapByActor == nil { + // Local edit - use version vector comparison + clientLamportAtChange = time.MaxLamport + } else if versionVector != nil { + lamport, ok := versionVector.Get(actorID) + if ok { + clientLamportAtChange = lamport + } else { + clientLamportAtChange = 0 + } } else { - if createdAt, ok := maxCreatedAtMapByActor[actorIDHex]; ok { + createdAt, ok := maxCreatedAtMapByActor[actorIDHex] + if ok { maxCreatedAt = createdAt } else { maxCreatedAt = time.InitialTicket } } - if node.canStyle(editedAt, maxCreatedAt) && len(attrs) > 0 { - maxCreatedAt = createdAtMapByActor[actorIDHex] + if node.canStyle(editedAt, maxCreatedAt, clientLamportAtChange) && len(attrs) > 0 { + maxCreatedAt := createdAtMapByActor[actorIDHex] createdAt := node.id.CreatedAt if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { createdAtMapByActor[actorIDHex] = createdAt @@ -1012,6 +1058,7 @@ func (t *Tree) RemoveStyle( attrs []string, editedAt *time.Ticket, maxCreatedAtMapByActor map[string]*time.Ticket, + versionVector time.VersionVector, ) (map[string]*time.Ticket, []GCPair, error) { fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt) if err != nil { @@ -1027,20 +1074,31 @@ func (t *Tree) RemoveStyle( if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) { node := token.Node actorIDHex := node.id.CreatedAt.ActorIDHex() + actorID := node.id.CreatedAt.ActorID() var maxCreatedAt *time.Ticket - if maxCreatedAtMapByActor == nil { - maxCreatedAt = time.MaxTicket + var clientLamportAtChange int64 + if versionVector == nil && maxCreatedAtMapByActor == nil { + // Local edit - use version vector comparison + clientLamportAtChange = time.MaxLamport + } else if versionVector != nil { + lamport, ok := versionVector.Get(actorID) + if ok { + clientLamportAtChange = lamport + } else { + clientLamportAtChange = 0 + } } else { - if createdAt, ok := maxCreatedAtMapByActor[actorIDHex]; ok { + createdAt, ok := maxCreatedAtMapByActor[actorIDHex] + if ok { maxCreatedAt = createdAt } else { maxCreatedAt = time.InitialTicket } } - if node.canStyle(editedAt, maxCreatedAt) && len(attrs) > 0 { - maxCreatedAt = createdAtMapByActor[actorIDHex] + if node.canStyle(editedAt, maxCreatedAt, clientLamportAtChange) && len(attrs) > 0 { + maxCreatedAt := createdAtMapByActor[actorIDHex] createdAt := node.id.CreatedAt if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { createdAtMapByActor[actorIDHex] = createdAt diff --git a/pkg/document/crdt/tree_test.go b/pkg/document/crdt/tree_test.go index 3384fe727..56d4695a0 100644 --- a/pkg/document/crdt/tree_test.go +++ b/pkg/document/crdt/tree_test.go @@ -405,28 +405,28 @@ func TestTreeEdit(t *testing.T) { assert.Equal(t, "

ab

cd

", tree.ToXML()) // style attributes with opening tag - _, _, err = tree.StyleByIndex(0, 1, map[string]string{"weight": "bold"}, helper.TimeT(ctx), nil) + _, _, err = tree.StyleByIndex(0, 1, map[string]string{"weight": "bold"}, helper.TimeT(ctx), nil, nil) assert.NoError(t, err) assert.Equal(t, `

ab

cd

`, tree.ToXML()) // style attributes with closing tag - _, _, err = tree.StyleByIndex(3, 4, map[string]string{"color": "red"}, helper.TimeT(ctx), nil) + _, _, err = tree.StyleByIndex(3, 4, map[string]string{"color": "red"}, helper.TimeT(ctx), nil, nil) assert.NoError(t, err) assert.Equal(t, `

ab

cd

`, tree.ToXML()) // style attributes with the whole - _, _, err = tree.StyleByIndex(0, 4, map[string]string{"size": "small"}, helper.TimeT(ctx), nil) + _, _, err = tree.StyleByIndex(0, 4, map[string]string{"size": "small"}, helper.TimeT(ctx), nil, nil) assert.NoError(t, err) assert.Equal(t, `

ab

cd

`, tree.ToXML()) // 02. style attributes to elements. - _, _, err = tree.StyleByIndex(0, 5, map[string]string{"style": "italic"}, helper.TimeT(ctx), nil) + _, _, err = tree.StyleByIndex(0, 5, map[string]string{"style": "italic"}, helper.TimeT(ctx), nil, nil) assert.NoError(t, err) assert.Equal(t, `

ab

`+ `

cd

`, tree.ToXML()) // 03. Ignore styling attributes to text nodes. - _, _, err = tree.StyleByIndex(1, 3, map[string]string{"bold": "true"}, helper.TimeT(ctx), nil) + _, _, err = tree.StyleByIndex(1, 3, map[string]string{"bold": "true"}, helper.TimeT(ctx), nil, nil) assert.NoError(t, err) assert.Equal(t, `

ab

`+ `

cd

`, tree.ToXML()) diff --git a/pkg/document/json/text.go b/pkg/document/json/text.go index 2420275ea..3e51456e8 100644 --- a/pkg/document/json/text.go +++ b/pkg/document/json/text.go @@ -80,6 +80,7 @@ func (p *Text) Edit( content, attrs, ticket, + nil, ) if err != nil { panic(err) @@ -119,6 +120,7 @@ func (p *Text) Style(from, to int, attributes map[string]string) *Text { nil, attributes, ticket, + nil, ) if err != nil { panic(err) diff --git a/pkg/document/json/tree.go b/pkg/document/json/tree.go index faae3f807..65e0f147b 100644 --- a/pkg/document/json/tree.go +++ b/pkg/document/json/tree.go @@ -221,7 +221,7 @@ func (t *Tree) Style(fromIdx, toIdx int, attributes map[string]string) bool { } ticket := t.context.IssueTimeTicket() - maxCreationMapByActor, pairs, err := t.Tree.Style(fromPos, toPos, attributes, ticket, nil) + maxCreationMapByActor, pairs, err := t.Tree.Style(fromPos, toPos, attributes, ticket, nil, nil) if err != nil { panic(err) } @@ -262,7 +262,7 @@ func (t *Tree) RemoveStyle(fromIdx, toIdx int, attributesToRemove []string) bool } ticket := t.context.IssueTimeTicket() - maxCreationMapByActor, pairs, err := t.Tree.RemoveStyle(fromPos, toPos, attributesToRemove, ticket, nil) + maxCreationMapByActor, pairs, err := t.Tree.RemoveStyle(fromPos, toPos, attributesToRemove, ticket, nil, nil) if err != nil { panic(err) } @@ -354,6 +354,7 @@ func (t *Tree) edit(fromPos, toPos *crdt.TreePos, contents []*TreeNode, splitLev ticket, t.context.IssueTimeTicket, nil, + nil, ) if err != nil { panic(err) diff --git a/pkg/document/operations/add.go b/pkg/document/operations/add.go index 9ea18d5d8..de74c71bb 100644 --- a/pkg/document/operations/add.go +++ b/pkg/document/operations/add.go @@ -52,7 +52,7 @@ func NewAdd( } // Execute executes this operation on the given document(`root`). -func (o *Add) Execute(root *crdt.Root) error { +func (o *Add) Execute(root *crdt.Root, _ time.VersionVector) error { parent := root.FindByCreatedAt(o.parentCreatedAt) obj, ok := parent.(*crdt.Array) diff --git a/pkg/document/operations/array_set.go b/pkg/document/operations/array_set.go index 11842a435..aff880e9a 100644 --- a/pkg/document/operations/array_set.go +++ b/pkg/document/operations/array_set.go @@ -52,9 +52,8 @@ func NewArraySet( } // Execute executes this operation on the given document(`root`). -func (o *ArraySet) Execute(root *crdt.Root) error { +func (o *ArraySet) Execute(root *crdt.Root, _ time.VersionVector) error { parent := root.FindByCreatedAt(o.parentCreatedAt) - obj, ok := parent.(*crdt.Array) if !ok { return ErrNotApplicableDataType diff --git a/pkg/document/operations/edit.go b/pkg/document/operations/edit.go index 3f8f830f1..4b561b702 100644 --- a/pkg/document/operations/edit.go +++ b/pkg/document/operations/edit.go @@ -70,12 +70,13 @@ func NewEdit( } // Execute executes this operation on the given document(`root`). -func (e *Edit) Execute(root *crdt.Root) error { +func (e *Edit) Execute(root *crdt.Root, versionVector time.VersionVector) error { parent := root.FindByCreatedAt(e.parentCreatedAt) switch obj := parent.(type) { case *crdt.Text: - _, _, pairs, err := obj.Edit(e.from, e.to, e.maxCreatedAtMapByActor, e.content, e.attributes, e.executedAt) + _, _, pairs, err := obj.Edit(e.from, e.to, e.maxCreatedAtMapByActor, + e.content, e.attributes, e.executedAt, versionVector) if err != nil { return err } diff --git a/pkg/document/operations/increase.go b/pkg/document/operations/increase.go index 5b3d126a6..36c3c4b46 100644 --- a/pkg/document/operations/increase.go +++ b/pkg/document/operations/increase.go @@ -43,7 +43,7 @@ func NewIncrease( } // Execute executes this operation on the given document(`root`). -func (o *Increase) Execute(root *crdt.Root) error { +func (o *Increase) Execute(root *crdt.Root, _ time.VersionVector) error { parent := root.FindByCreatedAt(o.parentCreatedAt) cnt, ok := parent.(*crdt.Counter) if !ok { diff --git a/pkg/document/operations/move.go b/pkg/document/operations/move.go index 5207b8ecf..077065be2 100644 --- a/pkg/document/operations/move.go +++ b/pkg/document/operations/move.go @@ -52,7 +52,7 @@ func NewMove( } // Execute executes this operation on the given document(`root`). -func (o *Move) Execute(root *crdt.Root) error { +func (o *Move) Execute(root *crdt.Root, _ time.VersionVector) error { parent := root.FindByCreatedAt(o.parentCreatedAt) obj, ok := parent.(*crdt.Array) diff --git a/pkg/document/operations/operation.go b/pkg/document/operations/operation.go index 8047f04e7..774b79844 100644 --- a/pkg/document/operations/operation.go +++ b/pkg/document/operations/operation.go @@ -34,7 +34,7 @@ var ( // Operation represents an operation to be executed on a document. type Operation interface { // Execute executes this operation on the given document(`root`). - Execute(root *crdt.Root) error + Execute(root *crdt.Root, versionVector time.VersionVector) error // ExecutedAt returns execution time of this operation. ExecutedAt() *time.Ticket diff --git a/pkg/document/operations/remove.go b/pkg/document/operations/remove.go index 4abf3cdcc..1c5446160 100644 --- a/pkg/document/operations/remove.go +++ b/pkg/document/operations/remove.go @@ -48,7 +48,7 @@ func NewRemove( } // Execute executes this operation on the given document(`root`). -func (o *Remove) Execute(root *crdt.Root) error { +func (o *Remove) Execute(root *crdt.Root, _ time.VersionVector) error { parentElem := root.FindByCreatedAt(o.parentCreatedAt) switch parent := parentElem.(type) { diff --git a/pkg/document/operations/set.go b/pkg/document/operations/set.go index 2e2a66ce2..19f3c2ff0 100644 --- a/pkg/document/operations/set.go +++ b/pkg/document/operations/set.go @@ -53,7 +53,7 @@ func NewSet( } // Execute executes this operation on the given document(`root`). -func (o *Set) Execute(root *crdt.Root) error { +func (o *Set) Execute(root *crdt.Root, _ time.VersionVector) error { parent := root.FindByCreatedAt(o.parentCreatedAt) obj, ok := parent.(*crdt.Object) diff --git a/pkg/document/operations/style.go b/pkg/document/operations/style.go index 14e9564c8..b66edaddf 100644 --- a/pkg/document/operations/style.go +++ b/pkg/document/operations/style.go @@ -63,14 +63,14 @@ func NewStyle( } // Execute executes this operation on the given document(`root`). -func (e *Style) Execute(root *crdt.Root) error { +func (e *Style) Execute(root *crdt.Root, versionVector time.VersionVector) error { parent := root.FindByCreatedAt(e.parentCreatedAt) obj, ok := parent.(*crdt.Text) if !ok { return ErrNotApplicableDataType } - _, pairs, err := obj.Style(e.from, e.to, e.maxCreatedAtMapByActor, e.attributes, e.executedAt) + _, pairs, err := obj.Style(e.from, e.to, e.maxCreatedAtMapByActor, e.attributes, e.executedAt, versionVector) if err != nil { return err } diff --git a/pkg/document/operations/tree_edit.go b/pkg/document/operations/tree_edit.go index c81b016cf..108849660 100644 --- a/pkg/document/operations/tree_edit.go +++ b/pkg/document/operations/tree_edit.go @@ -69,7 +69,7 @@ func NewTreeEdit( } // Execute executes this operation on the given `CRDTRoot`. -func (e *TreeEdit) Execute(root *crdt.Root) error { +func (e *TreeEdit) Execute(root *crdt.Root, versionVector time.VersionVector) error { parent := root.FindByCreatedAt(e.parentCreatedAt) switch obj := parent.(type) { @@ -117,6 +117,7 @@ func (e *TreeEdit) Execute(root *crdt.Root) error { } }(), e.maxCreatedAtMapByActor, + versionVector, ) if err != nil { return err diff --git a/pkg/document/operations/tree_style.go b/pkg/document/operations/tree_style.go index d642b2171..7b5676442 100644 --- a/pkg/document/operations/tree_style.go +++ b/pkg/document/operations/tree_style.go @@ -87,7 +87,7 @@ func NewTreeStyleRemove( } // Execute executes this operation on the given `CRDTRoot`. -func (e *TreeStyle) Execute(root *crdt.Root) error { +func (e *TreeStyle) Execute(root *crdt.Root, versionVector time.VersionVector) error { parent := root.FindByCreatedAt(e.parentCreatedAt) obj, ok := parent.(*crdt.Tree) if !ok { @@ -97,12 +97,13 @@ func (e *TreeStyle) Execute(root *crdt.Root) error { var pairs []crdt.GCPair var err error if len(e.attributes) > 0 { - _, pairs, err = obj.Style(e.from, e.to, e.attributes, e.executedAt, e.maxCreatedAtMapByActor) + _, pairs, err = obj.Style(e.from, e.to, e.attributes, e.executedAt, e.maxCreatedAtMapByActor, versionVector) if err != nil { return err } } else { - _, pairs, err = obj.RemoveStyle(e.from, e.to, e.attributesToRemove, e.executedAt, e.maxCreatedAtMapByActor) + _, pairs, err = obj.RemoveStyle(e.from, e.to, e.attributesToRemove, e.executedAt, + e.maxCreatedAtMapByActor, versionVector) if err != nil { return err } diff --git a/pkg/document/time/version_vector.go b/pkg/document/time/version_vector.go index 75741a965..6b1f4e944 100644 --- a/pkg/document/time/version_vector.go +++ b/pkg/document/time/version_vector.go @@ -35,6 +35,13 @@ func NewVersionVector() VersionVector { return make(VersionVector) } +// Get gets the version of the given actor. +// Returns the version and whether the actor exists in the vector. +func (v VersionVector) Get(id *ActorID) (int64, bool) { + version, exists := v[id.bytes] + return version, exists +} + // Set sets the given actor's version to the given value. func (v VersionVector) Set(id *ActorID, i int64) { v[id.bytes] = i diff --git a/test/integration/gc_test.go b/test/integration/gc_test.go index 4fc6195e6..357dd1820 100644 --- a/test/integration/gc_test.go +++ b/test/integration/gc_test.go @@ -1303,4 +1303,148 @@ func TestGarbageCollection(t *testing.T) { assert.Equal(t, 0, d1.GarbageLen()) assert.Equal(t, 0, d2.GarbageLen()) }) + + t.Run("attach > pushpull > detach lifecycle version vector test (run gc at last client detaches document)", func(t *testing.T) { + clients := activeClients(t, 2) + c1, c2 := clients[0], clients[1] + defer deactivateAndCloseClients(t, clients) + + ctx := context.Background() + d1 := document.New(helper.TestDocKey(t)) + assert.NoError(t, c1.Attach(ctx, d1)) + // d2.vv =[c1:1], minvv =[c1:1], db.vv {c1: [c1:1]} + assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 1))) + + d2 := document.New(helper.TestDocKey(t)) + assert.NoError(t, c2.Attach(ctx, d2)) + // d2.vv =[c1:1, c2:2], minvv =[c1:0, c2:0], db.vv {c1: [c1:1], c2: [c2:1]} + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 1), versionOf(d2.ActorID(), 2))) + + err := d1.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewText("text").Edit(0, 0, "a").Edit(1, 1, "b").Edit(2, 2, "c") + return nil + }, "sets text") + // d1/vv = [c1:2] + assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 2))) + assert.NoError(t, err) + + assert.NoError(t, c1.Sync(ctx)) + // d1.vv = [c1:3, c2:1], minvv = [c1:0, c2:0], db.vv {c1: [c1:2], c2: [c2:1]} + assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 3), versionOf(d2.ActorID(), 1))) + + assert.NoError(t, c2.Sync(ctx)) + // d2.vv = [c1:2, c2:3], minvv = [c1:1, c2:0], db.vv {c1: [c1:2], c2: [c1:1, c2:2]} + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 2), versionOf(d2.ActorID(), 3))) + + err = d2.Update(func(root *json.Object, p *presence.Presence) error { + root.GetText("text").Edit(2, 2, "c") + return nil + }, "insert c") + //d2.vv = [c1:2, c2:4] + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 2), versionOf(d2.ActorID(), 4))) + assert.NoError(t, err) + + err = d1.Update(func(root *json.Object, p *presence.Presence) error { + root.GetText("text").Edit(1, 3, "") + return nil + }, "delete bc") + //d1.vv = [c1:4, c2:1] + assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 1))) + assert.NoError(t, err) + assert.Equal(t, 2, d1.GarbageLen()) + assert.Equal(t, 0, d2.GarbageLen()) + + assert.NoError(t, c1.Sync(ctx)) + assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 1))) + + assert.NoError(t, c2.Sync(ctx)) + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 5))) + + assert.Equal(t, 2, d1.GarbageLen()) + assert.Equal(t, 2, d2.GarbageLen()) + + err = d2.Update(func(root *json.Object, p *presence.Presence) error { + root.GetText("text").Edit(2, 2, "1") + return nil + }, "insert c") + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 6))) + assert.NoError(t, err) + + assert.NoError(t, c2.Sync(ctx)) + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 6))) + + assert.Equal(t, 2, d1.GarbageLen()) + assert.Equal(t, 0, d2.GarbageLen()) + + assert.NoError(t, c1.Sync(ctx)) + assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 7), versionOf(d2.ActorID(), 6))) + + // TODO(JOOHOJANG): we have to consider removing detached client's lamport from version vector + assert.NoError(t, c1.Detach(ctx, d1)) + + assert.NoError(t, c2.Sync(ctx)) + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 8), versionOf(d2.ActorID(), 9))) + assert.Equal(t, `{"text":[{"val":"a"},{"val":"c"},{"val":"1"}]}`, d2.Marshal()) + + err = d2.Update(func(root *json.Object, p *presence.Presence) error { + root.GetText("text").Edit(0, 3, "") + return nil + }, "delete all") + assert.NoError(t, err) + assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 8), versionOf(d2.ActorID(), 10))) + assert.Equal(t, `{"text":[]}`, d2.Marshal()) + + assert.Equal(t, 3, d2.GarbageLen()) + assert.NoError(t, c2.Detach(ctx, d2)) + assert.Equal(t, 0, d2.GarbageLen()) + }) + + t.Run("detached client node deletion test", func(t *testing.T) { + clients := activeClients(t, 3) + c1, c2, c3 := clients[0], clients[1], clients[2] + defer deactivateAndCloseClients(t, clients) + ctx := context.Background() + d1 := document.New(helper.TestDocKey(t)) + d2 := document.New(helper.TestDocKey(t)) + d3 := document.New(helper.TestDocKey(t)) + + assert.NoError(t, c1.Attach(ctx, d1)) + assert.NoError(t, c2.Attach(ctx, d2)) + assert.NoError(t, c3.Attach(ctx, d3)) + + err := d1.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewText("text").Edit(0, 0, "a") // a + return nil + }, "insert abc") + assert.NoError(t, err) + + assert.NoError(t, c1.Sync(ctx)) + assert.NoError(t, c2.Sync(ctx)) + assert.NoError(t, c3.Sync(ctx)) + + err = d3.Update(func(root *json.Object, p *presence.Presence) error { + root.GetText("text").Edit(0, 0, "1") // 1a + return nil + }) + assert.NoError(t, err) + + assert.NoError(t, c3.Sync(ctx)) + assert.NoError(t, c1.Sync(ctx)) + assert.NoError(t, c2.Sync(ctx)) + + assert.NoError(t, c3.Detach(ctx, d3)) + assert.NoError(t, c1.Sync(ctx)) + assert.NoError(t, c2.Sync(ctx)) + + err = d2.Update(func(root *json.Object, p *presence.Presence) error { + root.GetText("text").Edit(0, 1, "x") // xa + return nil + }, "delete 123 and insert x") + assert.NoError(t, err) + + assert.NoError(t, c2.Sync(ctx)) + assert.NoError(t, c1.Sync(ctx)) + assert.Equal(t, `{"text":[{"val":"x"},{"val":"a"}]}`, d2.Marshal()) + assert.Equal(t, `{"text":[{"val":"x"},{"val":"a"}]}`, d1.Marshal()) + }) }