Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace MaxCreatedAtMapByActor with VersionVector #1088

Merged
merged 7 commits into from
Dec 9, 2024
+331 −72
Merged

Replace MaxCreatedAtMapByActor with VersionVector #1088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c5cf34c
Replace maxCreatedAtMapByActor with change's version vector for Text.…
chacha912 Dec 5, 2024
1f3fa9d
Replace maxCreatedAtMapByActor for Text.Style
chacha912 Dec 5, 2024
553c8e3
Replace maxCreatedAtMapByActor for Tree.Edit and Tree.Style
chacha912 Dec 5, 2024
6668937
Add test for node deletion after client detach
chacha912 Dec 5, 2024
ed521ec
Merge branch 'main' of https://github.com/yorkie-team/yorkie into rep…
chacha912 Dec 8, 2024
e784fe5
Check nodeExisted using both maxCreatedAt and version vector comparison
chacha912 Dec 9, 2024
40bf57f
Remove Option pattern from Operation interface
chacha912 Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/converter/from_bytes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/document/change/change.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
46 changes: 38 additions & 8 deletions pkg/document/crdt/rga_tree_split.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
chacha912 marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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])
Expand All @@ -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 {
Expand All @@ -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) {
Expand Down
18 changes: 9 additions & 9 deletions pkg/document/crdt/root_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand Down
23 changes: 19 additions & 4 deletions pkg/document/crdt/text.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ func (t *Text) Edit(
content string,
attributes map[string]string,
executedAt *time.Ticket,
versionVector time.VersionVector,
chacha912 marked this conversation as resolved.
Show resolved Hide resolved
) (*RGATreeSplitNodePos, map[string]*time.Ticket, []GCPair, error) {
val := NewTextValue(content, NewRHT())
for key, value := range attributes {
Expand All @@ -285,6 +286,7 @@ func (t *Text) Edit(
maxCreatedAtMapByActor,
val,
executedAt,
versionVector,
)
}

Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pkg/document/crdt/text_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading