self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2025-10-31 00:07:48 +02:00
Code Issues Releases Activity

Desktop, Cli, Mobile: Remove the need for sync locks (#11377)

Browse Source
This commit is contained in:
Laurent Cozic
2024-11-12 19:02:44 +00:00
committed by GitHub
parent 388701015d
commit 6c0258e8a6
4 changed files with 171 additions and 182 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
packages/lib/Synchronizer.ts
View File

@@ -131,7 +131,6 @@ export default class Synchronizer {
public lockHandler() {
if (this.lockHandler_) return this.lockHandler_;
this.lockHandler_ = new LockHandler(this.api());
this.lockHandler_.enabled = Setting.value('featureFlag.syncLockEnabled');
return this.lockHandler_;
}

11
packages/lib/models/settings/builtInMetadata.ts
View File

@@ -1610,17 +1610,6 @@ const builtInMetadata = (Setting: typeof SettingType) => {
isGlobal: true,
},
'featureFlag.syncLockEnabled': {
value: true,
type: SettingItemType.Bool,
public: true,
storage: SettingStorage.File,
label: () => 'Enable sync locks',
description: () => 'This is an experimental setting to disable sync locks',
section: 'sync',
isGlobal: true,
},
'featureFlag.linuxKeychain': {
value: false,
type: SettingItemType.Bool,

18
packages/lib/services/synchronizer/LockHandler.ts
View File

@@ -150,7 +150,7 @@ export default class LockHandler {
private refreshTimers_: RefreshTimers = {};
private autoRefreshInterval_: number = 1000 * 60;
private lockTtl_: number = defaultLockTtl;
private enabled_ = true;
private enabled_ = false;
public constructor(api: FileApi, options: LockHandlerOptions = null) {
if (!options) options = {};
@@ -206,6 +206,8 @@ export default class LockHandler {
public async locks(lockType: LockType = null): Promise<Lock[]> {
if (!this.enabled) return [];
if (this.enabled) throw new Error('Lock handler is enabled');
if (this.useBuiltInLocks) {
const locks = (await this.api_.listLocks()).items;
return locks;
@@ -227,6 +229,8 @@ export default class LockHandler {
}
private async saveLock(lock: Lock) {
if (!this.enabled) return;
if (this.enabled) throw new Error('Lock handler is enabled');
await this.api_.put(this.lockFilePath(lock), JSON.stringify(lock));
}
@@ -285,6 +289,10 @@ export default class LockHandler {
}
private async acquireExclusiveLock(clientType: LockClientType, clientId: string, options: AcquireLockOptions = null): Promise<Lock> {
if (!this.enabled) return nullLock();
if (this.enabled) throw new Error('Lock handler is enabled');
if (this.useBuiltInLocks) return this.api_.acquireLock(LockType.Exclusive, clientType, clientId);
// The logic to acquire an exclusive lock, while avoiding race conditions is as follow:
@@ -375,6 +383,8 @@ export default class LockHandler {
public startAutoLockRefresh(lock: Lock, errorHandler: Function): string {
if (!this.enabled) return '';
if (this.enabled) throw new Error('Lock handler is enabled');
const handle = this.autoLockRefreshHandle(lock);
if (this.refreshTimers_[handle]) {
throw new Error(`There is already a timer refreshing this lock: ${handle}`);
@@ -434,6 +444,8 @@ export default class LockHandler {
public stopAutoLockRefresh(lock: Lock) {
if (!this.enabled) return;
if (this.enabled) throw new Error('Lock handler is enabled');
const handle = this.autoLockRefreshHandle(lock);
if (!this.refreshTimers_[handle]) {
// Should not throw an error because lock may have been cleared in startAutoLockRefresh
@@ -449,6 +461,8 @@ export default class LockHandler {
public async acquireLock(lockType: LockType, clientType: LockClientType, clientId: string, options: AcquireLockOptions = null): Promise<Lock> {
if (!this.enabled) return nullLock();
if (this.enabled) throw new Error('Lock handler is enabled');
options = {
...defaultAcquireLockOptions(),
...options,
@@ -466,6 +480,8 @@ export default class LockHandler {
public async releaseLock(lockType: LockType, clientType: LockClientType, clientId: string) {
if (!this.enabled) return;
if (this.enabled) throw new Error('Lock handler is enabled');
if (this.useBuiltInLocks) {
await this.api_.releaseLock(lockType, clientType, clientId);
return;

323
packages/lib/services/synchronizer/synchronizer_LockHandler.test.ts
View File

@@ -1,196 +1,181 @@
import LockHandler, { LockType, LockHandlerOptions, Lock, activeLock, LockClientType } from '../../services/synchronizer/LockHandler';
import { isNetworkSyncTarget, fileApi, setupDatabaseAndSynchronizer, synchronizer, switchClient, msleep, expectThrow, expectNotThrow } from '../../testing/test-utils';
// import LockHandler, { LockType, LockHandlerOptions, Lock, activeLock, LockClientType } from '../../services/synchronizer/LockHandler';
// import { isNetworkSyncTarget, fileApi, setupDatabaseAndSynchronizer, synchronizer, switchClient, msleep, expectThrow, expectNotThrow } from '../../testing/test-utils';
// For tests with memory of file system we can use low intervals to make the tests faster.
// However if we use such low values with network sync targets, some calls might randomly fail with
// ECONNRESET and similar errors (Dropbox or OneDrive migth also throttle). Also we can't use a
// low lock TTL value because the lock might expire between the time it's written and the time it's checked.
// For that reason we add this multiplier for non-memory sync targets.
const timeoutMultiplier = isNetworkSyncTarget() ? 100 : 1;
// // For tests with memory of file system we can use low intervals to make the tests faster.
// // However if we use such low values with network sync targets, some calls might randomly fail with
// // ECONNRESET and similar errors (Dropbox or OneDrive migth also throttle). Also we can't use a
// // low lock TTL value because the lock might expire between the time it's written and the time it's checked.
// // For that reason we add this multiplier for non-memory sync targets.
// const timeoutMultiplier = isNetworkSyncTarget() ? 100 : 1;
let lockHandler_: LockHandler = null;
// let lockHandler_: LockHandler = null;
function newLockHandler(options: LockHandlerOptions = null): LockHandler {
return new LockHandler(fileApi(), options);
}
// function newLockHandler(options: LockHandlerOptions = null): LockHandler {
// return new LockHandler(fileApi(), options);
// }
function lockHandler(): LockHandler {
if (lockHandler_) return lockHandler_;
lockHandler_ = new LockHandler(fileApi());
return lockHandler_;
}
// function lockHandler(): LockHandler {
// if (lockHandler_) return lockHandler_;
// lockHandler_ = new LockHandler(fileApi());
// return lockHandler_;
// }
describe('synchronizer_LockHandler', () => {
beforeEach(async () => {
// logger.setLevel(Logger.LEVEL_WARN);
lockHandler_ = null;
await setupDatabaseAndSynchronizer(1);
await setupDatabaseAndSynchronizer(2);
await switchClient(1);
await synchronizer().start(); // Need to sync once to setup the sync target and allow locks to work
// logger.setLevel(Logger.LEVEL_DEBUG);
it('should be disabled', () => {
expect(true).toBe(true);
});
it('should acquire and release a sync lock', (async () => {
await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '123456');
const locks = await lockHandler().locks(LockType.Sync);
expect(locks.length).toBe(1);
expect(locks[0].type).toBe(LockType.Sync);
expect(locks[0].clientId).toBe('123456');
expect(locks[0].clientType).toBe(LockClientType.Mobile);
// beforeEach(async () => {
// lockHandler_ = null;
// await setupDatabaseAndSynchronizer(1);
// await setupDatabaseAndSynchronizer(2);
// await switchClient(1);
// await synchronizer().start(); // Need to sync once to setup the sync target and allow locks to work
// });
await lockHandler().releaseLock(LockType.Sync, LockClientType.Mobile, '123456');
expect((await lockHandler().locks(LockType.Sync)).length).toBe(0);
}));
// it('should acquire and release a sync lock', (async () => {
// await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '123456');
// const locks = await lockHandler().locks(LockType.Sync);
// expect(locks.length).toBe(1);
// expect(locks[0].type).toBe(LockType.Sync);
// expect(locks[0].clientId).toBe('123456');
// expect(locks[0].clientType).toBe(LockClientType.Mobile);
it('should not use files that are not locks', (async () => {
if (lockHandler().useBuiltInLocks) return; // Doesn't make sense with built-in locks
// await lockHandler().releaseLock(LockType.Sync, LockClientType.Mobile, '123456');
// expect((await lockHandler().locks(LockType.Sync)).length).toBe(0);
// }));
// Note: desktop.ini is blocked by Dropbox
await fileApi().put('locks/desktop.test.ini', 'a');
await fileApi().put('locks/exclusive.json', 'a');
await fileApi().put('locks/garbage.json', 'a');
await fileApi().put('locks/1_2_72c4d1b7253a4475bfb2f977117d26ed.json', 'a');
// it('should not use files that are not locks', (async () => {
// if (lockHandler().useBuiltInLocks) return; // Doesn't make sense with built-in locks
// Check that it doesn't cause an error if it fetches an old style lock
await fileApi().put('locks/sync_desktop_82c4d1b7253a4475bfb2f977117d26ed.json', 'a');
// // Note: desktop.ini is blocked by Dropbox
// await fileApi().put('locks/desktop.test.ini', 'a');
// await fileApi().put('locks/exclusive.json', 'a');
// await fileApi().put('locks/garbage.json', 'a');
// await fileApi().put('locks/1_2_72c4d1b7253a4475bfb2f977117d26ed.json', 'a');
const locks = await lockHandler().locks(LockType.Sync);
expect(locks.length).toBe(1);
expect(locks[0].type).toBe(LockType.Sync);
expect(locks[0].clientType).toBe(LockClientType.Mobile);
expect(locks[0].clientId).toBe('72c4d1b7253a4475bfb2f977117d26ed');
}));
// // Check that it doesn't cause an error if it fetches an old style lock
// await fileApi().put('locks/sync_desktop_82c4d1b7253a4475bfb2f977117d26ed.json', 'a');
it('should allow multiple sync locks', (async () => {
await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '111');
// const locks = await lockHandler().locks(LockType.Sync);
// expect(locks.length).toBe(1);
// expect(locks[0].type).toBe(LockType.Sync);
// expect(locks[0].clientType).toBe(LockClientType.Mobile);
// expect(locks[0].clientId).toBe('72c4d1b7253a4475bfb2f977117d26ed');
// }));
await switchClient(2);
// it('should allow multiple sync locks', (async () => {
// await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '111');
await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '222');
// await switchClient(2);
expect((await lockHandler().locks(LockType.Sync)).length).toBe(2);
// await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '222');
{
await lockHandler().releaseLock(LockType.Sync, LockClientType.Mobile, '222');
const locks = await lockHandler().locks(LockType.Sync);
expect(locks.length).toBe(1);
expect(locks[0].clientId).toBe('111');
}
}));
// expect((await lockHandler().locks(LockType.Sync)).length).toBe(2);
it('should auto-refresh a lock', (async () => {
const handler = newLockHandler({ autoRefreshInterval: 100 * timeoutMultiplier });
const lock = await handler.acquireLock(LockType.Sync, LockClientType.Desktop, '111');
const lockBefore = activeLock(await handler.locks(), new Date(), handler.lockTtl, LockType.Sync, LockClientType.Desktop, '111');
handler.startAutoLockRefresh(lock, () => {});
await msleep(500 * timeoutMultiplier);
const lockAfter = activeLock(await handler.locks(), new Date(), handler.lockTtl, LockType.Sync, LockClientType.Desktop, '111');
expect(lockAfter.updatedTime).toBeGreaterThan(lockBefore.updatedTime);
handler.stopAutoLockRefresh(lock);
}));
// {
// await lockHandler().releaseLock(LockType.Sync, LockClientType.Mobile, '222');
// const locks = await lockHandler().locks(LockType.Sync);
// expect(locks.length).toBe(1);
// expect(locks[0].clientId).toBe('111');
// }
// }));
it('should call the error handler when lock has expired while being auto-refreshed', (async () => {
const handler = newLockHandler({
lockTtl: 50 * timeoutMultiplier,
autoRefreshInterval: 200 * timeoutMultiplier,
});
// it('should auto-refresh a lock', (async () => {
// const handler = newLockHandler({ autoRefreshInterval: 100 * timeoutMultiplier });
// const lock = await handler.acquireLock(LockType.Sync, LockClientType.Desktop, '111');
// const lockBefore = activeLock(await handler.locks(), new Date(), handler.lockTtl, LockType.Sync, LockClientType.Desktop, '111');
// handler.startAutoLockRefresh(lock, () => {});
// await msleep(500 * timeoutMultiplier);
// const lockAfter = activeLock(await handler.locks(), new Date(), handler.lockTtl, LockType.Sync, LockClientType.Desktop, '111');
// expect(lockAfter.updatedTime).toBeGreaterThan(lockBefore.updatedTime);
// handler.stopAutoLockRefresh(lock);
// }));
const lock = await handler.acquireLock(LockType.Sync, LockClientType.Desktop, '111');
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
let autoLockError: any = null;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
handler.startAutoLockRefresh(lock, (error: any) => {
autoLockError = error;
});
try {
await msleep(250 * timeoutMultiplier);
expect(autoLockError).toBeTruthy();
expect(autoLockError.code).toBe('lockExpired');
} finally {
handler.stopAutoLockRefresh(lock);
}
}));
it('should not allow sync locks if there is an exclusive lock', (async () => {
await lockHandler().acquireLock(LockType.Exclusive, LockClientType.Desktop, '111');
await expectThrow(async () => {
await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '222');
}, 'hasExclusiveLock');
}));
it('should not allow exclusive lock if there are sync locks', (async () => {
const lockHandler = newLockHandler({ lockTtl: 1000 * 60 * 60 });
if (lockHandler.useBuiltInLocks) return; // Tested server side
await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '111');
await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '222');
await expectThrow(async () => {
await lockHandler.acquireLock(LockType.Exclusive, LockClientType.Desktop, '333');
}, 'hasSyncLock');
}));
it('should allow exclusive lock if the sync locks have expired', (async () => {
const lockHandler = newLockHandler({ lockTtl: 500 * timeoutMultiplier });
if (lockHandler.useBuiltInLocks) return; // Tested server side
await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '111');
await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '222');
await msleep(600 * timeoutMultiplier);
await expectNotThrow(async () => {
await lockHandler.acquireLock(LockType.Exclusive, LockClientType.Desktop, '333');
});
}));
it('should decide what is the active exclusive lock', (async () => {
const lockHandler = newLockHandler();
{
const locks: Lock[] = [
{
type: LockType.Exclusive,
clientId: '1',
clientType: LockClientType.Desktop,
updatedTime: Date.now(),
},
];
await msleep(100);
locks.push({
type: LockType.Exclusive,
clientId: '2',
clientType: LockClientType.Desktop,
updatedTime: Date.now(),
});
const lock = activeLock(locks, new Date(), lockHandler.lockTtl, LockType.Exclusive);
expect(lock.clientId).toBe('1');
}
}));
// it('should ignore locks by same client when trying to acquire exclusive lock', (async () => {
// const lockHandler = newLockHandler();
// await lockHandler.acquireLock(LockType.Sync, LockClientType.Desktop, '111');
// await expectThrow(async () => {
// await lockHandler.acquireLock(LockType.Exclusive, LockClientType.Desktop, '111', { clearExistingSyncLocksFromTheSameClient: false });
// }, 'hasSyncLock');
// await expectNotThrow(async () => {
// await lockHandler.acquireLock(LockType.Exclusive, LockClientType.Desktop, '111', { clearExistingSyncLocksFromTheSameClient: true });
// it('should call the error handler when lock has expired while being auto-refreshed', (async () => {
// const handler = newLockHandler({
// lockTtl: 50 * timeoutMultiplier,
// autoRefreshInterval: 200 * timeoutMultiplier,
// });
// const lock = activeLock(await lockHandler.locks(), new Date(), lockHandler.lockTtl, LockType.Exclusive);
// expect(lock.clientId).toBe('111');
// const lock = await handler.acquireLock(LockType.Sync, LockClientType.Desktop, '111');
// // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
// let autoLockError: any = null;
// // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
// handler.startAutoLockRefresh(lock, (error: any) => {
// autoLockError = error;
// });
// try {
// await msleep(250 * timeoutMultiplier);
// expect(autoLockError).toBeTruthy();
// expect(autoLockError.code).toBe('lockExpired');
// } finally {
// handler.stopAutoLockRefresh(lock);
// }
// }));
// it('should not allow sync locks if there is an exclusive lock', (async () => {
// await lockHandler().acquireLock(LockType.Exclusive, LockClientType.Desktop, '111');
// await expectThrow(async () => {
// await lockHandler().acquireLock(LockType.Sync, LockClientType.Mobile, '222');
// }, 'hasExclusiveLock');
// }));
// it('should not allow exclusive lock if there are sync locks', (async () => {
// const lockHandler = newLockHandler({ lockTtl: 1000 * 60 * 60 });
// if (lockHandler.useBuiltInLocks) return; // Tested server side
// await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '111');
// await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '222');
// await expectThrow(async () => {
// await lockHandler.acquireLock(LockType.Exclusive, LockClientType.Desktop, '333');
// }, 'hasSyncLock');
// }));
// it('should allow exclusive lock if the sync locks have expired', (async () => {
// const lockHandler = newLockHandler({ lockTtl: 500 * timeoutMultiplier });
// if (lockHandler.useBuiltInLocks) return; // Tested server side
// await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '111');
// await lockHandler.acquireLock(LockType.Sync, LockClientType.Mobile, '222');
// await msleep(600 * timeoutMultiplier);
// await expectNotThrow(async () => {
// await lockHandler.acquireLock(LockType.Exclusive, LockClientType.Desktop, '333');
// });
// }));
// it('should decide what is the active exclusive lock', (async () => {
// const lockHandler = newLockHandler();
// {
// const locks: Lock[] = [
// {
// type: LockType.Exclusive,
// clientId: '1',
// clientType: LockClientType.Desktop,
// updatedTime: Date.now(),
// },
// ];
// await msleep(100);
// locks.push({
// type: LockType.Exclusive,
// clientId: '2',
// clientType: LockClientType.Desktop,
// updatedTime: Date.now(),
// });
// const lock = activeLock(locks, new Date(), lockHandler.lockTtl, LockType.Exclusive);
// expect(lock.clientId).toBe('1');
// }
// }));
});