From 01dda865381762f9608f5a44d2a6f7d982c49048 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:10:08 -0700 Subject: [PATCH] Optimize markAllCallHistoryReadSync --- ts/sql/Server.ts | 102 ++++++++++++------ ...-mark-call-history-read-in-conversation.ts | 63 +++++++++++ ts/sql/migrations/index.ts | 6 +- ts/test-node/sql/migration_1100_test.ts | 81 ++++++++++++++ 4 files changed, 215 insertions(+), 37 deletions(-) create mode 100644 ts/sql/migrations/1100-optimize-mark-call-history-read-in-conversation.ts create mode 100644 ts/test-node/sql/migration_1100_test.ts diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index 5731eb01b545..4104a7ecabf2 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -2253,7 +2253,7 @@ export function getAllSyncTasksSync(db: Database): Array { })(); } -function saveMessageSync( +export function saveMessageSync( db: Database, data: MessageType, options: { @@ -3638,7 +3638,7 @@ async function clearCallHistory( ): Promise> { 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 { 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 { - await markAllCallHistoryReadWithPredicate(target, false); + const db = await getWritableInstance(); + markAllCallHistoryReadSync(db, target, false); } async function markAllCallHistoryReadInConversation( target: CallLogEventTarget ): Promise { 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 { - 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 { db.prepare(insertQuery).run(insertParams); } +async function saveCallHistory(callHistory: CallHistoryDetails): Promise { + const db = await getWritableInstance(); + saveCallHistorySync(db, callHistory); +} + async function hasGroupCallHistoryMessage( conversationId: string, eraId: string diff --git a/ts/sql/migrations/1100-optimize-mark-call-history-read-in-conversation.ts b/ts/sql/migrations/1100-optimize-mark-call-history-read-in-conversation.ts new file mode 100644 index 000000000000..4cbb3cb02509 --- /dev/null +++ b/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!'); +} diff --git a/ts/sql/migrations/index.ts b/ts/sql/migrations/index.ts index f716fd4dc6b2..3a0a8db2c7e2 100644 --- a/ts/sql/migrations/index.ts +++ b/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 { diff --git a/ts/test-node/sql/migration_1100_test.ts b/ts/test-node/sql/migration_1100_test.ts new file mode 100644 index 000000000000..d1d71239bf47 --- /dev/null +++ b/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); + }); + }); +});