Improve sync:

- Push when there is a change - Pull at regular intervals (5s)
microsoft · Sep 13, 2019 · b3771f1 · b3771f1
1 parent 2cc4781
commit b3771f1
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 35 deletions.
diff --git a/src/vs/workbench/contrib/userData/browser/userData.contribution.ts b/src/vs/workbench/contrib/userData/browser/userData.contribution.ts
@@ -51,19 +51,27 @@ class UserDataSyncContribution extends Disposable implements IWorkbenchContribut
 		@IRemoteUserDataService private readonly remoteUserDataService: IRemoteUserDataService,
 	) {
 		super();
-		this.loopSync();
+		this.sync(true);
 		this._register(Event.any<any>(
 			Event.filter(this.configurationService.onDidChangeConfiguration, e => e.affectsConfiguration('userConfiguration.enableSync') && this.configurationService.getValue<boolean>('userConfiguration.enableSync')),
 			this.remoteUserDataService.onDidChangeEnablement)
-			(() => this.loopSync()));
+			(() => this.sync(true)));
+
+		// Sync immediately if there is a local change.
+		this._register(Event.debounce(this.userDataSyncService.onDidChangeLocal, () => undefined, 500)(() => this.sync(false)));
 	}
 
-	private loopSync(): void {
+	private async sync(loop: boolean): Promise<void> {
 		if (this.configurationService.getValue<boolean>('userConfiguration.enableSync') && this.remoteUserDataService.isEnabled()) {
-			this.userDataSyncService.sync()
-				.then(null, () => null) // Surpress errors
-				.then(() => timeout(500))
-				.then(() => this.loopSync());
+			try {
+				await this.userDataSyncService.sync();
+			} catch (e) {
+				// Ignore errors
+			}
+			if (loop) {
+				await timeout(1000 * 5); // Loop sync for every 5s.
+				this.sync(loop);
+			}
 		}
 	}
 

diff --git a/src/vs/workbench/services/userData/common/settingsSync.ts b/src/vs/workbench/services/userData/common/settingsSync.ts
@@ -24,7 +24,7 @@ import { Emitter, Event } from 'vs/base/common/event';
 import { ILogService } from 'vs/platform/log/common/log';
 import { Position } from 'vs/editor/common/core/position';
 import { IHistoryService } from 'vs/workbench/services/history/common/history';
-import { CancelablePromise, createCancelablePromise } from 'vs/base/common/async';
+import { CancelablePromise, createCancelablePromise, ThrottledDelayer } from 'vs/base/common/async';
 
 interface ISyncPreviewResult {
 	readonly fileContent: IFileContent | null;
@@ -46,6 +46,10 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 	private _onDidChangStatus: Emitter<SyncStatus> = this._register(new Emitter<SyncStatus>());
 	readonly onDidChangeStatus: Event<SyncStatus> = this._onDidChangStatus.event;
 
+	private readonly throttledDelayer: ThrottledDelayer<void>;
+	private _onDidChangeLocal: Emitter<void> = this._register(new Emitter<void>());
+	readonly onDidChangeLocal: Event<void> = this._onDidChangeLocal.event;
+
 	constructor(
 		@IFileService private readonly fileService: IFileService,
 		@IWorkbenchEnvironmentService private readonly workbenchEnvironmentService: IWorkbenchEnvironmentService,
@@ -58,6 +62,23 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 		@IHistoryService private readonly historyService: IHistoryService,
 	) {
 		super();
+		this.throttledDelayer = this._register(new ThrottledDelayer<void>(500));
+		this._register(Event.filter(this.fileService.onFileChanges, e => e.contains(this.workbenchEnvironmentService.settingsResource))(() => this.throttledDelayer.trigger(() => this.onDidChangeSettings())));
+	}
+
+	private async onDidChangeSettings(): Promise<void> {
+		const localFileContent = await this.getLocalFileContent();
+		const lastSyncData = this.getLastSyncUserData();
+		if (localFileContent && lastSyncData) {
+			if (localFileContent.value.toString() !== lastSyncData.content) {
+				this._onDidChangeLocal.fire();
+				return;
+			}
+		}
+		if (!localFileContent || !lastSyncData) {
+			this._onDidChangeLocal.fire();
+			return;
+		}
 	}
 
 	private setStatus(status: SyncStatus): void {
@@ -100,15 +121,6 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 		}
 	}
 
-	async stopSync(): Promise<void> {
-		await this.fileService.del(SETTINGS_PREVIEW_RESOURCE);
-		if (this.syncPreviewResultPromise) {
-			this.syncPreviewResultPromise.cancel();
-			this.syncPreviewResultPromise = null;
-			this.setStatus(SyncStatus.Idle);
-		}
-	}
-
 	handleConflicts(): boolean {
 		if (this.status !== SyncStatus.HasConflicts) {
 			return false;
@@ -140,28 +152,27 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 			return;
 		}
 
-		const result = await this.syncPreviewResultPromise;
 		if (await this.fileService.exists(SETTINGS_PREVIEW_RESOURCE)) {
-
 			const settingsPreivew = await this.fileService.readFile(SETTINGS_PREVIEW_RESOURCE);
 			const content = settingsPreivew.value.toString();
 			if (this.hasErrors(content)) {
 				return Promise.reject(localize('errorInvalidSettings', "Unable to sync settings. Please resolve conflicts without any errors/warnings and try again."));
 			}
 
-			let remoteUserData = result.remoteUserData;
-			if (result.hasRemoteChanged) {
+			let { fileContent, remoteUserData, hasLocalChanged, hasRemoteChanged } = await this.syncPreviewResultPromise;
+			if (hasRemoteChanged) {
 				const ref = await this.writeToRemote(content, remoteUserData ? remoteUserData.ref : null);
 				remoteUserData = { ref, content };
 			}
-
-			if (result.hasLocalChanged) {
-				await this.writeToLocal(content, result.fileContent);
+			if (hasLocalChanged) {
+				await this.writeToLocal(content, fileContent);
 			}
-
 			if (remoteUserData) {
 				this.updateLastSyncValue(remoteUserData);
 			}
+
+			// Delete the preview
+			await this.fileService.del(SETTINGS_PREVIEW_RESOURCE);
 		}
 
 		this.syncPreviewResultPromise = null;
@@ -436,6 +447,10 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 	}
 
 	private updateLastSyncValue(remoteUserData: IUserData): void {
+		const lastSyncUserData = this.getLastSyncUserData();
+		if (lastSyncUserData && lastSyncUserData.ref === remoteUserData.ref) {
+			return;
+		}
 		this.storageService.store(SettingsSynchroniser.LAST_SYNC_SETTINGS_STORAGE_KEY, JSON.stringify(remoteUserData), StorageScope.GLOBAL);
 	}
 

diff --git a/src/vs/workbench/services/userData/common/userData.ts b/src/vs/workbench/services/userData/common/userData.ts
@@ -100,9 +100,9 @@ export const SETTINGS_PREVIEW_RESOURCE = URI.file('Settings-Preview').with({ sch
 export interface ISynchroniser {
 	readonly status: SyncStatus;
 	readonly onDidChangeStatus: Event<SyncStatus>;
+	readonly onDidChangeLocal: Event<void>;
 	sync(): Promise<boolean>;
 	continueSync(): Promise<boolean>;
-	stopSync(): Promise<void>;
 	handleConflicts(): boolean;
 }
 

diff --git a/src/vs/workbench/services/userData/common/userDataSyncService.ts b/src/vs/workbench/services/userData/common/userDataSyncService.ts
@@ -23,6 +23,8 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 	private _onDidChangStatus: Emitter<SyncStatus> = this._register(new Emitter<SyncStatus>());
 	readonly onDidChangeStatus: Event<SyncStatus> = this._onDidChangStatus.event;
 
+	readonly onDidChangeLocal: Event<void>;
+
 	constructor(
 		@IFileService fileService: IFileService,
 		@IRemoteUserDataService private readonly remoteUserDataService: IRemoteUserDataService,
@@ -35,6 +37,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 		];
 		this.updateStatus();
 		this._register(Event.any(this.remoteUserDataService.onDidChangeEnablement, ...this.synchronisers.map(s => Event.map(s.onDidChangeStatus, () => undefined)))(() => this.updateStatus()));
+		this.onDidChangeLocal = Event.any(...this.synchronisers.map(s => s.onDidChangeLocal));
 	}
 
 	async sync(): Promise<boolean> {
@@ -49,15 +52,6 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
 		return true;
 	}
 
-	async stopSync(): Promise<void> {
-		if (!this.remoteUserDataService.isEnabled()) {
-			throw new Error('Not enabled');
-		}
-		for (const synchroniser of this.synchronisers) {
-			await synchroniser.stopSync();
-		}
-	}
-
 	async continueSync(): Promise<boolean> {
 		if (!this.remoteUserDataService.isEnabled()) {
 			throw new Error('Not enabled');