Skip to content

Commit e7748bb

Browse files
PPLT_3672: [Smart concurrency] Scaling up/down asset discovery queue (#1849)
* feat: added functions to all the methods for cpuLoad and memoryUsage * feat: updates after testing this out on macOSX fix: minor code bugs && will continue with that * fix: upload-artifact@v4 * fix: download-artifact@v4 * fix: await 1sec for cpu load * feat: completed monitoring package test: added tests for the smae for every module in monitoring * test: Test fix * test: monitoring coverage fix * test: monitoring coverage fix 2 * test: fixing cli-exec specs * test: fixing cli-exec specs 2 * test: fixing cli-exec specs 3 * test: added tests for percy.test.js coverage fix * test: added tests for percy.test.js coverage fix 2 * test: added tests for cli-exec and core * refactor: update name from cpuload to cpuusage * Delete d.txt * refactor: resolving comments * test: Test fix * chore: lint fix * test: Test fix 2 * test: Test fix 3 * test: coverage fix * test: coverage fix 3 * test: coverage fix 3 * feat: added env variable to disable only discovery concurrency change
1 parent 01d95d0 commit e7748bb

21 files changed

+1546
-9
lines changed

.github/workflows/test.yml

+3-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ jobs:
2828
${{ hashFiles('.github/.cache-key') }}/
2929
- run: yarn
3030
- run: yarn build
31-
- uses: actions/upload-artifact@v3
31+
- uses: actions/upload-artifact@v4
3232
with:
3333
name: dist
3434
path: packages/*/dist
@@ -56,6 +56,7 @@ jobs:
5656
- '@percy/cli-config'
5757
- '@percy/sdk-utils'
5858
- '@percy/webdriver-utils'
59+
- '@percy/monitoring'
5960
runs-on: ${{ matrix.os }}
6061
steps:
6162
- uses: actions/checkout@v3
@@ -75,7 +76,7 @@ jobs:
7576
restore-keys: >
7677
${{ runner.os }}/node-${{ matrix.node }}/
7778
${{ hashFiles('.github/.cache-key') }}/
78-
- uses: actions/download-artifact@v3
79+
- uses: actions/download-artifact@v4
7980
with:
8081
name: dist
8182
path: packages

.github/workflows/windows.yml

+3-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ jobs:
2828
${{ hashFiles('.github/.cache-key') }}/
2929
- run: yarn
3030
- run: yarn build
31-
- uses: actions/upload-artifact@v3
31+
- uses: actions/upload-artifact@v4
3232
with:
3333
name: dist
3434
path: packages/*/dist
@@ -55,6 +55,7 @@ jobs:
5555
- '@percy/cli-config'
5656
- '@percy/sdk-utils'
5757
- '@percy/webdriver-utils'
58+
- '@percy/monitoring'
5859
runs-on: windows-latest
5960
steps:
6061
- uses: actions/checkout@v3
@@ -74,7 +75,7 @@ jobs:
7475
restore-keys: >
7576
${{ runner.os }}/node-14/
7677
${{ hashFiles('.github/.cache-key') }}/
77-
- uses: actions/download-artifact@v3
78+
- uses: actions/download-artifact@v4
7879
with:
7980
name: dist
8081
path: packages

packages/cli-exec/test/exec.test.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import exec from '@percy/cli-exec';
33
describe('percy exec', () => {
44
beforeEach(async () => {
55
process.env.PERCY_TOKEN = '<<PERCY_TOKEN>>';
6+
// due to lot of start calls, it slows spec to finish
7+
// therefore disabling it
8+
process.env.PERCY_DISABLE_SYSTEM_MONITORING = 'true';
69
process.env.PERCY_FORCE_PKG_VALUE = JSON.stringify({ name: '@percy/client', version: '1.0.0' });
710
jasmine.DEFAULT_TIMEOUT_INTERVAL = 25000;
811
await setupTest();
@@ -24,6 +27,7 @@ describe('percy exec', () => {
2427
delete process.env.PERCY_PARALLEL_TOTAL;
2528
delete process.env.PERCY_PARTIAL_BUILD;
2629
delete process.env.PERCY_CLIENT_ERROR_LOGS;
30+
delete process.env.PERCY_DISABLE_SYSTEM_MONITORING;
2731
});
2832

2933
describe('projectType is app', () => {
@@ -277,7 +281,6 @@ describe('percy exec', () => {
277281
'setTimeout(() => process.exit(1), 5000)'
278282
)]);
279283

280-
// wait until the process starts
281284
await new Promise(r => setTimeout(r, 1000));
282285
expect(logger.stdout).toEqual(jasmine.arrayContaining([
283286
jasmine.stringContaining('[percy] Running "node --eval ')

packages/cli-exec/test/start.test.js

+4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ describe('percy exec:start', () => {
1414

1515
beforeEach(async () => {
1616
process.env.PERCY_TOKEN = '<<PERCY_TOKEN>>';
17+
18+
// disabling as it increases spec time and logs system info
19+
process.env.PERCY_DISABLE_SYSTEM_MONITORING = 'true';
1720
process.env.PERCY_FORCE_PKG_VALUE = JSON.stringify({ name: '@percy/client', version: '1.0.0' });
1821
await setupTest();
1922

@@ -29,6 +32,7 @@ describe('percy exec:start', () => {
2932
delete process.env.PERCY_ENABLE;
3033
delete process.env.PERCY_PARALLEL_TOTAL;
3134
delete process.env.PERCY_PARTIAL_BUILD;
35+
delete process.env.PERCY_DISABLE_SYSTEM_MONITORING;
3236

3337
// it's important that percy is still running or we terminate the test process
3438
if (started) process.emit('SIGTERM');

packages/core/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"@percy/dom": "1.30.7-beta.2",
4949
"@percy/logger": "1.30.7-beta.2",
5050
"@percy/webdriver-utils": "1.30.7-beta.2",
51+
"@percy/monitoring": "1.30.7-beta.1",
5152
"content-disposition": "^0.5.4",
5253
"cross-spawn": "^7.0.3",
5354
"extract-zip": "^2.0.1",

packages/core/src/discovery.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ async function* captureSnapshotResources(page, snapshot, options) {
355355
// one concurrently. When skipping asset discovery, the callback is called immediately for each
356356
// snapshot, also processing snapshot resources when not dry-running.
357357
export async function* discoverSnapshotResources(queue, options, callback) {
358-
let { snapshots, skipDiscovery, dryRun } = options;
358+
let { snapshots, skipDiscovery, dryRun, checkAndUpdateConcurrency } = options;
359359

360360
yield* yieldAll(snapshots.reduce((all, snapshot) => {
361361
debugSnapshotOptions(snapshot);
@@ -368,6 +368,10 @@ export async function* discoverSnapshotResources(queue, options, callback) {
368368
callback(dryRun ? snap : processSnapshotResources(snap));
369369
}
370370
} else {
371+
// update concurrency before pushing new job in discovery queue
372+
// if case of monitoring is stopped due to in-activity,
373+
// it can take upto 1 sec to execute this fun
374+
checkAndUpdateConcurrency();
371375
all.push(queue.push(snapshot, callback));
372376
}
373377

packages/core/src/percy.js

+97-1
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,16 @@ import {
2626
discoverSnapshotResources,
2727
createDiscoveryQueue
2828
} from './discovery.js';
29+
import Monitoring from '@percy/monitoring';
2930
import { WaitForJob } from './wait-for-job.js';
3031

3132
const MAX_SUGGESTION_CALLS = 10;
3233

34+
// If no activity is done for 5 mins, we will stop monitoring
35+
// system metric eg: (cpu load && memory usage)
36+
const MONITOR_ACTIVITY_TIMEOUT = 300000;
37+
const MONITORING_INTERVAL_MS = 5000; // 5 sec
38+
3339
// A Percy instance will create a new build when started, handle snapshot creation, asset discovery,
3440
// and resource uploads, and will finalize the build when stopped. Snapshots are processed
3541
// concurrently and the build is not finalized until all snapshots have been handled.
@@ -107,8 +113,15 @@ export class Percy {
107113
this.browser = new Browser(this);
108114

109115
this.#discovery = createDiscoveryQueue(this);
116+
this.discoveryMaxConcurrency = this.#discovery.concurrency;
110117
this.#snapshots = createSnapshotsQueue(this);
111118

119+
this.monitoring = new Monitoring();
120+
// used continue monitoring if there is activity going on
121+
// if there is none, stop it
122+
this.resetMonitoringId = null;
123+
this.monitoringCheckLastExecutedAt = null;
124+
112125
// generator methods are wrapped to autorun and return promises
113126
for (let m of ['start', 'stop', 'flush', 'idle', 'snapshot', 'upload']) {
114127
// the original generator can be referenced with percy.yield.<method>
@@ -117,6 +130,29 @@ export class Percy {
117130
}
118131
}
119132

133+
systemMonitoringEnabled() {
134+
return (process.env.PERCY_DISABLE_SYSTEM_MONITORING !== 'true');
135+
}
136+
137+
async configureSystemMonitor() {
138+
await this.monitoring.startMonitoring({ interval: MONITORING_INTERVAL_MS });
139+
this.resetSystemMonitor();
140+
}
141+
142+
// Debouncing logic to only stop Monitoring system
143+
// if there is no any activity for 5 mins
144+
// means, no job is pushed in queue from 5 mins
145+
resetSystemMonitor() {
146+
if (this.resetMonitoringId) {
147+
clearTimeout(this.resetMonitoringId);
148+
this.resetMonitoringId = null;
149+
}
150+
151+
this.resetMonitoringId = setTimeout(() => {
152+
this.monitoring.stopMonitoring();
153+
}, MONITOR_ACTIVITY_TIMEOUT);
154+
}
155+
120156
// Shortcut for controlling the global logger's log level.
121157
loglevel(level) {
122158
return logger.loglevel(level);
@@ -169,6 +205,13 @@ export class Percy {
169205
this.cliStartTime = new Date().toISOString();
170206

171207
try {
208+
// started monitoring system metrics
209+
210+
if (this.systemMonitoringEnabled()) {
211+
await this.configureSystemMonitor();
212+
await this.monitoring.logSystemInfo();
213+
}
214+
172215
if (process.env.PERCY_CLIENT_ERROR_LOGS !== 'false') {
173216
this.log.warn('Notice: Percy collects CI logs to improve service and enhance your experience. These logs help us debug issues and provide insights on your dashboards, making it easier to optimize the product experience. Logs are stored securely for 30 days. You can opt out anytime with export PERCY_CLIENT_ERROR_LOGS=false, but keeping this enabled helps us offer the best support and features.');
174217
}
@@ -298,13 +341,66 @@ export class Percy {
298341
this.log.error(err);
299342
throw err;
300343
} finally {
344+
// stop monitoring system metric, if not already stopped
345+
this.monitoring.stopMonitoring();
346+
clearTimeout(this.resetMonitoringId);
347+
301348
// This issue doesn't comes under regular error logs,
302349
// it's detected if we just and stop percy server
303350
await this.checkForNoSnapshotCommandError();
304351
await this.sendBuildLogs();
305352
}
306353
}
307354

355+
checkAndUpdateConcurrency() {
356+
// early exit if monitoring is disabled
357+
if (!this.systemMonitoringEnabled()) return;
358+
359+
// early exit if asset discovery concurrency change is disabled
360+
// NOTE: system monitoring will still be running as only concurrency
361+
// change is disabled
362+
if (process.env.PERCY_DISABLE_CONCURRENCY_CHANGE === 'true') return;
363+
364+
// start system monitoring if not already doing...
365+
// this doesn't handle cases where there is suggest cpu spikes
366+
// in less 1 sec range and if monitoring is not in running state
367+
if (this.monitoringCheckLastExecutedAt && Date.now() - this.monitoringCheckLastExecutedAt < MONITORING_INTERVAL_MS) return;
368+
369+
if (!this.monitoring.running) this.configureSystemMonitor();
370+
else this.resetSystemMonitor();
371+
372+
// early return if last executed was less than 5 seconds
373+
// as we will get the same cpu/mem info under 5 sec interval
374+
const { cpuInfo, memoryUsageInfo } = this.monitoring.getMonitoringInfo();
375+
this.log.debug(`cpuInfo: ${JSON.stringify(cpuInfo)}`);
376+
this.log.debug(`memoryInfo: ${JSON.stringify(memoryUsageInfo)}`);
377+
378+
if (cpuInfo.currentUsagePercent >= 80 || memoryUsageInfo.currentUsagePercent >= 80) {
379+
let currentConcurrent = this.#discovery.concurrency;
380+
381+
// concurrency must be betweeen [1, (default/user defined value)]
382+
let newConcurrency = Math.max(1, parseInt(currentConcurrent / 2));
383+
newConcurrency = Math.min(this.discoveryMaxConcurrency, newConcurrency);
384+
385+
this.log.debug(`Downscaling discovery browser concurrency from ${this.#discovery.concurrency} to ${newConcurrency}`);
386+
this.#discovery.set({ concurrency: newConcurrency });
387+
} else if (cpuInfo.currentUsagePercent <= 50 && memoryUsageInfo.currentUsagePercent <= 50) {
388+
let currentConcurrent = this.#discovery.concurrency;
389+
let newConcurrency = currentConcurrent + 2;
390+
391+
// concurrency must be betweeen [1, (default/user-defined value)]
392+
newConcurrency = Math.min(this.discoveryMaxConcurrency, newConcurrency);
393+
newConcurrency = Math.max(1, newConcurrency);
394+
395+
this.log.debug(`Upscaling discovery browser concurrency from ${this.#discovery.concurrency} to ${newConcurrency}`);
396+
this.#discovery.set({ concurrency: newConcurrency });
397+
}
398+
399+
// reset timeout to stop monitoring after no-activity of 5 mins
400+
this.resetSystemMonitor();
401+
this.monitoringCheckLastExecutedAt = Date.now();
402+
}
403+
308404
// Takes one or more snapshots of a page while discovering resources to upload with the resulting
309405
// snapshots. Once asset discovery has completed for the provided snapshots, the queued task will
310406
// resolve and an upload task will be queued separately.
@@ -351,7 +447,7 @@ export class Percy {
351447
yield* discoverSnapshotResources(this.#discovery, {
352448
skipDiscovery: this.skipDiscovery,
353449
dryRun: this.dryRun,
354-
450+
checkAndUpdateConcurrency: this.checkAndUpdateConcurrency.bind(this),
355451
snapshots: yield* gatherSnapshots(options, {
356452
meta: { build: this.build },
357453
config: this.config

0 commit comments

Comments
 (0)