From 4dc1210eb5b34b3349792bc729611823fa5cf7e3 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Sat, 19 Jun 2021 10:34:44 +0100 Subject: [PATCH] All: Improved first sync speed when synchronising with Joplin Server --- .../support/serverPerformances/testPerf.sh | 4 +- packages/lib/Synchronizer.ts | 53 ++++++++++++++---- packages/lib/file-api-driver-joplinServer.ts | 7 ++- packages/lib/file-api-driver-memory.ts | 4 ++ packages/lib/file-api.ts | 16 ++++++ packages/server/schema.sqlite | Bin 290816 -> 290816 bytes packages/server/src/db.ts | 2 + .../20210618192423_jop_updated_time.ts | 29 ++++++++++ packages/server/src/models/ChangeModel.ts | 27 ++++++--- packages/server/src/models/ItemModel.ts | 3 + 10 files changed, 121 insertions(+), 24 deletions(-) create mode 100644 packages/server/src/migrations/20210618192423_jop_updated_time.ts diff --git a/packages/app-cli/tests/support/serverPerformances/testPerf.sh b/packages/app-cli/tests/support/serverPerformances/testPerf.sh index daffb2256..2ec011a5a 100755 --- a/packages/app-cli/tests/support/serverPerformances/testPerf.sh +++ b/packages/app-cli/tests/support/serverPerformances/testPerf.sh @@ -50,7 +50,7 @@ done cd "$ROOT_DIR/packages/app-cli" npm start -- --profile "$PROFILE_DIR" batch "$CMD_FILE" -npm start -- --profile "$PROFILE_DIR" import ~/Desktop/Joplin_17_06_2021.jex -# npm start -- --profile "$PROFILE_DIR" import ~/Desktop/Tout_18_06_2021.jex +# npm start -- --profile "$PROFILE_DIR" import ~/Desktop/Joplin_17_06_2021.jex +npm start -- --profile "$PROFILE_DIR" import ~/Desktop/Tout_18_06_2021.jex npm start -- --profile "$PROFILE_DIR" sync diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index c93dbcb39..319bfcee2 100644 --- a/packages/lib/Synchronizer.ts +++ b/packages/lib/Synchronizer.ts @@ -20,6 +20,7 @@ import JoplinError from './JoplinError'; import ShareService from './services/share/ShareService'; import TaskQueue from './TaskQueue'; import ItemUploader from './services/synchronizer/ItemUploader'; +import { FileApi } from './file-api'; const { sprintf } = require('sprintf-js'); const { Dirnames } = require('./services/synchronizer/utils/types'); @@ -27,6 +28,18 @@ interface RemoteItem { id: string; path?: string; type_?: number; + isDeleted?: boolean; + + // This the time when the file was created on the server. It is used for + // example for the locking mechanim or any file that's not an actual Joplin + // item. + updated_time?: number; + + // This is the time that corresponds to the actual Joplin item updated_time + // value. A note is always uploaded with a delay so the server updated_time + // value will always be ahead. However for synchronising we need to know the + // exact Joplin item updated_time value. + jop_updated_time?: number; } function isCannotSyncError(error: any): boolean { @@ -50,7 +63,7 @@ export default class Synchronizer { public static verboseMode: boolean = true; private db_: any; - private api_: any; + private api_: FileApi; private appType_: string; private logger_: Logger = new Logger(); private state_: string = 'idle'; @@ -74,7 +87,7 @@ export default class Synchronizer { public dispatch: Function; - public constructor(db: any, api: any, appType: string) { + public constructor(db: any, api: FileApi, appType: string) { this.db_ = db; this.api_ = api; this.appType_ = appType; @@ -307,7 +320,7 @@ export default class Synchronizer { if (this.syncTargetIsLocked_) throw new JoplinError('Sync target is locked - aborting API call', 'lockError'); try { - const output = await this.api()[fnName](...args); + const output = await (this.api() as any)[fnName](...args); return output; } catch (error) { const lockStatus = await this.lockErrorStatus_(); @@ -769,16 +782,27 @@ export default class Synchronizer { logger: this.logger(), }); - const remotes = listResult.items; + const remotes: RemoteItem[] = listResult.items; this.logSyncOperation('fetchingTotal', null, null, 'Fetching delta items from sync target', remotes.length); + const remoteIds = remotes.map(r => BaseItem.pathToId(r.path)); + const locals = await BaseItem.loadItemsByIds(remoteIds); + for (const remote of remotes) { if (this.cancelling()) break; - this.downloadQueue_.push(remote.path, async () => { - return this.apiCall('get', remote.path); - }); + let needsToDownload = true; + if (this.api().supportsAccurateTimestamp) { + const local = locals.find(l => l.id === BaseItem.pathToId(remote.path)); + if (local && local.updated_time === remote.jop_updated_time) needsToDownload = false; + } + + if (needsToDownload) { + this.downloadQueue_.push(remote.path, async () => { + return this.apiCall('get', remote.path); + }); + } } for (let i = 0; i < remotes.length; i++) { @@ -800,9 +824,10 @@ export default class Synchronizer { }; const path = remote.path; + const remoteId = BaseItem.pathToId(path); let action = null; let reason = ''; - let local = await BaseItem.loadItemByPath(path); + let local = locals.find(l => l.id === remoteId); let ItemClass = null; let content = null; @@ -821,10 +846,14 @@ export default class Synchronizer { action = 'deleteLocal'; reason = 'remote has been deleted'; } else { - content = await loadContent(); - if (content && content.updated_time > local.updated_time) { - action = 'updateLocal'; - reason = 'remote is more recent than local'; + if (this.api().supportsAccurateTimestamp && remote.jop_updated_time === local.updated_time) { + // Nothing to do, and no need to fetch the content + } else { + content = await loadContent(); + if (content && content.updated_time > local.updated_time) { + action = 'updateLocal'; + reason = 'remote is more recent than local'; + } } } } diff --git a/packages/lib/file-api-driver-joplinServer.ts b/packages/lib/file-api-driver-joplinServer.ts index 97e32b8bb..2f75526d7 100644 --- a/packages/lib/file-api-driver-joplinServer.ts +++ b/packages/lib/file-api-driver-joplinServer.ts @@ -36,6 +36,10 @@ export default class FileApiDriverJoplinServer { return true; } + public get supportsAccurateTimestamp() { + return true; + } + public requestRepeatCount() { return 3; } @@ -44,7 +48,8 @@ export default class FileApiDriverJoplinServer { const output = { path: rootPath ? path.substr(rootPath.length + 1) : path, updated_time: md.updated_time, - isDir: false, // !!md.is_directory, + jop_updated_time: md.jop_updated_time, + isDir: false, isDeleted: isDeleted, }; diff --git a/packages/lib/file-api-driver-memory.ts b/packages/lib/file-api-driver-memory.ts index b0886b740..d71f83caa 100644 --- a/packages/lib/file-api-driver-memory.ts +++ b/packages/lib/file-api-driver-memory.ts @@ -24,6 +24,10 @@ export default class FileApiDriverMemory { return true; } + public get supportsAccurateTimestamp() { + return true; + } + decodeContent_(content: any) { return Buffer.from(content, 'base64').toString('utf-8'); } diff --git a/packages/lib/file-api.ts b/packages/lib/file-api.ts index 64c7a2459..edd34cead 100644 --- a/packages/lib/file-api.ts +++ b/packages/lib/file-api.ts @@ -86,10 +86,26 @@ class FileApi { if (this.driver_.initialize) return this.driver_.initialize(this.fullPath('')); } + // This can be true if the driver implements uploading items in batch. Will + // probably only be supported by Joplin Server. public get supportsMultiPut(): boolean { return !!this.driver().supportsMultiPut; } + // This can be true when the sync target timestamps (updated_time) provided + // in the delta call are guaranteed to be accurate. That requires + // explicitely setting the timestamp, which is not done anymore on any sync + // target as it wasn't accurate (for example, the file system can't be + // relied on, and even OneDrive for some reason doesn't guarantee that the + // timestamp you set is what you get back). + // + // The only reliable one at the moment is Joplin Server since it reads the + // updated_time property directly from the item (it unserializes it + // server-side). + public get supportsAccurateTimestamp(): boolean { + return !!this.driver().supportsAccurateTimestamp; + } + async fetchRemoteDateOffset_() { const tempFile = `${this.tempDirName()}/timeCheck${Math.round(Math.random() * 1000000)}.txt`; const startTime = Date.now(); diff --git a/packages/server/schema.sqlite b/packages/server/schema.sqlite index d55c6b77619aeeaaf62f820469e4b6f57dd259e0..21ee0112c3d6653b22277bcee6bcbc44bd2f26ef 100644 GIT binary patch delta 995 zcmZp8AlUFgaDud84g&*&I~213>Ewwz#*8@|6PEBR@bT_o;J?kkp1+GfiQk-`hwlO3 zTE14k1U^$f9^U7?J2o2%OySknf{7Di0tIat#Zm>vF^!h z<@CU;j>!+?)WNK_$vpB99Wj&5p{&TsN%9aKA(Ol0A*OguUM~;P;Xe5`RLpsk!ZUt0 zK5<57Q_jf~_(V1tw8+?WSXQUrsQVk zIppLgC+1}27ni6QC8`)2sD$U6WJQH}6hyj3S-5-pWEdrtxtseZ8k(h;CHv-DMh5#= zS>{A0XL(h*d-!-Hm3bKZMR+@ci~@zlqYn_0i-BP?bHH7G1wxM46!3swo`9mw|MP42 zCnT08dYK28xfmN)6$gjqIlE>g75ip~x|LbFSA_domJ=`}jgbDy_N)>-$bJAhO@WXu zq7-clU?jq*Z3&D!@)Znole5iTEh-D$41*#)6QiP%EUQA3{oS&%Ly{A-LldXpIL|1{ zjTF=%7g!L|#mlL}AjbHUfun=HfZdYqIGZ}_5mtYm&D$j&F!FFR@x*Ued&y|Y49YAa z)05vY25_~c7_p0si!*krZ_j+gD9I$4fS&xPZ`{l%Gx-85%k;Ak8HJ}m2xm^*Uj3di nRe+!$7ELf%v_N6g0&aor+zCuq;|V2z_W%2tw*TMH%(wvnD71c&6i~|kDLma#WdMeP7TEJ`9C>94lL{QXL75Y zGFa@>S=QXN4?F@k)n2_d-{7&bEp+~rpw7Q)RD#3&7 z2awYg2oII5~Bz1ZKgpvpHY#VN< zjF!w?EyhOd;^N|r9j@E=zG0MPntt{nqp&kDLol?6dzvyfnrG&vq*i3*7vyB-#iwQF zq{b)b=ar=9l@!Nkro@-#WfqpEVv{wTUKqnHs*6nup)o-rKTjb6u4ej!aOTAA)$bWo n1qg=9q6r3z7AS05z%8(yJAvtHJfS4j{(nEy_W%2t88-j`$JuPe diff --git a/packages/server/src/db.ts b/packages/server/src/db.ts index dec149643..a6954a6c7 100644 --- a/packages/server/src/db.ts +++ b/packages/server/src/db.ts @@ -356,6 +356,7 @@ export interface Item extends WithDates, WithUuid { jop_share_id?: Uuid; jop_type?: number; jop_encryption_applied?: number; + jop_updated_time?: number; } export interface UserItem extends WithDates { @@ -503,6 +504,7 @@ export const databaseSchema: DatabaseTables = { jop_share_id: { type: 'string' }, jop_type: { type: 'number' }, jop_encryption_applied: { type: 'number' }, + jop_updated_time: { type: 'number' }, }, user_items: { id: { type: 'number' }, diff --git a/packages/server/src/migrations/20210618192423_jop_updated_time.ts b/packages/server/src/migrations/20210618192423_jop_updated_time.ts new file mode 100644 index 000000000..939703322 --- /dev/null +++ b/packages/server/src/migrations/20210618192423_jop_updated_time.ts @@ -0,0 +1,29 @@ +import { Knex } from 'knex'; +import { DbConnection } from '../db'; + +export async function up(db: DbConnection): Promise { + await db.schema.alterTable('items', function(table: Knex.CreateTableBuilder) { + table.integer('jop_updated_time').defaultTo(0).notNullable(); + }); + + while (true) { + const items = await db('items') + .select('id', 'content') + .where('jop_type', '>', 0) + .andWhere('jop_updated_time', '=', 0) + .limit(1000); + + if (!items.length) break; + + await db.transaction(async trx => { + for (const item of items) { + const unserialized = JSON.parse(item.content); + await trx('items').update({ jop_updated_time: unserialized.updated_time }).where('id', '=', item.id); + } + }); + } +} + +export async function down(_db: DbConnection): Promise { + +} diff --git a/packages/server/src/models/ChangeModel.ts b/packages/server/src/models/ChangeModel.ts index 351b4cedf..1c165952c 100644 --- a/packages/server/src/models/ChangeModel.ts +++ b/packages/server/src/models/ChangeModel.ts @@ -5,14 +5,12 @@ import { ErrorResyncRequired } from '../utils/errors'; import BaseModel, { SaveOptions } from './BaseModel'; import { PaginatedResults, Pagination, PaginationOrderDir } from './utils/pagination'; -export interface ChangeWithItem { - item: Item; - updated_time: number; - type: ChangeType; +export interface DeltaChange extends Change { + jop_updated_time?: number; } export interface PaginatedChanges extends PaginatedResults { - items: Change[]; + items: DeltaChange[]; } export interface ChangePagination { @@ -158,9 +156,20 @@ export default class ChangeModel extends BaseModel { .orderBy('counter', 'asc') .limit(pagination.limit) as any[]; - const changes = await query; + const changes: Change[] = await query; - const finalChanges = await this.removeDeletedItems(this.compressChanges(changes)); + const items: Item[] = await this.db('items').select('id', 'jop_updated_time').whereIn('items.id', changes.map(c => c.item_id)); + + let finalChanges: DeltaChange[] = this.compressChanges(changes); + finalChanges = await this.removeDeletedItems(finalChanges, items); + finalChanges = finalChanges.map(c => { + const item = items.find(item => item.id === c.item_id); + if (!item) return c; + return { + ...c, + jop_updated_time: item.jop_updated_time, + }; + }); return { items: finalChanges, @@ -171,14 +180,14 @@ export default class ChangeModel extends BaseModel { }; } - private async removeDeletedItems(changes: Change[]): Promise { + private async removeDeletedItems(changes: Change[], items: Item[] = null): Promise { const itemIds = changes.map(c => c.item_id); // We skip permission check here because, when an item is shared, we need // to fetch files that don't belong to the current user. This check // would not be needed anyway because the change items are generated in // a context where permissions have already been checked. - const items: Item[] = await this.db('items').select('id').whereIn('items.id', itemIds); + items = items === null ? await this.db('items').select('id').whereIn('items.id', itemIds) : items; const output: Change[] = []; diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index 9fd259fb1..ba05fa58e 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -285,6 +285,7 @@ export default class ItemModel extends BaseModel { item.share_id = itemRow.jop_share_id; item.type_ = itemRow.jop_type; item.encryption_applied = itemRow.jop_encryption_applied; + item.updated_time = itemRow.jop_updated_time; return item; } @@ -336,6 +337,7 @@ export default class ItemModel extends BaseModel { item.jop_type = joplinItem.type_; item.jop_encryption_applied = joplinItem.encryption_applied || 0; item.jop_share_id = joplinItem.share_id || ''; + item.jop_updated_time = joplinItem.updated_time; const joplinItemToSave = { ...joplinItem }; @@ -344,6 +346,7 @@ export default class ItemModel extends BaseModel { delete joplinItemToSave.share_id; delete joplinItemToSave.type_; delete joplinItemToSave.encryption_applied; + delete joplinItemToSave.updated_time; item.content = Buffer.from(JSON.stringify(joplinItemToSave)); } else {