Skip to content

Commit

Permalink
Remove panic in CRDT array (#524)
Browse files Browse the repository at this point in the history
Removing the panic to prevent server shutdown when encountering CRDT array errors.
To keep the usability of the SDK, this commit keeps the panic in `json` package.

---------

Co-authored-by: Youngteac Hong <susukang98@gmail.com>
  • Loading branch information
emplam27 and hackerwins authored May 29, 2023
1 parent 3b0606e commit 77d14f0
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 50 deletions.
16 changes: 12 additions & 4 deletions pkg/document/crdt/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ func (a *Array) Add(elem Element) *Array {
}

// Get returns the element of the given index.
func (a *Array) Get(idx int) Element {
return a.elements.Get(idx).elem
func (a *Array) Get(idx int) (Element, error) {
node, err := a.elements.Get(idx)
if err != nil {
return nil, err
}
return node.elem, nil
}

// FindPrevCreatedAt returns the creation time of the previous element of the
Expand All @@ -60,8 +64,12 @@ func (a *Array) FindPrevCreatedAt(createdAt *time.Ticket) *time.Ticket {
}

// Delete deletes the element of the given index.
func (a *Array) Delete(idx int, deletedAt *time.Ticket) Element {
return a.elements.Delete(idx, deletedAt).elem
func (a *Array) Delete(idx int, deletedAt *time.Ticket) (Element, error) {
node, err := a.elements.Delete(idx, deletedAt)
if err != nil {
return nil, err
}
return node.elem, nil
}

// MoveAfter moves the given `createdAt` element after the `prevCreatedAt`
Expand Down
18 changes: 12 additions & 6 deletions pkg/document/crdt/rga_tree_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,11 @@ func (a *RGATreeList) InsertAfter(prevCreatedAt *time.Ticket, elem Element) {
}

// Get returns the element of the given index.
func (a *RGATreeList) Get(idx int) *RGATreeListNode {
splayNode, offset := a.nodeMapByIndex.Find(idx)
func (a *RGATreeList) Get(idx int) (*RGATreeListNode, error) {
splayNode, offset, err := a.nodeMapByIndex.Find(idx)
if err != nil {
return nil, err
}
node := splayNode.Value()

if idx == 0 && splayNode == a.dummyHead.indexNode {
Expand All @@ -199,7 +202,7 @@ func (a *RGATreeList) Get(idx int) *RGATreeListNode {
}
}

return node
return node, nil
}

// DeleteByCreatedAt deletes the given element.
Expand Down Expand Up @@ -228,9 +231,12 @@ func (a *RGATreeList) StructureAsString() string {
}

// Delete deletes the node of the given index.
func (a *RGATreeList) Delete(idx int, deletedAt *time.Ticket) *RGATreeListNode {
target := a.Get(idx)
return a.DeleteByCreatedAt(target.CreatedAt(), deletedAt)
func (a *RGATreeList) Delete(idx int, deletedAt *time.Ticket) (*RGATreeListNode, error) {
target, err := a.Get(idx)
if err != nil {
return nil, err
}
return a.DeleteByCreatedAt(target.CreatedAt(), deletedAt), nil
}

// MoveAfter moves the given `createdAt` element after the `prevCreatedAt`
Expand Down
29 changes: 20 additions & 9 deletions pkg/document/crdt/rga_tree_split.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ type RGATreeSplit[V RGATreeSplitValue] struct {
treeByIndex *splay.Tree[*RGATreeSplitNode[V]]
treeByID *llrb.Tree[*RGATreeSplitNodeID, *RGATreeSplitNode[V]]

// removedNodeMap is a map that holds tombstone nodes
// when the edit operation is executed.
// removedNodeMap is a map to store removed nodes. It is used to
// delete the node physically when the garbage collection is executed.
removedNodeMap map[string]*RGATreeSplitNode[V]
}

Expand All @@ -310,22 +310,33 @@ func NewRGATreeSplit[V RGATreeSplitValue](initialHead *RGATreeSplitNode[V]) *RGA
}
}

func (s *RGATreeSplit[V]) createRange(from, to int) (*RGATreeSplitNodePos, *RGATreeSplitNodePos) {
fromPos := s.findNodePos(from)
func (s *RGATreeSplit[V]) createRange(from, to int) (*RGATreeSplitNodePos, *RGATreeSplitNodePos, error) {
fromPos, err := s.findNodePos(from)
if err != nil {
return nil, nil, err
}
if from == to {
return fromPos, fromPos
return fromPos, fromPos, nil
}

toPos, err := s.findNodePos(to)
if err != nil {
return nil, nil, err
}

return fromPos, s.findNodePos(to)
return fromPos, toPos, nil
}

func (s *RGATreeSplit[V]) findNodePos(index int) *RGATreeSplitNodePos {
splayNode, offset := s.treeByIndex.Find(index)
func (s *RGATreeSplit[V]) findNodePos(index int) (*RGATreeSplitNodePos, error) {
splayNode, offset, err := s.treeByIndex.Find(index)
if err != nil {
return nil, err
}
node := splayNode.Value()
return &RGATreeSplitNodePos{
id: node.ID(),
relativeOffset: offset,
}
}, nil
}

func (s *RGATreeSplit[V]) findNodeWithSplit(
Expand Down
18 changes: 9 additions & 9 deletions pkg/document/crdt/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestRoot(t *testing.T) {
array.Add(crdt.NewPrimitive(2, ctx.IssueTimeTicket()))
assert.Equal(t, "[0,1,2]", array.Marshal())

targetElement := array.Get(1)
targetElement, _ := array.Get(1)
array.DeleteByCreatedAt(targetElement.CreatedAt(), ctx.IssueTimeTicket())
root.RegisterRemovedElementPair(array, targetElement)
assert.Equal(t, "[0,2]", array.Marshal())
Expand All @@ -58,28 +58,28 @@ func TestRoot(t *testing.T) {
ctx := helper.TextChangeContext(root)
text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket())

fromPos, toPos := text.CreateRange(0, 0)
fromPos, toPos, _ := text.CreateRange(0, 0)
_, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "Hello World", text.String())
assert.Equal(t, 0, root.GarbageLen())

fromPos, toPos = text.CreateRange(5, 10)
fromPos, toPos, _ = text.CreateRange(5, 10)
_, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "HelloYorkied", text.String())
assert.Equal(t, 1, root.GarbageLen())

fromPos, toPos = text.CreateRange(0, 5)
fromPos, toPos, _ = text.CreateRange(0, 5)
_, _, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "Yorkied", text.String())
assert.Equal(t, 2, root.GarbageLen())

fromPos, toPos = text.CreateRange(6, 7)
fromPos, toPos, _ = text.CreateRange(6, 7)
_, _, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestRoot(t *testing.T) {
}

for _, tc := range tests {
fromPos, toPos := text.CreateRange(tc.from, tc.to)
fromPos, toPos, _ := text.CreateRange(tc.from, tc.to)
_, _, err := text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
Expand All @@ -135,21 +135,21 @@ func TestRoot(t *testing.T) {
ctx := helper.TextChangeContext(root)
text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket())

fromPos, toPos := text.CreateRange(0, 0)
fromPos, toPos, _ := text.CreateRange(0, 0)
_, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal())
assert.Equal(t, 0, root.GarbageLen())

fromPos, toPos = text.CreateRange(6, 11)
fromPos, toPos, _ = text.CreateRange(6, 11)
_, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal())
assert.Equal(t, 1, root.GarbageLen())

fromPos, toPos = text.CreateRange(0, 6)
fromPos, toPos, _ = text.CreateRange(0, 6)
_, _, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
Expand Down
2 changes: 1 addition & 1 deletion pkg/document/crdt/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func (t *Text) Remove(removedAt *time.Ticket) bool {
}

// CreateRange returns a pair of RGATreeSplitNodePos of the given integer offsets.
func (t *Text) CreateRange(from, to int) (*RGATreeSplitNodePos, *RGATreeSplitNodePos) {
func (t *Text) CreateRange(from, to int) (*RGATreeSplitNodePos, *RGATreeSplitNodePos, error) {
return t.rgaTreeSplit.createRange(from, to)
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/document/crdt/text_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ func TestText(t *testing.T) {
ctx := helper.TextChangeContext(root)
text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket())

fromPos, toPos := text.CreateRange(0, 0)
fromPos, toPos, _ := text.CreateRange(0, 0)
_, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal())

fromPos, toPos = text.CreateRange(6, 11)
fromPos, toPos, _ = text.CreateRange(6, 11)
_, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal())
Expand Down Expand Up @@ -69,17 +69,17 @@ func TestText(t *testing.T) {
ctx := helper.TextChangeContext(root)
text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket())

fromPos, toPos := text.CreateRange(0, 0)
fromPos, toPos, _ := text.CreateRange(0, 0)
_, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal())

fromPos, toPos = text.CreateRange(6, 11)
fromPos, toPos, _ = text.CreateRange(6, 11)
_, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal())

