From f701137bc1b4522a04cc60518b746e323e25ba16 Mon Sep 17 00:00:00 2001 From: Jeehyun Kim Date: Fri, 27 Oct 2023 15:26:19 +0900 Subject: [PATCH] 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 --- .github/workflows/ci.yml | 4 ++- .../kotlin/dev/yorkie/core/GCTest.kt | 32 +++++++++++++++++++ .../kotlin/dev/yorkie/document/Document.kt | 3 ++ .../yorkie/document/operation/SetOperation.kt | 3 +- .../dev/yorkie/document/DocumentTest.kt | 17 ++++++++++ 5 files changed, 57 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65efcde98..b29d3355d 100644 --- 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 index 9467dbd1e..39ab505b2 100644 --- 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): 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 index b15a81f67..037c3f7c3 100644 --- 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()) 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 index 70f962519..7797863f5 100644 --- 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 index 4483acbee..f553ea209 100644 --- 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) + } + } }