Skip to content

Commit

Permalink
Check nodeExisted using both maxCreatedAt and version vector comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
@chacha912
chacha912 committed Dec 9, 2024
1 parent ed521ec commit e784fe5
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 40 deletions.
2 changes: 1 addition & 1 deletion api/converter/from_bytes.go
View file
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.MaxLamport)
textNode.Remove(removedAt, time.MaxTicket, time.MaxLamport)
}
return textNode, nil
}
Expand Down
48 changes: 36 additions & 12 deletions pkg/document/crdt/rga_tree_split.go
View file
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, clientLamportAtChange int64) bool {
func (s *RGATreeSplitNode[V]) Remove(removedAt *time.Ticket,
maxCreatedAt *time.Ticket, clientLamportAtChange int64) bool {
justRemoved := s.removedAt == nil

if (s.createdAt().Lamport() <= clientLamportAtChange) &&
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, clientLamportAtChan
}

// canStyle checks if node is able to set style.
func (s *RGATreeSplitNode[V]) canStyle(editedAt *time.Ticket, clientLamportAtChange int64) bool {
return (s.createdAt().Lamport() <= clientLamportAtChange) &&
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))
}

Expand Down Expand Up @@ -505,7 +521,7 @@ func (s *RGATreeSplit[V]) findBetween(from, to *RGATreeSplitNode[V]) []*RGATreeS

