Skip to content

Commit

Permalink
Introduce an arcs runtime flag (diffbasedEntityInsertion) to determin…
Browse files Browse the repository at this point in the history
…e whether to use the diffbased entity insertion approach or not.

PiperOrigin-RevId: 368110612
  • Loading branch information
Alice Johnson authored and arcs-c3po committed Apr 14, 2021
1 parent 2e9efd5 commit 362fe21
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,19 @@
package arcs.android.storage.database

import android.content.Context
import androidx.annotation.VisibleForTesting
import androidx.lifecycle.LifecycleObserver
import arcs.core.storage.StorageKey
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.Database
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseIdentifier
import arcs.core.storage.database.DatabaseManager
import arcs.core.storage.database.DatabasePerformanceStatistics.Snapshot
import arcs.core.storage.database.runOnAllDatabases
import arcs.core.storage.database.sumOnAllDatabases
import arcs.core.util.guardedBy
import java.util.concurrent.atomic.AtomicReference
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
Expand All @@ -34,13 +37,16 @@ import kotlinx.coroutines.sync.withLock
class AndroidSqliteDatabaseManager(
context: Context,
// Maximum size of the database file, if it surpasses this size, the database gets reset.
maxDbSizeBytes: Int? = null
maxDbSizeBytes: Int? = null,
databaseConfig: DatabaseConfig = DatabaseConfig()
) : DatabaseManager, LifecycleObserver {
private val context = context.applicationContext
private val mutex = Mutex()
private val dbCache by guardedBy(mutex, mutableMapOf<DatabaseIdentifier, DatabaseImpl>())
private val maxDbSize = maxDbSizeBytes ?: MAX_DB_SIZE_BYTES
override val registry = AndroidSqliteDatabaseRegistry(context)
@VisibleForTesting
override var databaseConfig = AtomicReference(databaseConfig)

// TODO(b/174432505): Don't use the GLOBAL_INSTANCE, accept as a constructor param instead.
private val storageKeyManager = StorageKeyManager.GLOBAL_INSTANCE
Expand All @@ -58,7 +64,7 @@ class AndroidSqliteDatabaseManager(
val entry = registry.register(name, persistent)
return mutex.withLock {
dbCache[entry.name to entry.isPersistent]
?: DatabaseImpl(context, storageKeyManager, name, persistent)
?: DatabaseImpl(context, storageKeyManager, name, persistent, databaseConfig::get)
.also {
dbCache[entry.name to entry.isPersistent] = it
}
Expand Down
2 changes: 2 additions & 0 deletions java/arcs/android/storage/database/DatabaseImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import arcs.core.storage.StorageKey
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.Database
import arcs.core.storage.database.DatabaseClient
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseOp
import arcs.core.storage.database.DatabasePerformanceStatistics
Expand Down Expand Up @@ -125,6 +126,7 @@ class DatabaseImpl(
private val storageKeyManager: StorageKeyManager,
databaseName: String,
persistent: Boolean = true,
@VisibleForTesting val databaseConfig: () -> DatabaseConfig,
val onDatabaseClose: suspend () -> Unit = {}
) : Database, SQLiteOpenHelper(
context,
Expand Down
14 changes: 14 additions & 0 deletions java/arcs/core/storage/database/DatabaseManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package arcs.core.storage.database

import arcs.core.common.collectExceptions
import arcs.core.storage.StorageKey
import java.util.concurrent.atomic.AtomicReference
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.supervisorScope

Expand All @@ -26,6 +27,9 @@ interface DatabaseManager {
/** Manifest of [Database]s managed by this [DatabaseManager]. */
val registry: DatabaseRegistry

/** Database Configurations */
val databaseConfig: AtomicReference<DatabaseConfig>

/**
* Gets a [Database] for the given [name]. If [persistent] is `false`, the [Database] should
* only exist in-memory (if possible for the current platform).
Expand Down Expand Up @@ -87,6 +91,11 @@ interface DatabaseManager {
* Extracts all IDs of any hard reference that points to the given [backingStorageKey].
*/
suspend fun getAllHardReferenceIds(backingStorageKey: StorageKey): Set<String>

/**
* Updates the database configurations
*/
fun updateDatabaseConfig(databaseConfig: DatabaseConfig) = this.databaseConfig.set(databaseConfig)
}

/**
Expand Down Expand Up @@ -136,3 +145,8 @@ val DatabaseIdentifier.name: String
/** Whether or not the [Database] should be persisted to disk. */
val DatabaseIdentifier.persistent: Boolean
get() = second

/** Database configurations of the runtime flags relevant to the database and their values. */
data class DatabaseConfig(
val diffbasedEntityInsertion: Boolean = false
)
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import arcs.core.data.Schema
import arcs.core.storage.StorageKey
import arcs.core.storage.database.Database
import arcs.core.storage.database.DatabaseClient
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseIdentifier
import arcs.core.storage.database.DatabaseManager
Expand All @@ -28,6 +29,7 @@ import arcs.core.util.guardedBy
import arcs.core.util.performance.PerformanceStatistics
import arcs.core.util.performance.Timer
import arcs.jvm.util.JvmTime
import java.util.concurrent.atomic.AtomicReference
import kotlin.coroutines.coroutineContext
import kotlin.reflect.KClass
import kotlinx.coroutines.CoroutineScope
Expand All @@ -49,6 +51,7 @@ open class FakeDatabaseManager(val onGarbageCollection: () -> Unit = {}) : Datab
private val _manifest = FakeDatabaseRegistry()
private val clients = arrayListOf<DatabaseClient>()
override val registry: FakeDatabaseRegistry = _manifest
override var databaseConfig: AtomicReference<DatabaseConfig> = AtomicReference(DatabaseConfig())

fun addClients(vararg clients: DatabaseClient) = this.clients.addAll(clients)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import arcs.core.data.SchemaFields
import arcs.core.data.util.toReferencable
import arcs.core.storage.RawReference
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseManager
import arcs.core.storage.testutil.DummyStorageKey
Expand Down Expand Up @@ -267,6 +268,27 @@ class AndroidSqliteDatabaseManagerTest {
assertThat(manager.removeEntitiesHardReferencing(refKey, "id2")).isEqualTo(0)
}

@Test
fun updateDatabaseConfig_updatesRuntimeFlags() = runBlockingTest {
val database = manager.getDatabase("foo", true) as DatabaseImpl

val managerDatabaseConfigBefore = manager.databaseConfig.get()
val databaseDatabaseConfigBefore = database.databaseConfig()

manager.updateDatabaseConfig(DatabaseConfig(diffbasedEntityInsertion = true))

val managerDatabaseConfigAfter = manager.databaseConfig.get()
val databaseDatabaseConfigAfter = database.databaseConfig()

val expectedBefore = DatabaseConfig()
val expectedAfter = DatabaseConfig(diffbasedEntityInsertion = true)

assertThat(managerDatabaseConfigBefore).isEqualTo(expectedBefore)
assertThat(databaseDatabaseConfigBefore).isEqualTo(expectedBefore)
assertThat(managerDatabaseConfigAfter).isEqualTo(expectedAfter)
assertThat(databaseDatabaseConfigAfter).isEqualTo(expectedAfter)
}

private fun entityWithHardRef(refId: String) = DatabaseData.Entity(
RawEntity(
collections = mapOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import arcs.android.common.map
import arcs.android.common.transaction
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.testutil.DummyStorageKeyManager
import com.google.common.truth.Truth.assertThat
import org.junit.Test
Expand All @@ -43,11 +44,14 @@ class DatabaseDowngradeTest {
dummyHelper.assertIndexes(dummyHelper.getIndexNames())
dummyHelper.close()

val databaseConfig = DatabaseConfig()

val databaseImpl = DatabaseImpl(
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"arcs",
true
true,
{ databaseConfig }
)

// Open up the databaseImpl, so it performs a downgrade.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import arcs.android.storage.database.testutil.SmallIntegerIdGenerator
import arcs.android.util.testutil.AndroidLogRule
import arcs.core.data.SchemaRegistry
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.testutil.DummyStorageKey
import arcs.core.storage.testutil.DummyStorageKeyManager
import arcs.core.testutil.runFuzzTest
Expand All @@ -45,10 +46,12 @@ class DatabaseImplFuzzTest {
@Before
fun setUp() {
BuildFlags.WRITE_ONLY_STORAGE_STACK = true
val databaseConfig = DatabaseConfig()
database = DatabaseImpl(
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3"
"test.sqlite3",
databaseConfig = { databaseConfig }
)
StorageKeyManager.GLOBAL_INSTANCE.addParser(DummyStorageKey)
}
Expand Down
9 changes: 7 additions & 2 deletions javatests/arcs/android/storage/database/DatabaseImplTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import arcs.core.storage.RawReference
import arcs.core.storage.StorageKey
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.DatabaseClient
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseOp
import arcs.core.storage.database.ReferenceWithVersion
Expand Down Expand Up @@ -84,14 +85,16 @@ class DatabaseImplTest(private val parameters: ParameterizedBuildFlags) {
private lateinit var database: DatabaseImpl
private lateinit var db: SQLiteDatabase
private val fixtureEntities = FixtureEntities()
private val databaseConfig = DatabaseConfig()

@Before
fun setUp() {
BuildFlags.WRITE_ONLY_STORAGE_STACK = true
database = DatabaseImpl(
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3"
"test.sqlite3",
databaseConfig = { databaseConfig }
)
db = database.writableDatabase
StorageKeyManager.GLOBAL_INSTANCE.addParser(DummyStorageKey)
Expand Down Expand Up @@ -3974,7 +3977,8 @@ class DatabaseImplTest(private val parameters: ParameterizedBuildFlags) {
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3",
persistent = false
persistent = false,
{ databaseConfig }
)

assertThat(inMemoryDatabase.getSize()).isGreaterThan(0)
Expand Down Expand Up @@ -4134,6 +4138,7 @@ class DatabaseImplTest(private val parameters: ParameterizedBuildFlags) {
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3",
databaseConfig = { databaseConfig },
onDatabaseClose = {
onCloseCalled = true
}
Expand Down

0 comments on commit 362fe21

Please sign in to comment.