Fix missing collection of removed elements from the root (#140)

* Fix missing collection of removed elements from the root * clean up unused images before setting up the docker
yorkie-team · Oct 27, 2023 · f701137 · f701137
1 parent 55c49c1
commit f701137
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 2 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -63,7 +63,9 @@ jobs:
       - uses: gradle/gradle-build-action@v2
       - name: Setup Docker on macOS
         uses: douglascamata/setup-docker-macos-action@fix-python-dep
-      - run: docker-compose -f docker/docker-compose-ci.yml up --build -d
+      - run: |
+          docker system prune --all --force
+          docker-compose -f docker/docker-compose-ci.yml up --build -d
       - uses: actions/cache@v3
         id: avd-cache
         with:

diff --git a/yorkie/src/androidTest/kotlin/dev/yorkie/core/GCTest.kt b/yorkie/src/androidTest/kotlin/dev/yorkie/core/GCTest.kt
@@ -10,6 +10,7 @@ import dev.yorkie.document.json.JsonTree.TextNode
 import dev.yorkie.document.json.TreeBuilder.element
 import dev.yorkie.document.json.TreeBuilder.text
 import dev.yorkie.document.time.TimeTicket
+import dev.yorkie.gson
 import dev.yorkie.util.IndexTreeNode
 import kotlinx.coroutines.runBlocking
 import org.junit.Test
@@ -457,6 +458,37 @@ class GCTest {
         assertEquals(4, document.garbageLength)
     }
 
+    @Test
+    fun test_collecting_removed_elements_from_root_and_clone() = runBlocking {
+        data class Point(val x: Int, val y: Int)
+
+        val client = createClient()
+        val documentKey = UUID.randomUUID().toString().toDocKey()
+        val document = Document(documentKey)
+
+        client.activateAsync().await()
+        client.attachAsync(document, isRealTimeSync = false).await()
+
+        document.updateAsync { root, _ ->
+            root["point"] = gson.toJson(Point(0, 0))
+        }.await()
+
+        document.updateAsync { root, _ ->
+            root["point"] = gson.toJson(Point(1, 1))
+        }.await()
+
+        document.updateAsync { root, _ ->
+            root["point"] = gson.toJson(Point(2, 2))
+        }.await()
+
+        document.updateAsync { root, _ ->
+            root.remove("point")
+        }.await()
+
+        assertEquals(3, document.garbageLength)
+        assertEquals(3, document.garbageLengthFromClone)
+    }
+
     private fun getNodeLength(root: IndexTreeNode<CrdtTreeNode>): Int {
         return root.allChildren.fold(root.allChildren.size) { acc, child ->
             acc + getNodeLength(child)

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt b/yorkie/src/main/kotlin/dev/yorkie/document/Document.kt
@@ -84,6 +84,9 @@ public class Document(public val key: Key, private val options: Options = Option
     public val garbageLength: Int
         get() = root.getGarbageLength()
 
+    internal val garbageLengthFromClone: Int
+        get() = clone?.root?.getGarbageLength() ?: 0
+
     internal val onlineClients = MutableStateFlow(setOf<ActorID>())
 
     private val _presences = MutableStateFlow(UninitializedPresences)

diff --git a/yorkie/src/main/kotlin/dev/yorkie/document/operation/SetOperation.kt b/yorkie/src/main/kotlin/dev/yorkie/document/operation/SetOperation.kt
@@ -30,7 +30,8 @@ internal data class SetOperation(
         val parentObject = root.findByCreatedAt(parentCreatedAt)
         return if (parentObject is CrdtObject) {
             val copiedValue = value.deepCopy()
-            parentObject[key] = copiedValue
+            val removed = parentObject.set(key, copiedValue)
+            removed?.let(root::registerRemovedElement)
             root.registerElement(copiedValue, parentObject)
             listOf(OperationInfo.SetOpInfo(key, root.createPath(parentCreatedAt)))
         } else {

diff --git a/yorkie/src/test/kotlin/dev/yorkie/document/DocumentTest.kt b/yorkie/src/test/kotlin/dev/yorkie/document/DocumentTest.kt
@@ -5,6 +5,7 @@ import dev.yorkie.document.json.JsonArray
 import dev.yorkie.document.json.JsonText
 import dev.yorkie.document.operation.OperationInfo.RemoveOpInfo
 import dev.yorkie.document.operation.OperationInfo.SetOpInfo
+import dev.yorkie.document.time.TimeTicket
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
@@ -248,4 +249,20 @@ class DocumentTest {
             assertNull(target.getValueByPath("$..."))
         }
     }
+
+    @Test
+    fun `should remove previously inserted elements in heap when running GC`() {
+        runTest {
+            target.updateAsync { root, _ ->
+                root["a"] = 1
+                root["a"] = 2
+                root.remove("a")
+            }.await()
+            assertEquals("{}", target.toJson())
+            assertEquals(2, target.garbageLength)
+
+            target.garbageCollect(TimeTicket.MaxTimeTicket)
+            assertEquals(0, target.garbageLength)
+        }
+    }
 }