'SELECT' queries sometimes take too long to run

[LZRS]

Describe the bug Running a simple GET query through FhirEngine#get sometimes takes too long. After some tests, it seems it may be caused by the query having to wait for other queued transactions to complete for it to run. This seems to be because the 'SELECT' queries similar to other CRUD operations in Database implementation are wrapped in a Transaction.

From the docs

Room will only perform at most one transaction at a time, additional transactions are queued and executed on a first come, first serve order

Would you like to work on the issue? Please state if this issue should be assigned to you or who you think could help to solve this issue.

[MJ1998]

Could it be because you are calling FhirEngine.get on UI thread ? May be try wrapping your call with withContext(Dispatchers.IO) { }

[LZRS]

Could it be because you are calling FhirEngine.get on UI thread ? May be try wrapping your call with withContext(Dispatchers.IO) { } Using the withContext(Dispatchers.IO) { }, the delay still seems to happen

Also added a test with this branch

diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
index 16446abf..31a1e35f 100644
--- a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
+++ b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
@@ -50,12 +50,16 @@ import com.google.android.fhir.testing.readJsonArrayFromFile
 import com.google.android.fhir.versionId
 import com.google.common.truth.Correspondence
 import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import java.math.BigDecimal
 import java.time.Instant
 import java.util.Date
 import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withContext
 import org.hl7.fhir.r4.model.Address
 import org.hl7.fhir.r4.model.CarePlan
 import org.hl7.fhir.r4.model.CodeableConcept
@@ -120,7 +124,7 @@ class DatabaseImplTest {
   private fun buildFhirService(customSearchParameter: List<SearchParameter>? = null) {
     services =
       FhirServices.builder(context)
-        .inMemory()
+//        .inMemory()
         .apply {
           if (encrypted) enableEncryptionIfSupported()
           setSearchParameters(customSearchParameter)
@@ -150,6 +154,27 @@ class DatabaseImplTest {
     assertResourceEquals(TEST_PATIENT_2, database.select(ResourceType.Patient, TEST_PATIENT_2_ID))
   }

+  @Test
+  fun select_transactionDelay() {
+    runBlocking {
+      database.insert(TEST_PATIENT_1)
+
+      launch {
+        println("Transaction started")
+        database.withTransaction {
+          delay(3000)
+        }
+        println("Transaction completed")
+      }
+
+      launch {
+        println("Patient Loading")
+        database.select(ResourceType.Patient, TEST_PATIENT_1_ID)
+        println("Patient Loaded")
+      }
+    }
+  }
+
   @Test
   fun update_existentResource_shouldUpdateResource() = runBlocking {
     val patient = Patient()

It printed out

05-25 01:50:02.327 12533 12556 I TestRunner: started: select_transactionDelay[encrypted=false](com.google.android.fhir.db.impl.DatabaseImplTest)
05-25 01:50:03.777 12533 12556 I System.out: Transaction started
05-25 01:50:03.777 12533 12556 I System.out: Patient Loading
05-25 01:50:06.782 12533 12556 I System.out: Transaction completed
05-25 01:50:06.791 12533 12556 I System.out: Patient Loaded
05-25 01:50:06.796 12533 12556 I TestRunner: finished: select_transactionDelay[encrypted=false](com.google.android.fhir.db.impl.DatabaseImplTest)

[LZRS]

Removing the db.withTransaction from the select method

diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
index b156b467..4fab6b5a 100644
--- a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
+++ b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt
@@ -174,12 +174,17 @@ internal class DatabaseImpl(
   }

   override suspend fun select(type: ResourceType, id: String): Resource {
-    return db.withTransaction {
-      resourceDao.getResource(resourceId = id, resourceType = type)?.let {
+//    return db.withTransaction {
+//      resourceDao.getResource(resourceId = id, resourceType = type)?.let {
+//        iParser.parseResource(it)
+//      }
+//        ?: throw ResourceNotFoundException(type.name, id)
+//    } as Resource
+
+    return resourceDao.getResource(resourceId = id, resourceType = type)?.let {
         iParser.parseResource(it)
-      }
+      } as? Resource
         ?: throw ResourceNotFoundException(type.name, id)
-    } as Resource
   }

   override suspend fun insertSyncedResources(resources: List<Resource>) {

It prints

05-25 02:05:02.355 12752 12775 I TestRunner: started: select_transactionDelay[encrypted=false](com.google.android.fhir.db.impl.DatabaseImplTest)
05-25 02:05:03.771 12752 12775 I System.out: Transaction started
05-25 02:05:03.771 12752 12775 I System.out: Patient Loading
05-25 02:05:03.776 12752 12775 I System.out: Patient Loaded
05-25 02:05:06.773 12752 12775 I System.out: Transaction completed
05-25 02:05:06.779 12752 12775 I TestRunner: finished: select_transactionDelay[encrypted=false](com.google.android.fhir.db.impl.DatabaseImplTest)

[MJ1998]

@yigit Need your help here. I did some research but couldn't find anything on google. Only found one suggestion to make DAO layer asynchronous by providing a flow (or LiveData) of values. But I don't think this will help the above issue.

[MJ1998]

From discussion with @yigit, this particular read api should be executed independently of any transaction.

Reason (in Yigt's words): The transaction you add there doesn't really help with anything because you are only doing a read inside that transaction and it is a single read. SQLite will already do a "read transaction" for it implicitly, which will make that consistent.

Would you like to work on this @LZRS ?

[LZRS]

From discussion with @yigit, this particular read api should be executed independently of any transaction.

Reason (in Yigt's words): The transaction you add there doesn't really help with anything because you are only doing a read inside that transaction and it is a single read. SQLite will already do a "read transaction" for it implicitly, which will make that consistent.

Would you like to work on this @LZRS ?

Yeah, I would like to work on the issue.

Also on the same, would it also affect reads using FhirEngine#search , that joins to other tables?