perf(model-server): avoid starting suspendable transaction inside suspendable transactions

This avoids blocking one additional thread.

Author: Oleksandr Dzhychko

model-server/src/main/kotlin/org/modelix/model/server/handlers/KeyValueLikeModelServer.kt

@@ -112,7 +112,11 @@ class KeyValueLikeModelServer(val repositoriesManager: RepositoriesManager) {
                 if (isHealthy()) {
                     call.respondText(text = "healthy", contentType = ContentType.Text.Plain, status = HttpStatusCode.OK)
                 } else {
-                    call.respondText(text = "not healthy", contentType = ContentType.Text.Plain, status = HttpStatusCode.InternalServerError)
+                    call.respondText(
+                        text = "not healthy",
+                        contentType = ContentType.Text.Plain,
+                        status = HttpStatusCode.InternalServerError,
+                    )
                 }
             }
             get<Paths.getHeaders> {
@@ -317,18 +321,23 @@ class KeyValueLikeModelServer(val repositoriesManager: RepositoriesManager) {
                 HashUtil.isSha256(key) -> {
                     hashedObjects[key] = value ?: throw IllegalArgumentException("No value provided for $key")
                 }
+
                 BranchReference.tryParseBranch(key) != null -> {
                     branchChanges[BranchReference.tryParseBranch(key)!!] = value
                 }
+
                 key.startsWith(PROTECTED_PREFIX) -> {
                     throw NoPermissionException("Access to keys starting with '$PROTECTED_PREFIX' is only permitted to the model server itself.")
                 }
+
                 key.startsWith(RepositoriesManager.KEY_PREFIX) -> {
                     throw NoPermissionException("Access to keys starting with '${RepositoriesManager.KEY_PREFIX}' is only permitted to the model server itself.")
                 }
+
                 key == RepositoriesManager.LEGACY_SERVER_ID_KEY || key == RepositoriesManager.LEGACY_SERVER_ID_KEY2 -> {
                     throw NoPermissionException("'$key' is read-only.")
                 }
+
                 else -> {
                     userDefinedEntries[key] = value
                 }
@@ -342,9 +351,9 @@ class KeyValueLikeModelServer(val repositoriesManager: RepositoriesManager) {
             storeClient.putAll(userDefinedEntries)
             for ((branch, value) in branchChanges) {
                 if (value == null) {
-                    runBlocking { repositoriesManager.removeBranches(branch.repositoryId, setOf(branch.branchName)) }
+                    repositoriesManager.removeBranchesBlocking(branch.repositoryId, setOf(branch.branchName))
                 } else {
-                    runBlocking { repositoriesManager.mergeChanges(branch, value) }
+                    repositoriesManager.mergeChangesBlocking(branch, value)
                 }
             }
         }
@@ -366,7 +375,10 @@ class KeyValueLikeModelServer(val repositoriesManager: RepositoriesManager) {
         if (key.startsWith(RepositoriesManager.KEY_PREFIX)) {
             throw NoPermissionException("Access to keys starting with '${RepositoriesManager.KEY_PREFIX}' is only permitted to the model server itself.")
         }
-        if ((key == RepositoriesManager.LEGACY_SERVER_ID_KEY || key == RepositoriesManager.LEGACY_SERVER_ID_KEY2) && type.includes(EPermissionType.WRITE)) {
+        if ((key == RepositoriesManager.LEGACY_SERVER_ID_KEY || key == RepositoriesManager.LEGACY_SERVER_ID_KEY2) && type.includes(
+                EPermissionType.WRITE,
+            )
+        ) {
             throw NoPermissionException("'$key' is read-only.")
         }
         if (HashUtil.isSha256(key)) {

model-server/src/main/kotlin/org/modelix/model/server/handlers/RepositoriesManager.kt

@@ -200,8 +200,18 @@ class RepositoriesManager(val client: LocalModelClient) {
     }
 
     suspend fun removeBranches(repository: RepositoryId, branchNames: Set<String>) {
+        return store.runTransactionSuspendable {
+            removeBranchesBlocking(repository, branchNames)
+        }
+    }
+
+    /**
+     * Same as [removeBranches] but blocking.
+     * Caller is expected to execute it outside the request thread.
+     */
+    fun removeBranchesBlocking(repository: RepositoryId, branchNames: Set<String>) {
         if (branchNames.isEmpty()) return
-        store.runTransactionSuspendable {
+        store.runTransaction {
             val key = branchListKey(repository)
             val existingBranches = store[key]?.lines()?.toSet() ?: emptySet()
             val remainingBranches = existingBranches - branchNames
@@ -213,9 +223,18 @@ class RepositoriesManager(val client: LocalModelClient) {
     }
 
     suspend fun mergeChanges(branch: BranchReference, newVersionHash: String): String {
-        var result: String? = null
-        store.runTransactionSuspendable {
-            val headHash = getVersionHashInsideTransaction(branch)
+        return store.runTransactionSuspendable {
+            mergeChangesBlocking(branch, newVersionHash)
+        }
+    }
+
+    /**
+     * Same as [mergeChanges] but blocking.
+     * Caller is expected to execute it outside the request thread.
+     */
+    fun mergeChangesBlocking(branch: BranchReference, newVersionHash: String): String {
+        return store.runTransaction {
+            val headHash = getVersionHashBlocking(branch)
             val mergedHash = if (headHash == null) {
                 newVersionHash
             } else {
@@ -232,9 +251,8 @@ class RepositoriesManager(val client: LocalModelClient) {
             putVersionHash(branch, mergedHash)
             ensureRepositoriesAreInList(setOf(branch.repositoryId))
             ensureBranchesAreInList(branch.repositoryId, setOf(branch.branchName))
-            result = mergedHash
+            mergedHash
         }
-        return result!!
     }
 
     suspend fun getVersion(branch: BranchReference): CLVersion? {
@@ -243,12 +261,24 @@ class RepositoriesManager(val client: LocalModelClient) {
 
     suspend fun getVersionHash(branch: BranchReference): String? {
         return store.runTransactionSuspendable {
-            getVersionHashInsideTransaction(branch)
+            getVersionHashBlocking(branch)
         }
     }
 
-    private fun getVersionHashInsideTransaction(branch: BranchReference): String? {
-        return store[branchKey(branch)] ?: store[legacyBranchKey(branch)]?.also { store.put(branchKey(branch), it, true) }
+    /**
+     * Same as [getVersionHash] but blocking.
+     * Caller is expected to execute it outside the request thread.
+     */
+    private fun getVersionHashBlocking(branch: BranchReference): String? {
+        return store.runTransaction {
+            store[branchKey(branch)] ?: store[legacyBranchKey(branch)]?.also {
+                store.put(
+                    branchKey(branch),
+                    it,
+                    true,
+                )
+            }
+        }
     }
 
     private fun putVersionHash(branch: BranchReference, hash: String?) {
@@ -368,6 +398,7 @@ class ObjectDataMap(private val byHashObjects: Map<String, String>) : ObjectData
     init {
         HashUtil.checkObjectHashes(byHashObjects)
     }
+
     override suspend fun asMap(): Map<String, String> = byHashObjects
     override fun asFlow(): Flow<Pair<String, String>> = byHashObjects.entries.asFlow().map { it.toPair() }
 }