fromPos, toPos = text.CreateRange(0, 1)
fromPos, toPos, _ = text.CreateRange(0, 1)
err = text.Style(fromPos, toPos, map[string]string{"b": "1"}, ctx.IssueTimeTicket())
assert.NoError(t, err)
assert.Equal(
Expand Down
19 changes: 18 additions & 1 deletion pkg/document/json/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,31 @@ func (p *Array) InsertIntegerAfter(index int, v int) *Array {
return p
}

// Get element of the given index.
func (p *Array) Get(idx int) crdt.Element {
if p.Len() <= idx {
return nil
}

element, err := p.Array.Get(idx)
if err != nil {
panic(err)
}

return element
}

// Delete deletes the element of the given index.
func (p *Array) Delete(idx int) crdt.Element {
if p.Len() <= idx {
return nil
}

ticket := p.context.IssueTimeTicket()
deleted := p.Array.Delete(idx, ticket)
deleted, err := p.Array.Delete(idx, ticket)
if err != nil {
panic(err)
}
p.context.Push(operations.NewRemove(
p.CreatedAt(),
deleted.CreatedAt(),
Expand Down
24 changes: 21 additions & 3 deletions pkg/document/json/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,24 @@ func NewText(ctx *change.Context, text *crdt.Text) *Text {
}
}

// CreateRange creates a range from the given positions.
func (p *Text) CreateRange(from, to int) (*crdt.RGATreeSplitNodePos, *crdt.RGATreeSplitNodePos) {
fromPos, toPos, err := p.Text.CreateRange(from, to)
if err != nil {
panic(err)
}
return fromPos, toPos
}

// Edit edits the given range with the given content and attributes.
func (p *Text) Edit(from, to int, content string, attributes ...map[string]string) *Text {
if from > to {
panic("from should be less than or equal to to")
}
fromPos, toPos := p.Text.CreateRange(from, to)
fromPos, toPos, err := p.Text.CreateRange(from, to)
if err != nil {
panic(err)
}

// TODO(hackerwins): We need to consider the case where the length of
// attributes is greater than 1.
Expand Down Expand Up @@ -85,7 +97,10 @@ func (p *Text) Style(from, to int, attributes map[string]string) *Text {
if from > to {
panic("from should be less than or equal to to")
}
fromPos, toPos := p.Text.CreateRange(from, to)
fromPos, toPos, err := p.Text.CreateRange(from, to)
if err != nil {
panic(err)
}

ticket := p.context.IssueTimeTicket()
if err := p.Text.Style(
Expand Down Expand Up @@ -113,7 +128,10 @@ func (p *Text) Select(from, to int) *Text {
if from > to {
panic("from should be less than or equal to to")
}
fromPos, toPos := p.Text.CreateRange(from, to)
fromPos, toPos, err := p.Text.CreateRange(from, to)
if err != nil {
panic(err)
}

ticket := p.context.IssueTimeTicket()
p.Text.Select(
Expand Down
15 changes: 7 additions & 8 deletions pkg/splay/splay.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"strings"
)

// ErrOutOfIndex is returned when the given index is out of index.
var ErrOutOfIndex = fmt.Errorf("out of index")

// Value represents the data stored in the nodes of Tree.
type Value interface {
Len() int
Expand Down Expand Up @@ -183,9 +186,9 @@ func (t *Tree[V]) IndexOf(node *Node[V]) int {
}

// Find returns the Node and offset of the given index.
func (t *Tree[V]) Find(index int) (*Node[V], int) {
func (t *Tree[V]) Find(index int) (*Node[V], int, error) {
if t.root == nil {
return nil, 0
return nil, 0, nil
}

node := t.root
Expand All @@ -203,14 +206,10 @@ func (t *Tree[V]) Find(index int) (*Node[V], int) {
}

if offset > node.value.Len() {
panic(fmt.Sprintf(
"out of bound of text index: node.length %d, pos %d",
node.value.Len(),
offset,
))
return nil, 0, fmt.Errorf("node length %d, index %d: %w", node.value.Len(), offset, ErrOutOfIndex)
}

return node, offset
return node, offset, nil
}

// String returns a string containing node values.
Expand Down
12 changes: 8 additions & 4 deletions pkg/splay/splay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@ func TestSplayTree(t *testing.T) {
t.Run("insert and splay test", func(t *testing.T) {
tree := splay.NewTree[*stringValue](nil)

node, idx := tree.Find(0)
node, idx, err := tree.Find(0)
assert.Nil(t, node)
assert.NoError(t, err)
assert.Equal(t, 0, idx)

nodeA := tree.Insert(newSplayNode("A2"))
Expand All @@ -71,17 +72,20 @@ func TestSplayTree(t *testing.T) {
assert.Equal(t, 5, tree.IndexOf(nodeC))
assert.Equal(t, 9, tree.IndexOf(nodeD))

node, offset := tree.Find(1)
node, offset, err := tree.Find(1)
assert.Equal(t, nodeA, node)
assert.Equal(t, 1, offset)
assert.NoError(t, err)

node, offset = tree.Find(7)
node, offset, err = tree.Find(7)
assert.Equal(t, nodeC, node)
assert.Equal(t, 2, offset)
assert.NoError(t, err)

node, offset = tree.Find(11)
node, offset, err = tree.Find(11)
assert.Equal(t, nodeD, node)
assert.Equal(t, 2, offset)
assert.NoError(t, err)
})

t.Run("deletion test", func(t *testing.T) {
Expand Down

0 comments on commit 77d14f0

Please sign in to comment.