func (s *RGATreeSplit[V]) deleteNodes(
candidates []*RGATreeSplitNode[V],
_ map[string]*time.Ticket,
maxCreatedAtMapByActor map[string]*time.Ticket,
editedAt *time.Ticket,
versionVector time.VersionVector,
) (map[string]*time.Ticket, map[string]*RGATreeSplitNode[V]) {
Expand All @@ -524,25 +540,33 @@ func (s *RGATreeSplit[V]) deleteNodes(
nodesToKeep = append(nodesToKeep, leftEdge)

for _, node := range candidates {
actorID := node.createdAt().ActorID()
actorIDHex := node.createdAt().ActorIDHex()
actorID := node.createdAt().ActorID()

var maxCreatedAt *time.Ticket
var clientLamportAtChange int64
if versionVector == nil {
if versionVector == nil && maxCreatedAtMapByActor == nil {
// Local edit - use version vector comparison
clientLamportAtChange = time.MaxLamport
} else {
} else if versionVector != nil {
lamport, ok := versionVector.Get(actorID)
if ok {
clientLamportAtChange = lamport
} else {
clientLamportAtChange = 0
}
} else {
createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
if ok {
maxCreatedAt = createdAt
} else {
maxCreatedAt = time.InitialTicket
}
}

// TODO(chacha912): We should migrate db to add maxCreatedAt to change vv for existing changes.
// Since we've modified all changes to use vv instead of maxCreatedAt (assuming we've tested this),
// we need to verify this migration with tests.
if node.Remove(editedAt, clientLamportAtChange) {
// 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
22 changes: 16 additions & 6 deletions pkg/document/crdt/text.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func (t *Text) Edit(
func (t *Text) Style(
from,
to *RGATreeSplitNodePos,
_ map[string]*time.Ticket,
maxCreatedAtMapByActor map[string]*time.Ticket,
attributes map[string]string,
executedAt *time.Ticket,
versionVector time.VersionVector,
Expand All @@ -315,23 +315,33 @@ func (t *Text) Style(
var toBeStyled []*RGATreeSplitNode[*TextValue]

for _, node := range nodes {
actorID := node.id.createdAt.ActorID()
actorIDHex := node.id.createdAt.ActorIDHex()
actorID := node.id.createdAt.ActorID()

var maxCreatedAt *time.Ticket
var clientLamportAtChange int64
if versionVector == nil {
if versionVector == nil && maxCreatedAtMapByActor == nil {
// Local edit - use version vector comparison
clientLamportAtChange = time.MaxLamport
} else {
} else if versionVector != nil {
lamport, ok := versionVector.Get(actorID)
if ok {
clientLamportAtChange = lamport
} else {
clientLamportAtChange = 0
}
} else {
createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
if ok {
maxCreatedAt = createdAt
} else {
maxCreatedAt = time.InitialTicket
}
}

// TODO(chacha912): We should migrate db to add maxCreatedAt to change vv for existing changes.
if node.canStyle(executedAt, clientLamportAtChange) {
// 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) {
Expand Down
88 changes: 67 additions & 21 deletions pkg/document/crdt/tree.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,22 +381,40 @@ func (n *TreeNode) remove(removedAt *time.Ticket) bool {
return false
}

// TODO(chacha912): We should migrate db to add maxCreatedAt to change vv for existing changes.
func (n *TreeNode) canDelete(removedAt *time.Ticket, clientLamportAtChange int64) bool {
if (n.id.CreatedAt.Lamport() <= clientLamportAtChange) &&
// 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
}

// TODO(chacha912): We should migrate db to add maxCreatedAt to change vv for existing changes.
func (n *TreeNode) canStyle(editedAt *time.Ticket, clientLamportAtChange int64) 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.Lamport() <= clientLamportAtChange) &&
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))
}

Expand Down Expand Up @@ -818,7 +836,7 @@ func (t *Tree) Edit(
func (t *Tree) collectBetween(
fromParent *TreeNode, fromLeft *TreeNode,
toParent *TreeNode, toLeft *TreeNode,
_ map[string]*time.Ticket, editedAt *time.Ticket,
maxCreatedAtMapByActor map[string]*time.Ticket, editedAt *time.Ticket,
versionVector time.VersionVector,
) ([]*TreeNode, []*TreeNode, map[string]*time.Ticket, error) {
var toBeRemoveds []*TreeNode
Expand Down Expand Up @@ -846,24 +864,34 @@ func (t *Tree) collectBetween(
}
}

actorID := node.id.CreatedAt.ActorID()
actorIDHex := node.id.CreatedAt.ActorIDHex()
actorID := node.id.CreatedAt.ActorID()

var maxCreatedAt *time.Ticket
var clientLamportAtChange int64
if versionVector == nil {
if versionVector == nil && maxCreatedAtMapByActor == nil {
// Local edit - use version vector comparison
clientLamportAtChange = time.MaxLamport
} else {
} else if versionVector != nil {
lamport, ok := versionVector.Get(actorID)
if ok {
clientLamportAtChange = lamport
} else {
clientLamportAtChange = 0
}
} else {
createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
if ok {
maxCreatedAt = createdAt
} else {
maxCreatedAt = time.InitialTicket
}
}

// 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, clientLamportAtChange) || slices.Contains(toBeRemoveds, node.Index.Parent.Value) {
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) {
Expand Down Expand Up @@ -960,7 +988,7 @@ func (t *Tree) Style(
from, to *TreePos,
attrs map[string]string,
editedAt *time.Ticket,
_ map[string]*time.Ticket,
maxCreatedAtMapByActor map[string]*time.Ticket,
versionVector time.VersionVector,
) (map[string]*time.Ticket, []GCPair, error) {
fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt)
Expand All @@ -976,22 +1004,31 @@ func (t *Tree) Style(
createdAtMapByActor := make(map[string]*time.Ticket)
if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) {
node := token.Node
actorID := node.id.CreatedAt.ActorID()
actorIDHex := node.id.CreatedAt.ActorIDHex()
actorID := node.id.CreatedAt.ActorID()

var maxCreatedAt *time.Ticket
var clientLamportAtChange int64
if versionVector == nil {
if versionVector == nil && maxCreatedAtMapByActor == nil {
// Local edit - use version vector comparison
clientLamportAtChange = time.MaxLamport
} else {
} else if versionVector != nil {
lamport, ok := versionVector.Get(actorID)
if ok {
clientLamportAtChange = lamport
} else {
clientLamportAtChange = 0
}
} else {
createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
if ok {
maxCreatedAt = createdAt
} else {
maxCreatedAt = time.InitialTicket
}
}

if node.canStyle(editedAt, clientLamportAtChange) && len(attrs) > 0 {
if node.canStyle(editedAt, maxCreatedAt, clientLamportAtChange) && len(attrs) > 0 {
maxCreatedAt := createdAtMapByActor[actorIDHex]
createdAt := node.id.CreatedAt
if maxCreatedAt == nil || createdAt.After(maxCreatedAt) {
Expand Down Expand Up @@ -1020,7 +1057,7 @@ func (t *Tree) RemoveStyle(
to *TreePos,
attrs []string,
editedAt *time.Ticket,
_ map[string]*time.Ticket,
maxCreatedAtMapByActor map[string]*time.Ticket,
versionVector time.VersionVector,
) (map[string]*time.Ticket, []GCPair, error) {
fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt)
Expand All @@ -1036,22 +1073,31 @@ func (t *Tree) RemoveStyle(
createdAtMapByActor := make(map[string]*time.Ticket)
if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) {
node := token.Node
actorID := node.id.CreatedAt.ActorID()
actorIDHex := node.id.CreatedAt.ActorIDHex()
actorID := node.id.CreatedAt.ActorID()

var maxCreatedAt *time.Ticket
var clientLamportAtChange int64
if versionVector == nil {
if versionVector == nil && maxCreatedAtMapByActor == nil {
// Local edit - use version vector comparison
clientLamportAtChange = time.MaxLamport
} else {
} else if versionVector != nil {
lamport, ok := versionVector.Get(actorID)
if ok {
clientLamportAtChange = lamport
} else {
clientLamportAtChange = 0
}
} else {
createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
if ok {
maxCreatedAt = createdAt
} else {
maxCreatedAt = time.InitialTicket
}
}

if node.canStyle(editedAt, clientLamportAtChange) && len(attrs) > 0 {
if node.canStyle(editedAt, maxCreatedAt, clientLamportAtChange) && len(attrs) > 0 {
maxCreatedAt := createdAtMapByActor[actorIDHex]
createdAt := node.id.CreatedAt
if maxCreatedAt == nil || createdAt.After(maxCreatedAt) {
Expand Down

0 comments on commit e784fe5

Please sign in to comment.