From c63c21bce46d9230688d0c5ebd31f5dd861324de Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Mon, 13 Sep 2021 23:33:18 +0200 Subject: [PATCH] Fix performance storage jsonfile consistency at write: MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit + Use a lock file to avoid concurrent read->write + Switch to synchronous fs operations + Ensure a unique id will be used as a performance mark Signed-off-by: Jérôme Benoit --- package-lock.json | 28 ++++++++++++++++++++-- package.json | 2 ++ rollup.config.js | 2 +- src/charging-station/Bootstrap.ts | 11 +++++---- src/performance/PerformanceStatistics.ts | 12 +++++----- src/performance/storage/JSONFileStorage.ts | 27 +++++++++++---------- 6 files changed, 55 insertions(+), 27 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9bfe0cc0..b99eed09 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5035,6 +5035,15 @@ "integrity": "sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA==", "dev": true }, + "@types/proper-lockfile": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/@types/proper-lockfile/-/proper-lockfile-4.1.2.tgz", + "integrity": "sha512-kd4LMvcnpYkspDcp7rmXKedn8iJSCoa331zRRamUp5oanKt/CefbEGPQP7G89enz7sKD4bvsr8mHSsC8j5WOvA==", + "dev": true, + "requires": { + "@types/retry": "*" + } + }, "@types/responselike": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/@types/responselike/-/responselike-1.0.0.tgz", @@ -5044,6 +5053,12 @@ "@types/node": "*" } }, + "@types/retry": { + "version": "0.12.1", + "resolved": "https://registry.npmjs.org/@types/retry/-/retry-0.12.1.tgz", + "integrity": "sha512-xoDlM2S4ortawSWORYqsdU+2rxdh4LRW9ytc3zmT37RIKQh6IHyKwwtKhKis9ah8ol07DCkZxPt8BBvPjC6v4g==", + "dev": true + }, "@types/seedrandom": { "version": "2.4.27", "resolved": "https://registry.npmjs.org/@types/seedrandom/-/seedrandom-2.4.27.tgz", @@ -15850,6 +15865,16 @@ "react-is": "^16.8.1" } }, + "proper-lockfile": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/proper-lockfile/-/proper-lockfile-4.1.2.tgz", + "integrity": "sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==", + "requires": { + "graceful-fs": "^4.2.4", + "retry": "^0.12.0", + "signal-exit": "^3.0.2" + } + }, "protocol-buffers": { "version": "4.2.0", "resolved": "https://registry.npmjs.org/protocol-buffers/-/protocol-buffers-4.2.0.tgz", @@ -17578,8 +17603,7 @@ "retry": { "version": "0.12.0", "resolved": "https://registry.npmjs.org/retry/-/retry-0.12.0.tgz", - "integrity": "sha1-G0KmJmoh8HQh0bC1S33BZ7AcATs=", - "dev": true + "integrity": "sha1-G0KmJmoh8HQh0bC1S33BZ7AcATs=" }, "reusify": { "version": "1.0.4", diff --git a/package.json b/package.json index 8dcf8d96..b3173fcb 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "chalk": "^4.1.2", "mongodb": "^4.1.1", "poolifier": "^2.1.0", + "proper-lockfile": "^4.1.2", "source-map-support": "^0.5.20", "tar": "^6.1.11", "tslib": "^2.3.1", @@ -94,6 +95,7 @@ "@types/mocha": "^9.0.0", "@types/mochawesome": "^6.2.1", "@types/node": "^14.17.15", + "@types/proper-lockfile": "^4.1.2", "@types/tar": "^4.0.5", "@types/uuid": "^8.3.1", "@types/ws": "^7.4.7", diff --git a/rollup.config.js b/rollup.config.js index 8825169f..3900559c 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -20,7 +20,7 @@ export default { preserveModulesRoot: 'src', ...!isDevelopmentBuild && { plugins: [terser({ numWorkers: 2 })] } }, - external: ['basic-ftp', 'chalk', 'crypto', 'fs', '@mikro-orm/core', '@mikro-orm/reflection', 'mongodb', 'path', 'perf_hooks', 'poolifier', 'reflect-metadata', 'tar', 'url', 'uuid', 'ws', 'winston-daily-rotate-file', 'winston/lib/winston/transports', 'winston', 'worker_threads'], + external: ['basic-ftp', 'chalk', 'crypto', 'fs', '@mikro-orm/core', '@mikro-orm/reflection', 'mongodb', 'path', 'perf_hooks', 'poolifier', 'proper-lockfile', 'reflect-metadata', 'tar', 'url', 'uuid', 'ws', 'winston-daily-rotate-file', 'winston/lib/winston/transports', 'winston', 'worker_threads'], plugins: [ json(), ts({ diff --git a/src/charging-station/Bootstrap.ts b/src/charging-station/Bootstrap.ts index 7fe1ef7b..36d694e7 100644 --- a/src/charging-station/Bootstrap.ts +++ b/src/charging-station/Bootstrap.ts @@ -17,6 +17,7 @@ export default class Bootstrap { private static instance: Bootstrap | null = null; private static workerImplementation: WorkerAbstract | null = null; private static storage: Storage; + private static numberOfChargingStations: number; private version: string = version; private started: boolean; private workerScript: string; @@ -39,21 +40,21 @@ export default class Bootstrap { public async start(): Promise { if (isMainThread && !this.started) { try { - let numStationsTotal = 0; + Bootstrap.numberOfChargingStations = 0; await Bootstrap.storage.open(); await Bootstrap.workerImplementation.start(); // Start ChargingStation object in worker thread if (Configuration.getStationTemplateURLs()) { for (const stationURL of Configuration.getStationTemplateURLs()) { try { - const nbStations = stationURL.numberOfStations ? stationURL.numberOfStations : 0; + const nbStations = stationURL.numberOfStations ?? 0; for (let index = 1; index <= nbStations; index++) { const workerData: ChargingStationWorkerData = { index, templateFile: path.join(path.resolve(__dirname, '../'), 'assets', 'station-templates', path.basename(stationURL.file)) }; await Bootstrap.workerImplementation.addElement(workerData); - numStationsTotal++; + Bootstrap.numberOfChargingStations++; } } catch (error) { console.error(chalk.red('Charging station start with template file ' + stationURL.file + ' error '), error); @@ -62,10 +63,10 @@ export default class Bootstrap { } else { console.warn(chalk.yellow('No stationTemplateURLs defined in configuration, exiting')); } - if (numStationsTotal === 0) { + if (Bootstrap.numberOfChargingStations === 0) { console.warn(chalk.yellow('No charging station template enabled in configuration, exiting')); } else { - console.log(chalk.green(`Charging stations simulator ${this.version} started with ${numStationsTotal.toString()} charging station(s) and ${Utils.workerDynamicPoolInUse() ? `${Configuration.getWorkerPoolMinSize().toString()}/` : ''}${Bootstrap.workerImplementation.size}${Utils.workerPoolInUse() ? `/${Configuration.getWorkerPoolMaxSize().toString()}` : ''} worker(s) concurrently running in '${Configuration.getWorkerProcess()}' mode${Bootstrap.workerImplementation.maxElementsPerWorker ? ` (${Bootstrap.workerImplementation.maxElementsPerWorker} charging station(s) per worker)` : ''}`)); + console.log(chalk.green(`Charging stations simulator ${this.version} started with ${Bootstrap.numberOfChargingStations.toString()} charging station(s) and ${Utils.workerDynamicPoolInUse() ? `${Configuration.getWorkerPoolMinSize().toString()}/` : ''}${Bootstrap.workerImplementation.size}${Utils.workerPoolInUse() ? `/${Configuration.getWorkerPoolMaxSize().toString()}` : ''} worker(s) concurrently running in '${Configuration.getWorkerProcess()}' mode${Bootstrap.workerImplementation.maxElementsPerWorker ? ` (${Bootstrap.workerImplementation.maxElementsPerWorker} charging station(s) per worker)` : ''}`)); } this.started = true; } catch (error) { diff --git a/src/performance/PerformanceStatistics.ts b/src/performance/PerformanceStatistics.ts index d594f138..ff791336 100644 --- a/src/performance/PerformanceStatistics.ts +++ b/src/performance/PerformanceStatistics.ts @@ -26,14 +26,14 @@ export default class PerformanceStatistics { } public static beginMeasure(id: string): string { - const beginId = 'begin' + id.charAt(0).toUpperCase() + id.slice(1); - performance.mark(beginId); - return beginId; + const markId = `${id.charAt(0).toUpperCase() + id.slice(1)}~${Utils.generateUUID()}`; + performance.mark(markId); + return markId; } - public static endMeasure(name: string, beginId: string): void { - performance.measure(name, beginId); - performance.clearMarks(beginId); + public static endMeasure(name: string, markId: string): void { + performance.measure(name, markId); + performance.clearMarks(markId); } public addRequestStatistic(command: RequestCommand | IncomingRequestCommand, messageType: MessageType): void { diff --git a/src/performance/storage/JSONFileStorage.ts b/src/performance/storage/JSONFileStorage.ts index c91a14d5..30c83d3d 100644 --- a/src/performance/storage/JSONFileStorage.ts +++ b/src/performance/storage/JSONFileStorage.ts @@ -5,6 +5,7 @@ import FileUtils from '../../utils/FileUtils'; import Statistics from '../../types/Statistics'; import { Storage } from './Storage'; import fs from 'fs'; +import lockfile from 'proper-lockfile'; export class JSONFileStorage extends Storage { private fd: number | null = null; @@ -16,19 +17,19 @@ export class JSONFileStorage extends Storage { public storePerformanceStatistics(performanceStatistics: Statistics): void { this.checkPerformanceRecordsFile(); - fs.readFile(this.dbName, 'utf8', (error, data) => { - if (error) { - FileUtils.handleFileException(this.logPrefix, Constants.PERFORMANCE_RECORDS_FILETYPE, this.dbName, error); - } else { - const performanceRecords: Statistics[] = data ? JSON.parse(data) as Statistics[] : []; - performanceRecords.push(performanceStatistics); - fs.writeFile(this.dbName, JSON.stringify(performanceRecords, null, 2), 'utf8', (err) => { - if (err) { - FileUtils.handleFileException(this.logPrefix, Constants.PERFORMANCE_RECORDS_FILETYPE, this.dbName, err); - } - }); - } - }); + lockfile.lock(this.dbName, { stale: 5000, retries: 3 }) + .then(async (release) => { + try { + const fileData = fs.readFileSync(this.dbName, 'utf8'); + const performanceRecords: Statistics[] = fileData ? JSON.parse(fileData) as Statistics[] : []; + performanceRecords.push(performanceStatistics); + fs.writeFileSync(this.dbName, JSON.stringify(performanceRecords, null, 2), 'utf8'); + } catch (error) { + FileUtils.handleFileException(this.logPrefix, Constants.PERFORMANCE_RECORDS_FILETYPE, this.dbName, error); + } + await release(); + }) + .catch(() => { }); } public open(): void { -- 2.34.1