Optimize markAllCallHistoryReadSync

ts/sql/Server.ts

@@ -2253,7 +2253,7 @@ export function getAllSyncTasksSync(db: Database): Array<SyncTaskType> {
   })();
 }
 
-function saveMessageSync(
+export function saveMessageSync(
   db: Database,
   data: MessageType,
   options: {
@@ -3638,7 +3638,7 @@ async function clearCallHistory(
 ): Promise<ReadonlyArray<string>> {
   const db = await getWritableInstance();
   return db.transaction(() => {
-    const timestamp = getTimestampForCallLogEventTarget(db, target);
+    const timestamp = getMessageTimestampForCallLogEventTarget(db, target);
 
     const [selectCallIdsQuery, selectCallIdsParams] = sql`
       SELECT callsHistory.callId
@@ -3807,48 +3807,76 @@ async function markCallHistoryRead(callId: string): Promise<void> {
   db.prepare(query).run(params);
 }
 
-function getTimestampForCallLogEventTarget(
+function getMessageTimestampForCallLogEventTarget(
   db: Database,
   target: CallLogEventTarget
 ): number {
-  let { timestamp } = target;
+  let { callId, peerId } = target;
+  const { timestamp } = target;
 
-  if (target.peerId != null && target.callId != null) {
+  if (callId == null || peerId == null) {
+    const predicate =
+      peerId != null
+        ? sqlFragment`callsHistory.peerId IS ${target.peerId}`
+        : sqlFragment`TRUE`;
+
+    // Get the most recent call history timestamp for the target.timestamp
     const [selectQuery, selectParams] = sql`
-      SELECT callsHistory.timestamp
+      SELECT callsHistory.callId, callsHistory.peerId
       FROM callsHistory
-      WHERE callsHistory.callId IS ${target.callId}
-        AND callsHistory.peerId IS ${target.peerId}
+      WHERE ${predicate}
+        AND callsHistory.timestamp <= ${timestamp}
+      ORDER BY callsHistory.timestamp DESC
+      LIMIT 1
     `;
-    const value = db.prepare(selectQuery).pluck().get(selectParams);
 
-    if (value != null) {
-      timestamp = value;
-    } else {
-      log.warn(
-        'getTimestampForCallLogEventTarget: Target call not found',
-        target.callId
-      );
+    const row = db.prepare(selectQuery).get(selectParams);
+    if (row == null) {
+      log.warn('getTimestampForCallLogEventTarget: Target call not found');
+      return timestamp;
     }
+    callId = row.callId as string;
+    peerId = row.peerId as AciString;
   }
 
-  return timestamp;
+  const [selectQuery, selectParams] = sql`
+    SELECT messages.sent_at
+    FROM messages
+    WHERE messages.type IS 'call-history'
+      AND messages.conversationId IS ${peerId}
+      AND messages.callId IS ${callId}
+    LIMIT 1
+  `;
+
+  const messageTimestamp = db.prepare(selectQuery).pluck().get(selectParams);
+
+  if (messageTimestamp == null) {
+    log.warn(
+      'getTimestampForCallLogEventTarget: Target call message not found'
+    );
+  }
+
+  return messageTimestamp ?? target.timestamp;
 }
 
-async function markAllCallHistoryReadWithPredicate(
+export function markAllCallHistoryReadSync(
+  db: Database,
   target: CallLogEventTarget,
   inConversation: boolean
-) {
-  const db = await getWritableInstance();
+): void {
+  if (inConversation) {
+    strictAssert(target.peerId, 'peerId is required');
+  }
+
   db.transaction(() => {
     const jsonPatch = JSON.stringify({
       seenStatus: SeenStatus.Seen,
     });
 
-    const timestamp = getTimestampForCallLogEventTarget(db, target);
+    const timestamp = getMessageTimestampForCallLogEventTarget(db, target);
 
     const predicate = inConversation
-      ? sqlFragment`callsHistory.peerId IS ${target.peerId}`
+      ? sqlFragment`messages.conversationId IS ${target.peerId}`
       : sqlFragment`TRUE`;
 
     const [updateQuery, updateParams] = sql`
@@ -3856,14 +3884,10 @@ async function markAllCallHistoryReadWithPredicate(
       SET
         seenStatus = ${SEEN_STATUS_SEEN},
         json = json_patch(json, ${jsonPatch})
-      WHERE id IN (
-        SELECT id FROM messages
-        INNER JOIN callsHistory ON callsHistory.callId IS messages.callId
-        WHERE messages.type IS 'call-history'
-          AND messages.seenStatus IS ${SEEN_STATUS_UNSEEN}
-          AND callsHistory.timestamp <= ${timestamp}
-          AND ${predicate}
-      )
+      WHERE messages.type IS 'call-history'
+        AND ${predicate}
+        AND messages.seenStatus IS ${SEEN_STATUS_UNSEEN}
+        AND messages.sent_at <= ${timestamp}
     `;
 
     db.prepare(updateQuery).run(updateParams);
@@ -3873,14 +3897,16 @@ async function markAllCallHistoryReadWithPredicate(
 async function markAllCallHistoryRead(
   target: CallLogEventTarget
 ): Promise<void> {
-  await markAllCallHistoryReadWithPredicate(target, false);
+  const db = await getWritableInstance();
+  markAllCallHistoryReadSync(db, target, false);
 }
 
 async function markAllCallHistoryReadInConversation(
   target: CallLogEventTarget
 ): Promise<void> {
   strictAssert(target.peerId, 'peerId is required');
-  await markAllCallHistoryReadWithPredicate(target, true);
+  const db = await getWritableInstance();
+  markAllCallHistoryReadSync(db, target, true);
 }
 
 function getCallHistoryGroupDataSync(
@@ -4173,9 +4199,10 @@ async function getCallHistoryGroups(
     .reverse();
 }
 
-async function saveCallHistory(callHistory: CallHistoryDetails): Promise<void> {
-  const db = await getWritableInstance();
-
+export function saveCallHistorySync(
+  db: Database,
+  callHistory: CallHistoryDetails
+): void {
   const [insertQuery, insertParams] = sql`
     INSERT OR REPLACE INTO callsHistory (
       callId,
@@ -4201,6 +4228,11 @@ async function saveCallHistory(callHistory: CallHistoryDetails): Promise<void> {
   db.prepare(insertQuery).run(insertParams);
 }
 
+async function saveCallHistory(callHistory: CallHistoryDetails): Promise<void> {
+  const db = await getWritableInstance();
+  saveCallHistorySync(db, callHistory);
+}
+
 async function hasGroupCallHistoryMessage(
   conversationId: string,
   eraId: string

ts/sql/migrations/1100-optimize-mark-call-history-read-in-conversation.ts

@@ -0,0 +1,63 @@
+// Copyright 2024 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { Database } from '@signalapp/better-sqlite3';
+import type { LoggerType } from '../../types/Logging';
+import { sql } from '../util';
+
+export const version = 1100;
+
+export function updateToSchemaVersion1100(
+  currentVersion: number,
+  db: Database,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 1100) {
+    return;
+  }
+
+  db.transaction(() => {
+    const [query] = sql`
+      -- Fix: Query went from readStatus to seenStatus but index wasn't updated
+      DROP INDEX IF EXISTS messages_callHistory_readStatus;
+      DROP INDEX IF EXISTS messages_callHistory_seenStatus;
+      CREATE INDEX messages_callHistory_seenStatus
+        ON messages (type, seenStatus)
+        WHERE type IS 'call-history';
+
+      -- Update to index created in 89: add sent_at to make it covering, and where clause to make it smaller
+      DROP INDEX IF EXISTS messages_call;
+      CREATE INDEX messages_call ON messages
+        (type, conversationId, callId, sent_at)
+        WHERE type IS 'call-history';
+
+      -- Update to index created in 89: add callId and peerId to make it covering
+      DROP INDEX IF EXISTS callsHistory_order;
+      CREATE INDEX callsHistory_order ON callsHistory
+        (timestamp DESC, callId, peerId);
+
+      -- Update to index created in 89: add timestamp for querying by order and callId to make it covering
+      DROP INDEX IF EXISTS callsHistory_byConversation;
+      DROP INDEX IF EXISTS callsHistory_byConversation_order;
+      CREATE INDEX callsHistory_byConversation_order ON callsHistory (peerId, timestamp DESC, callId);
+
+      -- Optimize markAllCallHistoryRead
+      DROP INDEX IF EXISTS messages_callHistory_markReadBefore;
+      CREATE INDEX messages_callHistory_markReadBefore
+        ON messages (type, seenStatus, sent_at DESC)
+        WHERE type IS 'call-history';
+
+      -- Optimize markAllCallHistoryReadInConversation
+      DROP INDEX IF EXISTS messages_callHistory_markReadByConversationBefore;
+      CREATE INDEX messages_callHistory_markReadByConversationBefore
+        ON messages (type, conversationId, seenStatus, sent_at DESC)
+        WHERE type IS 'call-history';
+    `;
+
+    db.exec(query);
+  })();
+
+  db.pragma('user_version = 1100');
+
+  logger.info('updateToSchemaVersion1100: success!');
+}

ts/sql/migrations/index.ts

@@ -84,10 +84,11 @@ import { updateToSchemaVersion1050 } from './1050-group-send-endorsements';
 import { updateToSchemaVersion1060 } from './1060-addressable-messages-and-sync-tasks';
 import { updateToSchemaVersion1070 } from './1070-attachment-backup';
 import { updateToSchemaVersion1080 } from './1080-nondisappearing-addressable';
+import { updateToSchemaVersion1090 } from './1090-message-delete-indexes';
 import {
-  updateToSchemaVersion1090,
+  updateToSchemaVersion1100,
   version as MAX_VERSION,
-} from './1090-message-delete-indexes';
+} from './1100-optimize-mark-call-history-read-in-conversation';
 
 function updateToSchemaVersion1(
   currentVersion: number,
@@ -2040,6 +2041,7 @@ export const SCHEMA_VERSIONS = [
   updateToSchemaVersion1070,
   updateToSchemaVersion1080,
   updateToSchemaVersion1090,
+  updateToSchemaVersion1100,
 ];
 
 export class DBVersionFromFutureError extends Error {

ts/test-node/sql/migration_1100_test.ts

@@ -0,0 +1,81 @@
+// Copyright 2024 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import type { Database } from '@signalapp/better-sqlite3';
+import SQL from '@signalapp/better-sqlite3';
+import { findLast } from 'lodash';
+import { insertData, updateToVersion } from './helpers';
+import { markAllCallHistoryReadSync } from '../../sql/Server';
+import { SeenStatus } from '../../MessageSeenStatus';
+import { CallMode } from '../../types/Calling';
+import {
+  CallDirection,
+  CallType,
+  DirectCallStatus,
+} from '../../types/CallDisposition';
+import { strictAssert } from '../../util/assert';
+
+describe('SQL/updateToSchemaVersion1100', () => {
+  let db: Database;
+  beforeEach(() => {
+    db = new SQL(':memory:');
+    updateToVersion(db, 1100);
+  });
+
+  afterEach(() => {
+    db.close();
+  });
+
+  describe('Optimize markAllCallHistoryReadInConversation', () => {
+    it('is fast', () => {
+      const COUNT = 10_000;
+
+      const messages = Array.from({ length: COUNT }, (_, index) => {
+        return {
+          id: `test-message-${index}`,
+          type: 'call-history',
+          seenStatus: SeenStatus.Unseen,
+          conversationId: `test-conversation-${index % 30}`,
+          sent_at: index,
+          json: {
+            callId: `test-call-${index}`,
+          },
+        };
+      });
+
+      const callsHistory = Array.from({ length: COUNT }, (_, index) => {
+        return {
+          callId: `test-call-${index}`,
+          peerId: `test-conversation-${index % 30}`,
+          timestamp: index,
+          ringerId: null,
+          mode: CallMode.Direct,
+          type: CallType.Video,
+          direction: CallDirection.Incoming,
+          status: DirectCallStatus.Missed,
+        };
+      });
+
+      insertData(db, 'messages', messages);
+      insertData(db, 'callsHistory', callsHistory);
+
+      const latestCallInConversation = findLast(callsHistory, call => {
+        return call.peerId === 'test-conversation-0';
+      });
+
+      strictAssert(latestCallInConversation, 'missing latest call');
+
+      const target = {
+        timestamp: latestCallInConversation.timestamp,
+        callId: latestCallInConversation.callId,
+        peerId: latestCallInConversation.peerId,
+      };
+
+      const start = performance.now();
+      markAllCallHistoryReadSync(db, target, true);
+      const end = performance.now();
+      assert.isBelow(end - start, 50);
+    });
+  });
+});