Improve thread delete performance.

2025-12-24 13:08:46 +00:00 · 2024-10-02 10:19:34 -04:00
parent 93609106b0
commit 66e6b5506e
6 changed files with 183 additions and 35 deletions
--- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogTables.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogTables.kt
@@ -89,13 +89,16 @@ class MessageSendLogTables constructor(context: Context?, databaseHelper: Signal
      "CREATE INDEX msl_payload_date_sent_index ON $TABLE_NAME ($DATE_SENT)"
    )

+    const val AFTER_MESSAGE_DELETE_TRIGGER_NAME = "msl_message_delete"
+    const val AFTER_MESSAGE_DELETE_TRIGGER = """
+      CREATE TRIGGER $AFTER_MESSAGE_DELETE_TRIGGER_NAME AFTER DELETE ON ${MessageTable.TABLE_NAME} 
+      BEGIN 
+        DELETE FROM $TABLE_NAME WHERE $ID IN (SELECT ${MslMessageTable.PAYLOAD_ID} FROM ${MslMessageTable.TABLE_NAME} WHERE ${MslMessageTable.MESSAGE_ID} = old.${MessageTable.ID});
+      END
+    """
+
    val CREATE_TRIGGERS = arrayOf(
-      """
-        CREATE TRIGGER msl_message_delete AFTER DELETE ON ${MessageTable.TABLE_NAME} 
-        BEGIN 
-          DELETE FROM $TABLE_NAME WHERE $ID IN (SELECT ${MslMessageTable.PAYLOAD_ID} FROM ${MslMessageTable.TABLE_NAME} WHERE ${MslMessageTable.MESSAGE_ID} = old.${MessageTable.ID});
-        END
-      """,
+      AFTER_MESSAGE_DELETE_TRIGGER,
      """
        CREATE TRIGGER msl_attachment_delete AFTER DELETE ON ${AttachmentTable.TABLE_NAME}
        BEGIN
@@ -381,6 +384,41 @@ class MessageSendLogTables constructor(context: Context?, databaseHelper: Signal
    db.delete(MslPayloadTable.TABLE_NAME, query, args)
  }

+  /**
+   * Drop the trigger for updating the [MslPayloadTable] on message deletes. Should only be used for expected large deletes.
+   * The caller must be in a transaction and called with a matching [restoreAfterMessageDeleteTrigger] before the transaction
+   * completes.
+   *
+   * Note: The caller is not responsible for performing the missing trigger operations and they will be performed in
+   * [restoreAfterMessageDeleteTrigger].
+   */
+  fun dropAfterMessageDeleteTrigger() {
+    check(SignalDatabase.inTransaction)
+    writableDatabase.execSQL("DROP TRIGGER IF EXISTS ${MslPayloadTable.AFTER_MESSAGE_DELETE_TRIGGER_NAME}")
+  }
+
+  /**
+   * Restore the trigger for updating the [MslPayloadTable] on message deletes. Must only be called within the same transaction after calling
+   * [dropAfterMessageDeleteTrigger].
+   */
+  fun restoreAfterMessageDeleteTrigger() {
+    check(SignalDatabase.inTransaction)
+
+    val restoreDeleteMessagesOperation = """
+      DELETE FROM ${MslPayloadTable.TABLE_NAME} 
+      WHERE ${MslPayloadTable.TABLE_NAME}.${MslPayloadTable.ID} IN (
+        SELECT ${MslMessageTable.TABLE_NAME}.${MslMessageTable.PAYLOAD_ID} 
+        FROM ${MslMessageTable.TABLE_NAME} 
+        WHERE ${MslMessageTable.TABLE_NAME}.${MslMessageTable.MESSAGE_ID} NOT IN (
+          SELECT ${MessageTable.TABLE_NAME}.${MessageTable.ID} FROM ${MessageTable.TABLE_NAME}
+        )
+      )
+    """
+
+    writableDatabase.execSQL(restoreDeleteMessagesOperation)
+    writableDatabase.execSQL(MslPayloadTable.AFTER_MESSAGE_DELETE_TRIGGER)
+  }
+
  override fun remapRecipient(oldRecipientId: RecipientId, newRecipientId: RecipientId) {
    val values = ContentValues().apply {
      put(MslRecipientTable.RECIPIENT_ID, newRecipientId.serialize())
--- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageTable.kt
@@ -77,7 +77,6 @@ import org.thoughtcrime.securesms.database.SignalDatabase.Companion.distribution
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.groupReceipts
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.groups
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.mentions
-import org.thoughtcrime.securesms.database.SignalDatabase.Companion.messageLog
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.messages
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.reactions
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.recipients
@@ -2099,7 +2098,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat

      deletedAttachments = attachments.deleteAttachmentsForMessage(messageId)
      mentions.deleteMentionsForMessage(messageId)
-      messageLog.deleteAllRelatedToMessage(messageId)
+      SignalDatabase.messageLog.deleteAllRelatedToMessage(messageId)
      reactions.deleteReactions(MessageId(messageId))
      deleteGroupStoryReplies(messageId)
      disassociateStoryQuotes(messageId)
@@ -3456,17 +3455,62 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat

  fun deleteMessagesInThreadBeforeDate(threadId: Long, date: Long, inclusive: Boolean): Int {
    val condition = if (inclusive) "<=" else "<"
+    val extraWhere = "AND ${TABLE_NAME}.$DATE_RECEIVED $condition $date"

-    return writableDatabase
-      .delete(TABLE_NAME)
-      .where("$THREAD_ID = ? AND $DATE_RECEIVED $condition $date", threadId)
-      .run()
+    return deleteMessagesInThread(listOf(threadId), extraWhere)
  }

-  fun deleteAbandonedMessages(): Int {
+  fun deleteMessagesInThread(threadIds: Collection<Long>, extraWhere: String = ""): Int {
+    var deletedCount = 0
+
+    writableDatabase.withinTransaction { db ->
+      SignalDatabase.messageSearch.dropAfterMessageDeleteTrigger()
+      SignalDatabase.messageLog.dropAfterMessageDeleteTrigger()
+
+      for (threadId in threadIds) {
+        val subSelect = "SELECT ${TABLE_NAME}.$ID FROM $TABLE_NAME WHERE ${TABLE_NAME}.$THREAD_ID = $threadId $extraWhere"
+
+        // Bulk deleting FK tables for large message delete efficiency
+        db.delete(StorySendTable.TABLE_NAME)
+          .where("${StorySendTable.TABLE_NAME}.${StorySendTable.MESSAGE_ID} IN ($subSelect)")
+          .run()
+
+        db.delete(ReactionTable.TABLE_NAME)
+          .where("${ReactionTable.TABLE_NAME}.${ReactionTable.MESSAGE_ID} IN ($subSelect)")
+          .run()
+
+        db.delete(CallTable.TABLE_NAME)
+          .where("${CallTable.TABLE_NAME}.${CallTable.MESSAGE_ID} IN ($subSelect)")
+          .run()
+
+        // Must delete rows from FTS table before deleting from main table due to FTS requirement when deleting by rowid
+        db.delete(SearchTable.FTS_TABLE_NAME)
+          .where("${SearchTable.FTS_TABLE_NAME}.${SearchTable.ID} IN ($subSelect)")
+          .run()
+
+        // Actually delete messages
+        deletedCount += db.delete(TABLE_NAME)
+          .where("$THREAD_ID = ? $extraWhere", threadId)
+          .run()
+      }
+
+      SignalDatabase.messageSearch.restoreAfterMessageDeleteTrigger()
+      SignalDatabase.messageLog.restoreAfterMessageDeleteTrigger()
+    }
+
+    return deletedCount
+  }
+
+  fun deleteAbandonedMessages(threadId: Long? = null): Int {
+    val where = if (threadId == null) {
+      "$THREAD_ID NOT IN (SELECT ${ThreadTable.TABLE_NAME}.${ThreadTable.ID} FROM ${ThreadTable.TABLE_NAME} WHERE ${ThreadTable.ACTIVE} = 1)"
+    } else {
+      "$THREAD_ID = $threadId AND (SELECT ${ThreadTable.TABLE_NAME}.${ThreadTable.ACTIVE} FROM ${ThreadTable.TABLE_NAME} WHERE ${ThreadTable.TABLE_NAME}.${ThreadTable.ID} = $threadId) != 1"
+    }
+
    val deletes = writableDatabase
      .delete(TABLE_NAME)
-      .where("$THREAD_ID NOT IN (SELECT _id FROM ${ThreadTable.TABLE_NAME} WHERE ${ThreadTable.ACTIVE} = 1)")
+      .where(where)
      .run()

    if (deletes > 0) {
@@ -3494,7 +3538,7 @@ open class MessageTable(context: Context?, databaseHelper: SignalDatabase) : Dat
      .readToSingleLongOrNull()
  }

-  fun deleteMessages(messagesToDelete: List<MessageTable.SyncMessageId>): List<SyncMessageId> {
+  fun deleteMessages(messagesToDelete: List<SyncMessageId>): List<SyncMessageId> {
    val threads = mutableSetOf<Long>()
    val unhandled = mutableListOf<SyncMessageId>()

--- a/app/src/main/java/org/thoughtcrime/securesms/database/SearchTable.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/SearchTable.kt
@@ -46,6 +46,11 @@ class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
    private const val TRIGGER_AFTER_INSERT = "message_ai"
    private const val TRIGGER_AFTER_DELETE = "message_ad"
    private const val TRIGGER_AFTER_UPDATE = "message_au"
+    private const val AFTER_MESSAGE_DELETE_TRIGGER = """
+      CREATE TRIGGER $TRIGGER_AFTER_DELETE AFTER DELETE ON ${MessageTable.TABLE_NAME} BEGIN
+        INSERT INTO $FTS_TABLE_NAME($FTS_TABLE_NAME, $ID, $BODY, $THREAD_ID) VALUES('delete', old.${MessageTable.ID}, old.${MessageTable.BODY}, old.${MessageTable.THREAD_ID});
+      END;
+    """

    @Language("sql")
    val CREATE_TRIGGERS = arrayOf(
@@ -54,11 +59,7 @@ class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
          INSERT INTO $FTS_TABLE_NAME($ID, $BODY, $THREAD_ID) VALUES (new.${MessageTable.ID}, new.${MessageTable.BODY}, new.${MessageTable.THREAD_ID});
        END;
      """,
-      """
-        CREATE TRIGGER $TRIGGER_AFTER_DELETE AFTER DELETE ON ${MessageTable.TABLE_NAME} BEGIN
-          INSERT INTO $FTS_TABLE_NAME($FTS_TABLE_NAME, $ID, $BODY, $THREAD_ID) VALUES('delete', old.${MessageTable.ID}, old.${MessageTable.BODY}, old.${MessageTable.THREAD_ID});
-        END;
-      """,
+      AFTER_MESSAGE_DELETE_TRIGGER,
      """
        CREATE TRIGGER $TRIGGER_AFTER_UPDATE AFTER UPDATE ON ${MessageTable.TABLE_NAME} BEGIN
          INSERT INTO $FTS_TABLE_NAME($FTS_TABLE_NAME, $ID, $BODY, $THREAD_ID) VALUES('delete', old.${MessageTable.ID}, old.${MessageTable.BODY}, old.${MessageTable.THREAD_ID});
@@ -137,6 +138,25 @@ class SearchTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
    }
  }

+  /**
+   * Drop the trigger for updating the search table on deletes. Should only be used for expected large deletes.
+   * The caller must be in a transaction, update the search table manually before message deletes because of FTS indexing
+   * requirements, and be called with a matching [restoreAfterMessageDeleteTrigger] before the transaction completes.
+   */
+  fun dropAfterMessageDeleteTrigger() {
+    check(SignalDatabase.inTransaction)
+    writableDatabase.execSQL("DROP TRIGGER IF EXISTS $TRIGGER_AFTER_DELETE")
+  }
+
+  /**
+   * Restore the trigger for updating the search table on message deletes. Must only be called within the same transaction
+   * after calling [dropAfterMessageDeleteTrigger] and performing the dropped trigger's actions manually.
+   */
+  fun restoreAfterMessageDeleteTrigger() {
+    check(SignalDatabase.inTransaction)
+    writableDatabase.execSQL(AFTER_MESSAGE_DELETE_TRIGGER)
+  }
+
  /**
   * Re-adds every message to the index. It's fine to insert the same message twice; the table will naturally de-dupe.
   *
--- a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadTable.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadTable.kt
@@ -33,7 +33,6 @@ import org.signal.libsignal.zkgroup.groups.GroupMasterKey
 import org.thoughtcrime.securesms.conversationlist.model.ConversationFilter
 import org.thoughtcrime.securesms.database.MessageTable.MarkedMessageInfo
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.attachments
-import org.thoughtcrime.securesms.database.SignalDatabase.Companion.calls
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.drafts
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.groupReceipts
 import org.thoughtcrime.securesms.database.SignalDatabase.Companion.mentions
@@ -50,6 +49,7 @@ import org.thoughtcrime.securesms.database.model.serialize
 import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.groups.BadGroupIdException
 import org.thoughtcrime.securesms.groups.GroupId
+import org.thoughtcrime.securesms.jobs.DeleteAbandonedAttachmentsJob
 import org.thoughtcrime.securesms.jobs.MultiDeviceDeleteSyncJob
 import org.thoughtcrime.securesms.jobs.OptimizeMessageSearchIndexJob
 import org.thoughtcrime.securesms.keyvalue.SignalStore
@@ -345,17 +345,14 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
        }
      }

-    val deletes = writableDatabase.withinTransaction {
+    writableDatabase.withinTransaction {
      messages.deleteAbandonedMessages()
      attachments.trimAllAbandonedAttachments()
      groupReceipts.deleteAbandonedRows()
      mentions.deleteAbandonedMentions()
-      return@withinTransaction attachments.deleteAbandonedAttachmentFiles()
    }

-    if (deletes > 0) {
-      Log.i(TAG, "Trim all threads caused $deletes attachments to be deleted.")
-    }
+    DeleteAbandonedAttachmentsJob.enqueue()

    if (syncThreadTrimDeletes && threadTrimsToSync.isNotEmpty()) {
      MultiDeviceDeleteSyncJob.enqueueThreadDeletes(threadTrimsToSync, isFullDelete = false)
@@ -378,18 +375,15 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
    }

    var threadTrimToSync: ThreadDeleteSyncInfo? = null
-    val deletes = writableDatabase.withinTransaction {
+    writableDatabase.withinTransaction {
      threadTrimToSync = trimThreadInternal(threadId, syncThreadTrimDeletes, length, trimBeforeDate, inclusive)
-      messages.deleteAbandonedMessages()
+      messages.deleteAbandonedMessages(threadId)
      attachments.trimAllAbandonedAttachments()
      groupReceipts.deleteAbandonedRows()
      mentions.deleteAbandonedMentions()
-      return@withinTransaction attachments.deleteAbandonedAttachmentFiles()
    }

-    if (deletes > 0) {
-      Log.i(TAG, "Trim thread $threadId caused $deletes attachments to be deleted.")
-    }
+    DeleteAbandonedAttachmentsJob.enqueue()

    if (syncThreadTrimDeletes && threadTrimToSync != null) {
      MultiDeviceDeleteSyncJob.enqueueThreadDeletes(listOf(threadTrimToSync!!), isFullDelete = false)
@@ -1164,12 +1158,11 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
        db.deactivateThread(query)
      }

-      messages.deleteAbandonedMessages()
+      messages.deleteMessagesInThread(selectedConversations)
      attachments.trimAllAbandonedAttachments()
      groupReceipts.deleteAbandonedRows()
      mentions.deleteAbandonedMentions()
      drafts.clearDrafts(selectedConversations)
-      attachments.deleteAbandonedAttachmentFiles()
      synchronized(threadIdCache) {
        for (recipientId in recipientIds) {
          threadIdCache.remove(recipientId)
@@ -1177,6 +1170,8 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
      }
    }

+    DeleteAbandonedAttachmentsJob.enqueue()
+
    if (syncThreadDeletes) {
      MultiDeviceDeleteSyncJob.enqueueThreadDeletes(addressableMessages, isFullDelete = true)
    }
@@ -1199,7 +1194,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
      messages.deleteAllThreads()
      drafts.clearAllDrafts()
      db.deactivateThreads()
-      calls.deleteAllCalls()
+      SignalDatabase.calls.deleteAllCalls()
      synchronized(threadIdCache) {
        threadIdCache.clear()
      }
--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/DeleteAbandonedAttachmentsJob.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/DeleteAbandonedAttachmentsJob.kt
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2024 Signal Messenger, LLC
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+package org.thoughtcrime.securesms.jobs
+
+import org.signal.core.util.logging.Log
+import org.thoughtcrime.securesms.database.SignalDatabase.Companion.attachments
+import org.thoughtcrime.securesms.dependencies.AppDependencies
+import org.thoughtcrime.securesms.jobmanager.Job
+import kotlin.time.Duration.Companion.days
+
+/**
+ * Deletes attachment files that are no longer referenced in the database.
+ */
+class DeleteAbandonedAttachmentsJob private constructor(parameters: Parameters) : Job(parameters) {
+
+  companion object {
+    private val TAG = Log.tag(DeleteAbandonedAttachmentsJob::class)
+    const val KEY = "DeleteAbandonedAttachmentsJob"
+
+    fun enqueue() {
+      AppDependencies.jobManager.add(DeleteAbandonedAttachmentsJob())
+    }
+  }
+
+  constructor() : this(
+    parameters = Parameters.Builder()
+      .setMaxInstancesForFactory(2)
+      .setLifespan(1.days.inWholeMilliseconds)
+      .build()
+  )
+
+  override fun serialize(): ByteArray? = null
+  override fun getFactoryKey(): String = KEY
+  override fun onFailure() = Unit
+
+  override fun run(): Result {
+    val deletes = attachments.deleteAbandonedAttachmentFiles()
+    Log.i(TAG, "Deleted $deletes abandoned attachments.")
+    return Result.success()
+  }
+
+  class Factory : Job.Factory<DeleteAbandonedAttachmentsJob> {
+    override fun create(parameters: Parameters, serializedData: ByteArray?): DeleteAbandonedAttachmentsJob {
+      return DeleteAbandonedAttachmentsJob(parameters)
+    }
+  }
+}
--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java
@@ -140,6 +140,7 @@ public final class JobManagerFactories {
      put(ConversationShortcutUpdateJob.KEY,         new ConversationShortcutUpdateJob.Factory());
      put(CopyAttachmentToArchiveJob.KEY,            new CopyAttachmentToArchiveJob.Factory());
      put(CreateReleaseChannelJob.KEY,               new CreateReleaseChannelJob.Factory());
+      put(DeleteAbandonedAttachmentsJob.KEY,         new DeleteAbandonedAttachmentsJob.Factory());
      put(DirectoryRefreshJob.KEY,                   new DirectoryRefreshJob.Factory());
      put(DonationReceiptRedemptionJob.KEY,          new DonationReceiptRedemptionJob.Factory());
      put(DownloadLatestEmojiDataJob.KEY,            new DownloadLatestEmojiDataJob.Factory());