From 6aa4bb549adc29b638b2ef66fc869d432e8d6893 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 3 May 2023 09:59:47 -0400 Subject: [PATCH] Add database indices to improve message delete performance. --- .../securesms/database/MessageTable.kt | 6 ++- .../securesms/database/ReactionTable.kt | 5 ++ .../securesms/database/SignalDatabase.kt | 1 + .../helpers/SignalDatabaseMigrations.kt | 7 ++- .../V186_ForeignKeyIndicesMigration.kt | 54 +++++++++++++++++++ .../migrations/ApplicationMigrations.java | 7 ++- 6 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V186_ForeignKeyIndicesMigration.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt index 9850070200..0488bcb00b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt @@ -283,7 +283,11 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat "CREATE INDEX IF NOT EXISTS $INDEX_THREAD_STORY_SCHEDULED_DATE_LATEST_REVISION_ID ON $TABLE_NAME ($THREAD_ID, $DATE_RECEIVED, $STORY_TYPE, $PARENT_STORY_ID, $SCHEDULED_DATE, $LATEST_REVISION_ID);", "CREATE INDEX IF NOT EXISTS message_quote_id_quote_author_scheduled_date_latest_revision_id_index ON $TABLE_NAME ($QUOTE_ID, $QUOTE_AUTHOR, $SCHEDULED_DATE, $LATEST_REVISION_ID);", "CREATE INDEX IF NOT EXISTS message_exported_index ON $TABLE_NAME ($EXPORTED);", - "CREATE INDEX IF NOT EXISTS message_id_type_payment_transactions_index ON $TABLE_NAME ($ID,$TYPE) WHERE $TYPE & ${MessageTypes.SPECIAL_TYPE_PAYMENTS_NOTIFICATION} != 0;" + "CREATE INDEX IF NOT EXISTS message_id_type_payment_transactions_index ON $TABLE_NAME ($ID,$TYPE) WHERE $TYPE & ${MessageTypes.SPECIAL_TYPE_PAYMENTS_NOTIFICATION} != 0;", + "CREATE INDEX IF NOT EXISTS message_original_message_id_index ON $TABLE_NAME ($ORIGINAL_MESSAGE_ID);", + "CREATE INDEX IF NOT EXISTS message_latest_revision_id_index ON $TABLE_NAME ($LATEST_REVISION_ID)", + "CREATE INDEX IF NOT EXISTS message_from_recipient_id_index ON $TABLE_NAME ($FROM_RECIPIENT_ID)", + "CREATE INDEX IF NOT EXISTS message_to_recipient_id_index ON $TABLE_NAME ($TO_RECIPIENT_ID)" ) private val MMS_PROJECTION_BASE = arrayOf( diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ReactionTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/ReactionTable.kt index df36e091b8..9f1ebd8c87 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/ReactionTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/ReactionTable.kt @@ -40,6 +40,11 @@ class ReactionTable(context: Context, databaseHelper: SignalDatabase) : Database ) """ + @JvmField + val CREATE_INDEXES = arrayOf( + "CREATE INDEX IF NOT EXISTS reaction_author_id_index ON $TABLE_NAME ($AUTHOR_ID)" + ) + private fun readReaction(cursor: Cursor): ReactionRecord { return ReactionRecord( emoji = CursorUtil.requireString(cursor, EMOJI), diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt index ae9e4a9a51..395dc0a126 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt @@ -134,6 +134,7 @@ open class SignalDatabase(private val context: Application, databaseSecret: Data executeStatements(db, DistributionListTables.CREATE_INDEXES) executeStatements(db, PendingPniSignatureMessageTable.CREATE_INDEXES) executeStatements(db, CallTable.CREATE_INDEXES) + executeStatements(db, ReactionTable.CREATE_INDEXES) executeStatements(db, SearchTable.CREATE_TRIGGERS) executeStatements(db, MessageSendLogTables.CREATE_TRIGGERS) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index 2346e13df6..9f71694106 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -41,6 +41,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V182_CallTableMigra import org.thoughtcrime.securesms.database.helpers.migration.V183_CallLinkTableMigration import org.thoughtcrime.securesms.database.helpers.migration.V184_CallLinkReplaceIndexMigration import org.thoughtcrime.securesms.database.helpers.migration.V185_MessageRecipientsAndEditMessageMigration +import org.thoughtcrime.securesms.database.helpers.migration.V186_ForeignKeyIndicesMigration /** * Contains all of the database migrations for [SignalDatabase]. Broken into a separate file for cleanliness. @@ -49,7 +50,7 @@ object SignalDatabaseMigrations { val TAG: String = Log.tag(SignalDatabaseMigrations.javaClass) - const val DATABASE_VERSION = 185 + const val DATABASE_VERSION = 186 @JvmStatic fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { @@ -200,6 +201,10 @@ object SignalDatabaseMigrations { if (oldVersion < 185) { V185_MessageRecipientsAndEditMessageMigration.migrate(context, db, oldVersion, newVersion) } + + if (oldVersion < 186) { + V186_ForeignKeyIndicesMigration.migrate(context, db, oldVersion, newVersion) + } } @JvmStatic diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V186_ForeignKeyIndicesMigration.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V186_ForeignKeyIndicesMigration.kt new file mode 100644 index 0000000000..ff995d2076 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V186_ForeignKeyIndicesMigration.kt @@ -0,0 +1,54 @@ +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import net.zetetic.database.sqlcipher.SQLiteDatabase +import org.signal.core.util.Stopwatch +import org.signal.core.util.logging.Log + +/** + * We added some foreign keys to the message table (particularly on original_message_id and latest_revision_id) + * that depend on the message table itself. But there were no indices to look up messages by those fields. + * So every time we deleted a message, SQLite had to do a linear scan of the message table to verify that no + * original_message_id or latest_revision_id fields referenced the deleted message._id. + * + * And that is very slow. Like, 40 seconds to delete 100 messages slow. + * + * Thankfully, the solution is simple: add indices on those columns. + * + * While I was at it, I looked at other columns that would need indices as well. + */ +object V186_ForeignKeyIndicesMigration : SignalDatabaseMigration { + + private val TAG = Log.tag(V186_ForeignKeyIndicesMigration::class.java) + + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + val stopwatch = Stopwatch("migration") + + db.execSQL("CREATE INDEX message_original_message_id_index ON message (original_message_id)") + stopwatch.split("original_message_id") + + db.execSQL("CREATE INDEX message_latest_revision_id_index ON message (latest_revision_id)") + stopwatch.split("latest_revision_id") + + db.execSQL("CREATE INDEX message_from_recipient_id_index ON message (from_recipient_id)") + stopwatch.split("from_recipient_id") + + db.execSQL("CREATE INDEX message_to_recipient_id_index ON message (to_recipient_id)") + stopwatch.split("to_recipient_id") + + db.execSQL("CREATE INDEX reaction_author_id_index ON reaction (author_id)") + stopwatch.split("reaction_author") + + // Previous migration screwed up an index replacement, so we need to fix that too + db.execSQL("DROP INDEX IF EXISTS message_quote_id_quote_author_scheduled_date_index") + db.execSQL("CREATE INDEX IF NOT EXISTS message_quote_id_quote_author_scheduled_date_latest_revision_id_index ON message (quote_id, quote_author, scheduled_date, latest_revision_id)") + stopwatch.split("message_fix") + + // The recipient_id indices could be considered "low quality" indices, since they have a smaller domain. + // Running analyze will help SQLite choose the right index to use in the future. + db.execSQL("ANALYZE message") + stopwatch.split("analyze") + + stopwatch.stop(TAG) + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java index 2e8edc1a68..80f01b6ac4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java @@ -126,9 +126,10 @@ public class ApplicationMigrations { static final int REBUILD_MESSAGE_FTS_INDEX_3 = 81; static final int TO_FROM_RECIPIENTS = 82; static final int REBUILD_MESSAGE_FTS_INDEX_4 = 83; + static final int INDEX_DATABASE_MIGRATION = 84; } - public static final int CURRENT_VERSION = 83; + public static final int CURRENT_VERSION = 84; /** * This *must* be called after the {@link JobManager} has been instantiated, but *before* the call @@ -562,6 +563,10 @@ public class ApplicationMigrations { jobs.put(Version.REBUILD_MESSAGE_FTS_INDEX_4, new RebuildMessageSearchIndexMigrationJob()); } + if (lastSeenVersion < Version.INDEX_DATABASE_MIGRATION) { + jobs.put(Version.INDEX_DATABASE_MIGRATION, new DatabaseMigrationJob()); + } + return jobs; }