Skip to content

Commit 4f16ae6

Browse files
sandy081sandy081
committed
Fix #91556
1 parent f9de4bb commit 4f16ae6

3 files changed

Lines changed: 157 additions & 17 deletions

File tree

‎src/vs/platform/userDataSync/common/userDataAutoSyncService.ts‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,22 @@
66
import { timeout, Delayer } from 'vs/base/common/async';
77
import { Event, Emitter } from 'vs/base/common/event';
88
import { Disposable } from 'vs/base/common/lifecycle';
9-
import { IUserDataSyncLogService, IUserDataSyncService, SyncStatus, IUserDataAutoSyncService, UserDataSyncError, UserDataSyncErrorCode, IUserDataSyncEnablementService } from 'vs/platform/userDataSync/common/userDataSync';
9+
import { IUserDataSyncLogService, IUserDataSyncService, SyncStatus, IUserDataAutoSyncService, UserDataSyncError, UserDataSyncErrorCode, IUserDataSyncEnablementService, ALL_SYNC_RESOURCES } from 'vs/platform/userDataSync/common/userDataSync';
1010
import { IAuthenticationTokenService } from 'vs/platform/authentication/common/authentication';
1111
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
1212

13-
type AutoSyncTriggerClassification = {
14-
source: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
13+
type AutoSyncClassification = {
14+
sources: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
1515
};
1616

17+
1718
export class UserDataAutoSyncService extends Disposable implements IUserDataAutoSyncService {
1819

1920
_serviceBrand: any;
2021

21-
private enabled: boolean = false;
22+
private enabled: boolean = this.getDefaultEnablementValue();
2223
private successiveFailures: number = 0;
24+
private lastSyncTriggerTime: number | undefined = undefined;
2325
private readonly syncDelayer: Delayer<void>;
2426

2527
private readonly _onError: Emitter<UserDataSyncError> = this._register(new Emitter<UserDataSyncError>());
@@ -41,6 +43,9 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto
4143
this._register(this.userDataSyncEnablementService.onDidChangeResourceEnablement(() => this.triggerAutoSync(['resourceEnablement'])));
4244
}
4345

46+
// For tests purpose only
47+
protected getDefaultEnablementValue(): boolean { return false; }
48+
4449
private updateEnablement(stopIfDisabled: boolean, auto: boolean): void {
4550
const { enabled, reason } = this.isAutoSyncEnabled();
4651
if (this.enabled === enabled) {
@@ -65,6 +70,7 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto
6570
private async sync(loop: boolean, auto: boolean): Promise<void> {
6671
if (this.enabled) {
6772
try {
73+
this.lastSyncTriggerTime = new Date().getTime();
6874
await this.userDataSyncService.sync();
6975
this.resetFailures();
7076
} catch (e) {
@@ -112,18 +118,34 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto
112118
this.successiveFailures = 0;
113119
}
114120

121+
private sources: string[] = [];
115122
async triggerAutoSync(sources: string[]): Promise<void> {
116-
sources.forEach(source => this.telemetryService.publicLog2<{ source: string }, AutoSyncTriggerClassification>('sync/triggerAutoSync', { source }));
117-
if (this.enabled) {
118-
return this.syncDelayer.trigger(() => {
119-
this.logService.info('Auto Sync: Triggered.');
120-
return this.sync(false, true);
121-
}, this.successiveFailures
122-
? 1000 * 1 * Math.min(this.successiveFailures, 60) /* Delay by number of seconds as number of failures up to 1 minute */
123-
: 1000);
124-
} else {
125-
this.syncDelayer.cancel();
123+
if (!this.enabled) {
124+
return this.syncDelayer.cancel();
125+
}
126+
127+
/*
128+
If sync is not triggered by sync resource (triggered by other sources like window focus etc.,)
129+
then limit sync to once per minute
130+
*/
131+
const isNotTriggeredBySyncResource = ALL_SYNC_RESOURCES.every(syncResource => !sources.includes(syncResource));
132+
if (isNotTriggeredBySyncResource && this.lastSyncTriggerTime
133+
&& Math.round((new Date().getTime() - this.lastSyncTriggerTime) / 1000) < 60) {
134+
this.logService.debug('Auto Sync Skipped: Limited to once per minute.');
135+
return;
126136
}
137+
138+
this.sources.push(...sources);
139+
return this.syncDelayer.trigger(() => {
140+
this.telemetryService.publicLog2<{ sources: string[] }, AutoSyncClassification>('sync/triggered', { sources: this.sources });
141+
this.sources = [];
142+
143+
this.logService.info('Auto Sync: Triggered.');
144+
return this.sync(false, true);
145+
}, this.successiveFailures
146+
? 1000 * 1 * Math.min(this.successiveFailures, 60) /* Delay by number of failures times number of seconds max till 1 minute */
147+
: 0); /* Do not delay if there are no failures */
148+
127149
}
128150

129151
}

‎src/vs/platform/userDataSync/common/userDataSyncService.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ import { URI } from 'vs/base/common/uri';
1919
import { SettingsSynchroniser } from 'vs/platform/userDataSync/common/settingsSync';
2020
import { isEqual } from 'vs/base/common/resources';
2121
import { SnippetsSynchroniser } from 'vs/platform/userDataSync/common/snippetsSync';
22+
import { Throttler } from 'vs/base/common/async';
2223

23-
type SyncErrorClassification = {
24-
source: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
24+
type SyncClassification = {
25+
source?: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
2526
};
2627

2728
const SESSION_ID_KEY = 'sync.sessionId';
@@ -31,6 +32,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
3132

3233
_serviceBrand: any;
3334

35+
private readonly syncThrottler: Throttler;
3436
private readonly synchronisers: IUserDataSynchroniser[];
3537

3638
private _status: SyncStatus = SyncStatus.Uninitialized;
@@ -68,6 +70,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
6870
@IStorageService private readonly storageService: IStorageService
6971
) {
7072
super();
73+
this.syncThrottler = new Throttler();
7174
this.settingsSynchroniser = this._register(this.instantiationService.createInstance(SettingsSynchroniser));
7275
this.keybindingsSynchroniser = this._register(this.instantiationService.createInstance(KeybindingsSynchroniser));
7376
this.snippetsSynchroniser = this._register(this.instantiationService.createInstance(SnippetsSynchroniser));
@@ -111,7 +114,10 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
111114

112115
async sync(): Promise<void> {
113116
await this.checkEnablement();
117+
await this.syncThrottler.queue(() => this.doSync());
118+
}
114119

120+
private async doSync(): Promise<void> {
115121
const startTime = new Date().getTime();
116122
this._syncErrors = [];
117123
try {
@@ -120,6 +126,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
120126
this.setStatus(SyncStatus.Syncing);
121127
}
122128

129+
this.telemetryService.publicLog2<{}, SyncClassification>('sync/getmanifest');
123130
let manifest = await this.userDataSyncStoreService.manifest();
124131

125132
// Server has no data but this machine was synced before
@@ -335,7 +342,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
335342
if (e instanceof UserDataSyncStoreError) {
336343
switch (e.code) {
337344
case UserDataSyncErrorCode.TooLarge:
338-
this.telemetryService.publicLog2<{ source: string }, SyncErrorClassification>('sync/errorTooLarge', { source });
345+
this.telemetryService.publicLog2<{ source: string }, SyncClassification>('sync/errorTooLarge', { source });
339346
}
340347
throw e;
341348
}

‎src/vs/platform/userDataSync/test/common/userDataAutoSyncService.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as assert from 'assert';
7+
import { UserDataSyncClient, UserDataSyncTestServer } from 'vs/platform/userDataSync/test/common/userDataSyncClient';
8+
import { DisposableStore } from 'vs/base/common/lifecycle';
9+
import { UserDataAutoSyncService } from 'vs/platform/userDataSync/common/userDataAutoSyncService';
10+
import { IUserDataSyncService, SyncResource, IUserDataSyncEnablementService } from 'vs/platform/userDataSync/common/userDataSync';
11+
12+
class TestUserDataAutoSyncService extends UserDataAutoSyncService {
13+
protected getDefaultEnablementValue(): boolean { return true; }
14+
}
15+
16+
suite('UserDataAutoSyncService', () => {
17+
18+
const disposableStore = new DisposableStore();
19+
20+
teardown(() => disposableStore.clear());
21+
22+
test('test auto sync with sync resource change triggers sync', async () => {
23+
// Setup the client
24+
const target = new UserDataSyncTestServer();
25+
const client = disposableStore.add(new UserDataSyncClient(target));
26+
await client.setUp();
27+
28+
// Sync once and reset requests
29+
await client.instantiationService.get(IUserDataSyncService).sync();
30+
target.reset();
31+
32+
client.instantiationService.get(IUserDataSyncEnablementService).setEnablement(true);
33+
const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService);
34+
35+
// Trigger auto sync with settings change
36+
await testObject.triggerAutoSync([SyncResource.Settings]);
37+
38+
// Make sure only one request is made
39+
assert.deepEqual(target.requests, [{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} }]);
40+
});
41+
42+
test('test auto sync with sync resource change triggers sync for every change', async () => {
43+
// Setup the client
44+
const target = new UserDataSyncTestServer();
45+
const client = disposableStore.add(new UserDataSyncClient(target));
46+
await client.setUp();
47+
48+
// Sync once and reset requests
49+
await client.instantiationService.get(IUserDataSyncService).sync();
50+
target.reset();
51+
52+
client.instantiationService.get(IUserDataSyncEnablementService).setEnablement(true);
53+
const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService);
54+
55+
// Trigger auto sync with settings change multiple times
56+
for (let counter = 0; counter < 3; counter++) {
57+
await testObject.triggerAutoSync([SyncResource.Settings]);
58+
}
59+
60+
// Make sure only one request is made
61+
assert.deepEqual(target.requests, [
62+
{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} },
63+
{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} },
64+
{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} }
65+
]);
66+
});
67+
68+
test('test auto sync with non sync resource change triggers sync', async () => {
69+
// Setup the client
70+
const target = new UserDataSyncTestServer();
71+
const client = disposableStore.add(new UserDataSyncClient(target));
72+
await client.setUp();
73+
74+
// Sync once and reset requests
75+
await client.instantiationService.get(IUserDataSyncService).sync();
76+
target.reset();
77+
78+
client.instantiationService.get(IUserDataSyncEnablementService).setEnablement(true);
79+
const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService);
80+
81+
// Trigger auto sync with window focus once
82+
await testObject.triggerAutoSync(['windowFocus']);
83+
84+
// Make sure only one request is made
85+
assert.deepEqual(target.requests, [{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} }]);
86+
});
87+
88+
test('test auto sync with non sync resource change does not trigger continuous syncs', async () => {
89+
// Setup the client
90+
const target = new UserDataSyncTestServer();
91+
const client = disposableStore.add(new UserDataSyncClient(target));
92+
await client.setUp();
93+
94+
// Sync once and reset requests
95+
await client.instantiationService.get(IUserDataSyncService).sync();
96+
target.reset();
97+
98+
client.instantiationService.get(IUserDataSyncEnablementService).setEnablement(true);
99+
const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService);
100+
101+
// Trigger auto sync with window focus multiple times
102+
for (let counter = 0; counter < 100; counter++) {
103+
await testObject.triggerAutoSync(['windowFocus']);
104+
}
105+
106+
// Make sure only one request is made
107+
assert.deepEqual(target.requests, [{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} }]);
108+
});
109+
110+
111+
});

0 commit comments

Comments
 (0)