Skip to content

Commit 481acb2

Browse files
authored
Merge pull request #1638 from matrix-org/jryans/rm-olm-profiling
Remove detailed Olm session logging
2 parents e6a21cc + 6830921 commit 481acb2

File tree

3 files changed

+20
-33
lines changed

3 files changed

+20
-33
lines changed

src/crypto/olmlib.js

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,8 @@ export async function ensureOlmSessionsForDevices(
213213
// synchronous operation, as otherwise it is possible to have deadlocks
214214
// where multiple tasks wait indefinitely on another task to update some set
215215
// of common devices.
216-
for (const [userId, devices] of Object.entries(devicesByUser)) {
216+
for (const [, devices] of Object.entries(devicesByUser)) {
217217
for (const deviceInfo of devices) {
218-
const deviceId = deviceInfo.deviceId;
219218
const key = deviceInfo.getIdentityKey();
220219

221220
if (key === olmDevice.deviceCurve25519Key) {
@@ -224,15 +223,12 @@ export async function ensureOlmSessionsForDevices(
224223
continue;
225224
}
226225

227-
const forWhom = `for ${key} (${userId}:${deviceId})`;
228226
if (!olmDevice._sessionsInProgress[key]) {
229227
// pre-emptively mark the session as in-progress to avoid race
230228
// conditions. If we find that we already have a session, then
231229
// we'll resolve
232-
log.debug(`Marking Olm session in progress ${forWhom}`);
233230
olmDevice._sessionsInProgress[key] = new Promise(resolve => {
234231
resolveSession[key] = (...args) => {
235-
log.debug(`Resolved Olm session in progress ${forWhom}`);
236232
delete olmDevice._sessionsInProgress[key];
237233
resolve(...args);
238234
};
@@ -266,11 +262,9 @@ export async function ensureOlmSessionsForDevices(
266262
}
267263

268264
const forWhom = `for ${key} (${userId}:${deviceId})`;
269-
log.debug(`Ensuring Olm session ${forWhom}`);
270265
const sessionId = await olmDevice.getSessionIdForDevice(
271266
key, resolveSession[key], log,
272267
);
273-
log.debug(`Got Olm session ${sessionId} ${forWhom}`);
274268
if (sessionId !== null && resolveSession[key]) {
275269
// we found a session, but we had marked the session as
276270
// in-progress, so resolve it now, which will unmark it and
@@ -299,18 +293,6 @@ export async function ensureOlmSessionsForDevices(
299293
const oneTimeKeyAlgorithm = "signed_curve25519";
300294
let res;
301295
let taskDetail = `one-time keys for ${devicesWithoutSession.length} devices`;
302-
// If your homeserver takes a nap here and never replies, this process
303-
// would hang indefinitely. While that's easily fixed by setting a
304-
// timeout on this request, let's first log whether that's the root
305-
// cause we're seeing in practice.
306-
// See also https://github.com/vector-im/element-web/issues/16194
307-
let otkTimeoutLogger;
308-
// XXX: Perhaps there should be a default timeout?
309-
if (otkTimeout) {
310-
otkTimeoutLogger = setTimeout(() => {
311-
log.error(`Homeserver never replied while claiming ${taskDetail}`);
312-
}, otkTimeout);
313-
}
314296
try {
315297
log.debug(`Claiming ${taskDetail}`);
316298
res = await baseApis.claimOneTimeKeys(
@@ -323,8 +305,6 @@ export async function ensureOlmSessionsForDevices(
323305
}
324306
log.log(`Failed to claim ${taskDetail}`, e, devicesWithoutSession);
325307
throw e;
326-
} finally {
327-
clearTimeout(otkTimeoutLogger);
328308
}
329309

330310
if (failedServers && "failures" in res) {

src/crypto/store/indexeddb-crypto-store-backend.js

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {logger} from '../../logger';
2020
import * as utils from "../../utils";
2121

2222
export const VERSION = 9;
23+
const PROFILE_TRANSACTIONS = false;
2324

2425
/**
2526
* Implementation of a CryptoStore which is backed by an existing
@@ -759,20 +760,26 @@ export class Backend {
759760
}
760761

761762
doTxn(mode, stores, func, log = logger) {
762-
const txnId = this._nextTxnId++;
763-
const startTime = Date.now();
764-
const description = `${mode} crypto store transaction ${txnId} in ${stores}`;
765-
log.debug(`Starting ${description}`);
763+
let startTime;
764+
let description;
765+
if (PROFILE_TRANSACTIONS) {
766+
const txnId = this._nextTxnId++;
767+
startTime = Date.now();
768+
description = `${mode} crypto store transaction ${txnId} in ${stores}`;
769+
log.debug(`Starting ${description}`);
770+
}
766771
const txn = this._db.transaction(stores, mode);
767772
const promise = promiseifyTxn(txn);
768773
const result = func(txn);
769-
promise.then(() => {
770-
const elapsedTime = Date.now() - startTime;
771-
log.debug(`Finished ${description}, took ${elapsedTime} ms`);
772-
}, () => {
773-
const elapsedTime = Date.now() - startTime;
774-
log.error(`Failed ${description}, took ${elapsedTime} ms`);
775-
});
774+
if (PROFILE_TRANSACTIONS) {
775+
promise.then(() => {
776+
const elapsedTime = Date.now() - startTime;
777+
log.debug(`Finished ${description}, took ${elapsedTime} ms`);
778+
}, () => {
779+
const elapsedTime = Date.now() - startTime;
780+
log.error(`Failed ${description}, took ${elapsedTime} ms`);
781+
});
782+
}
776783
return promise.then(() => {
777784
return result;
778785
});

src/store/indexeddb-local-backend.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ LocalIndexedDBStoreBackend.prototype = {
435435
* @return {Promise} Resolves if the data was persisted.
436436
*/
437437
_persistSyncData: function(nextBatch, roomsData, groupsData) {
438-
logger.log("Persisting sync data up to ", nextBatch);
438+
logger.log("Persisting sync data up to", nextBatch);
439439
return utils.promiseTry(() => {
440440
const txn = this.db.transaction(["sync"], "readwrite");
441441
const store = txn.objectStore("sync");

0 commit comments

Comments
 (0)