Remove node from index during GC (#913)

While improving GC structure, we missed removing nodes from internal indexes of Text when purging tombstones(#866). --------- Co-authored-by: Youngteac Hong <susukang98@gmail.com>
yorkie-team · Jul 4, 2024 · 8ab096e · 8ab096e
1 parent 58d45c6
commit 8ab096e
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 0 deletions.
diff --git a/pkg/document/crdt/rga_tree_split.go b/pkg/document/crdt/rga_tree_split.go
@@ -628,6 +628,9 @@ func (s *RGATreeSplit[V]) ToTestString() string {
 func (s *RGATreeSplit[V]) Purge(child GCChild) error {
 	node := child.(*RGATreeSplitNode[V])
 
+	s.treeByIndex.Delete(node.indexNode)
+	s.treeByID.Remove(node.id)
+
 	node.prev.next = node.next
 	if node.next != nil {
 		node.next.prev = node.prev

diff --git a/pkg/document/crdt/text.go b/pkg/document/crdt/text.go
@@ -22,6 +22,8 @@ import (
 	"unicode/utf16"
 
 	"github.com/yorkie-team/yorkie/pkg/document/time"
+	"github.com/yorkie-team/yorkie/pkg/llrb"
+	"github.com/yorkie-team/yorkie/pkg/splay"
 )
 
 // TextValue is a value of Text which has an attributes that represent
@@ -366,3 +368,13 @@ func (t *Text) ToTestString() string {
 func (t *Text) CheckWeight() bool {
 	return t.rgaTreeSplit.CheckWeight()
 }
+
+// TreeByIndex returns IndexTree of the text for debugging purpose.
+func (t *Text) TreeByIndex() *splay.Tree[*RGATreeSplitNode[*TextValue]] {
+	return t.rgaTreeSplit.treeByIndex
+}
+
+// TreeByID returns the tree by ID for debugging purpose.
+func (t *Text) TreeByID() *llrb.Tree[*RGATreeSplitNodeID, *RGATreeSplitNode[*TextValue]] {
+	return t.rgaTreeSplit.treeByID
+}
diff --git a/pkg/document/document_test.go b/pkg/document/document_test.go
@@ -524,4 +524,29 @@ func TestDocument(t *testing.T) {
 		assert.Equal(t, "{}", doc.Marshal())
 		assert.Equal(t, 0, doc.GarbageLen())
 	})
+
+	t.Run("purge node from index during GC test", func(t *testing.T) {
+		doc := document.New("d1")
+
+		assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error {
+			root.SetNewText("k1")
+			return nil
+		}))
+		assert.Equal(t, 1, doc.Root().GetText("k1").TreeByID().Len())
+
+		assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("k1").Edit(0, 0, "ABC", nil)
+			return nil
+		}))
+		assert.Equal(t, 2, doc.Root().GetText("k1").TreeByID().Len())
+
+		assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("k1").Edit(1, 3, "", nil)
+			return nil
+		}))
+		assert.Equal(t, 3, doc.Root().GetText("k1").TreeByID().Len())
+
+		doc.GarbageCollect(time.MaxTicket)
+		assert.Equal(t, 2, doc.Root().GetText("k1").TreeByID().Len())
+	})
 }
diff --git a/test/integration/text_test.go b/test/integration/text_test.go
@@ -262,6 +262,8 @@ func TestText(t *testing.T) {
 		syncClientsThenAssertEqual(t, []clientAndDocPair{{c1, d1}, {c2, d2}})
 		assert.True(t, d1.Root().GetText("k1").CheckWeight())
 		assert.True(t, d2.Root().GetText("k1").CheckWeight())
+		assert.True(t, d1.Root().GetText("k1").TreeByIndex().CheckWeight())
+		assert.True(t, d2.Root().GetText("k1").TreeByIndex().CheckWeight())
 	})
 
 	// Peritext test