From 8886188578f0e1b0cc9cba1833e43e136c46c1ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Genevi=C3=A8ve=20Bastien?= Date: Thu, 3 Oct 2024 10:22:57 -0400 Subject: [PATCH] trRouting: Move port and cacheAllScenarios to instance specific config This adds the `trRouting` property to the config type. This property contains data for the `single` instance for individual calculations (UI and public API) and `batch` instance (for tasks). For each instance, the `port` and `cacheAllScenarios` properties can be set. The configuration objects replace the `trRouting` preference that contained the `port` and `batchPortStart` and the current `trRoutingCacheAllScenarios` configuration option. Port management when starting or querying trRouting is updated to match the new configuration options. Keep the current `trRoutingCacheAllScenarios` configuration option, but add a warning when it is being used. --- examples/config.js | 20 ++++- .../TrRoutingProcessManager.ts | 42 +++++++--- .../__tests__/TrRoutingProcessManager.test.ts | 78 +++++++++++++++++++ .../trRouting/TrRoutingServiceBackend.ts | 9 ++- .../src/config/defaultPreferences.config.ts | 4 - .../shared/__tests__/project.config.test.ts | 7 +- .../src/config/shared/project.config.ts | 47 ++++++++++- .../src/api/services.socketRoutes.ts | 4 +- .../src/services/simulation/SimulationRun.ts | 3 +- 9 files changed, 183 insertions(+), 31 deletions(-) diff --git a/examples/config.js b/examples/config.js index 00ce3bb10..d5ea09e78 100644 --- a/examples/config.js +++ b/examples/config.js @@ -11,12 +11,24 @@ module.exports = { }, maxParallelCalculators: 2, - // Maximum number of parallel calculation - // TODO trRouting should support multi-threading so we shouldn't have to start multiple instances + // Maximum number of parallel calculation. Used in tasks to start the calculator with this number of threads. // maxParallelCalculators: 2, - // Enable caching of connections for all scenarios in trRouting. Will use more memory - trRoutingCacheAllScenarios: false, + // @deprecated: Use the cacheAllScenarios in the 'trRouting' configuration instead + // trRoutingCacheAllScenarios: false, + // Configuration for the trRouting services. Single is for the single calculation instance (from the UI and public API), while batch is for the batch calculation instance, for tasks + //trRouting: { + // single: { + // port: 4000, + // // Enable caching of connections for all scenarios in trRouting. Will use more memory + // cacheAllScenarios: false + // }, + // batch: { + // port: 14000, + // // Enable caching of connections for all scenarios in trRouting. Will use more memory and may not be necessary for batch calculations as currently there's only one scenario per task + // cacheAllScenarios: false + // } + //}, mapDefaultCenter: { lat: 45.5092960, diff --git a/packages/chaire-lib-backend/src/utils/processManagers/TrRoutingProcessManager.ts b/packages/chaire-lib-backend/src/utils/processManagers/TrRoutingProcessManager.ts index af65a5fcb..7ce95f9c0 100644 --- a/packages/chaire-lib-backend/src/utils/processManagers/TrRoutingProcessManager.ts +++ b/packages/chaire-lib-backend/src/utils/processManagers/TrRoutingProcessManager.ts @@ -6,7 +6,6 @@ */ import os from 'os'; import { directoryManager } from '../filesystem/directoryManager'; -import Preferences from 'chaire-lib-common/lib/config/Preferences'; import ProcessManager from './ProcessManager'; import osrmService from '../osrm/OSRMService'; import config from '../../config/server.config'; @@ -81,10 +80,28 @@ const startTrRoutingProcess = async ( return processStatus; }; +const getCacheAllScenarios = () => { + // FIXME Should be simply `return + // config.trRouting?.single?.cacheAllScenarios === undefined ? false : + // config.trRouting.single.cacheAllScenarios;`, but we need to handle + // deprecation of `trRoutingCacheAllScenarios` value. Deprecated config + // option has precedence as it means it was already set by the user. The + // cacheAllScenarios should always be set as it has a default value. + const valueFromDeprecatedConfig = config.trRoutingCacheAllScenarios; + if (valueFromDeprecatedConfig !== undefined) { + console.warn( + 'The `trRoutingCacheAllScenarios` configuration is deprecated and will be removed in the future. Please use `trRouting.single.cacheAllScenarios` instead.' + ); + return valueFromDeprecatedConfig; + } + return config.trRouting?.single?.cacheAllScenarios === undefined + ? false + : config.trRouting.single.cacheAllScenarios; +}; + const start = async (parameters: { port?: number; debug?: boolean; cacheDirectoryPath?: string }) => { - const port = parameters.port || Preferences.get('trRouting.port'); - const cacheAllScenarios = - config.trRoutingCacheAllScenarios === undefined ? false : config.trRoutingCacheAllScenarios; + const port = parameters.port || config.trRouting?.single?.port || 4000; + const cacheAllScenarios = getCacheAllScenarios(); const params: Parameters[3] = { debug: parameters.debug, @@ -97,7 +114,7 @@ const start = async (parameters: { port?: number; debug?: boolean; cacheDirector }; const stop = async (parameters: { port?: number; debug?: boolean; cacheDirectoryPath?: string }) => { - const port = parameters.port || Preferences.get('trRouting.port'); + const port = parameters.port || config.trRouting?.single?.port || 4000; const serviceName = getServiceName(port); const tagName = 'trRouting'; @@ -110,10 +127,9 @@ const restart = async (parameters: { cacheDirectoryPath?: string; doNotStartIfStopped?: boolean; }) => { - const port = parameters.port || Preferences.get('trRouting.port'); + const port = parameters.port || config.trRouting?.single?.port || 4000; const serviceName = getServiceName(port); - const cacheAllScenarios = - config.trRoutingCacheAllScenarios === undefined ? false : config.trRoutingCacheAllScenarios; + const cacheAllScenarios = getCacheAllScenarios(); const params: Parameters[3] = { debug: parameters.debug, @@ -136,7 +152,7 @@ const restart = async (parameters: { }; const status = async (parameters: { port?: number }) => { - const port = parameters.port || Preferences.get('trRouting.port'); + const port = parameters.port || config.trRouting?.single?.port || 4000; const serviceName = getServiceName(port); if (await ProcessManager.isServiceRunning(serviceName)) { @@ -156,7 +172,7 @@ const status = async (parameters: { port?: number }) => { const startBatch = async function ( numberOfCpus: number, - port: number = Preferences.get('trRouting.batchPortStart', 14000), + port: number = config.trRouting?.batch?.port || 14000, cacheDirectoryPath?: string ) { // Ensure we don't use more CPU than configured @@ -166,8 +182,10 @@ const startBatch = async function ( console.warn('Asking for too many trRouting threads (%d), reducing to %d', numberOfCpus, maxThreadCount); numberOfCpus = maxThreadCount; } + const cacheAllScenarios = + config.trRouting?.batch?.cacheAllScenarios === undefined ? false : config.trRouting.batch.cacheAllScenarios; - const params = { cacheDirectoryPath: cacheDirectoryPath }; + const params = { cacheDirectoryPath: cacheDirectoryPath, cacheAllScenarios }; await startTrRoutingProcess(port, false, numberOfCpus, params); @@ -178,7 +196,7 @@ const startBatch = async function ( }; }; -const stopBatch = async function (port: number = Preferences.get('trRouting.batchPortStart', 14000)) { +const stopBatch = async function (port: number = config.trRouting?.batch?.port || 14000) { await stop({ port: port }); return { diff --git a/packages/chaire-lib-backend/src/utils/processManagers/__tests__/TrRoutingProcessManager.test.ts b/packages/chaire-lib-backend/src/utils/processManagers/__tests__/TrRoutingProcessManager.test.ts index 1bf6777dc..11cdb6a13 100644 --- a/packages/chaire-lib-backend/src/utils/processManagers/__tests__/TrRoutingProcessManager.test.ts +++ b/packages/chaire-lib-backend/src/utils/processManagers/__tests__/TrRoutingProcessManager.test.ts @@ -1,9 +1,17 @@ +/* + * Copyright 2022, Polytechnique Montreal and contributors + * + * This file is licensed under the MIT License. + * License text available at https://opensource.org/licenses/MIT + */ +import _cloneDeep from 'lodash/cloneDeep'; import { directoryManager } from '../../filesystem/directoryManager'; import TrRoutingProcessManager from '../TrRoutingProcessManager'; import ProcessManager from '../ProcessManager'; import osrmService from '../../osrm/OSRMService'; import OSRMMode from '../../osrm/OSRMMode'; import config from '../../../config/server.config'; +import { setProjectConfiguration } from 'chaire-lib-common/lib/config/shared/project.config'; jest.mock('../ProcessManager', () => ({ startProcess: jest.fn(), @@ -31,9 +39,12 @@ startProcessMock.mockImplementation(async (serviceName: string, service: tagName, name: serviceName })); +let defaultConfig = _cloneDeep(config); beforeEach(function () { startProcessMock.mockClear(); stopProcessMock.mockClear(); + // Reset the config to default + setProjectConfiguration(defaultConfig); // Override max parallel setting // TODO might be a better way to do this config.maxParallelCalculators = 8; @@ -130,6 +141,51 @@ describe('TrRouting Process Manager: start', () => { false ); }); + test('start process with configured port and cacheAllScenarios for single trRouting', async () => { + // Add a port and cacheAllScenarios to the trRouting single config + const port = 4002; + setProjectConfiguration({ trRouting: { single: { port, cacheAllScenarios: true } } as any }); + const status = await TrRoutingProcessManager.start({}); + expect(status).toEqual({ + status: 'started', + action: 'start', + service: 'trRouting', + name: `trRouting${port}` + }); + expect(startProcessMock).toHaveBeenCalledTimes(1); + expect(startProcessMock).toHaveBeenCalledWith( + `trRouting${port}`, + 'trRouting', + 'trRouting', + [`--port=${port}`, `--osrmPort=${walkingOsrmMode.getHostPort().port}`, `--osrmHost=${walkingOsrmMode.getHostPort().host}`, '--debug=0', `--cachePath=${directoryManager.projectDirectory}/cache/test`, '--threads=2', '--cacheAllConnectionSets=true'], + 'ready.', + false, + undefined, + false + ); + }); + test('start process with deprecated trRoutingCacheAllScenarios configuration option', async () => { + // Add a port and cacheAllScenarios to the trRouting single config + setProjectConfiguration({ trRoutingCacheAllScenarios: true }); + const status = await TrRoutingProcessManager.start({}); + expect(status).toEqual({ + status: 'started', + action: 'start', + service: 'trRouting', + name: 'trRouting4000' + }); + expect(startProcessMock).toHaveBeenCalledTimes(1); + expect(startProcessMock).toHaveBeenCalledWith( + 'trRouting4000', + 'trRouting', + 'trRouting', + ['--port=4000', `--osrmPort=${walkingOsrmMode.getHostPort().port}`, `--osrmHost=${walkingOsrmMode.getHostPort().host}`, '--debug=0', `--cachePath=${directoryManager.projectDirectory}/cache/test`, '--threads=2', '--cacheAllConnectionSets=true'], + 'ready.', + false, + undefined, + false + ); + }); test('start batch process with 1 cpu', async () => { const status = await TrRoutingProcessManager.startBatch(1); expect(status).toEqual({ @@ -210,4 +266,26 @@ describe('TrRouting Process Manager: start', () => { false ); }); + test('start batch process with 4 cpus with port and cacheAll configuration', async () => { + // Add a port and cacheAllScenarios to the trRouting batch config + const port = 14002; + setProjectConfiguration({ trRouting: { batch: { port, cacheAllScenarios: true } } as any }); + const status = await TrRoutingProcessManager.startBatch(4); + expect(status).toEqual({ + status: 'started', + service: 'trRoutingBatch', + port + }); + expect(startProcessMock).toHaveBeenCalledTimes(1); + expect(startProcessMock).toHaveBeenCalledWith( + `trRouting${port}`, + 'trRouting', + 'trRouting', + [`--port=${port}`, `--osrmPort=${walkingOsrmMode.getHostPort().port}`, `--osrmHost=${walkingOsrmMode.getHostPort().host}`, '--debug=0', `--cachePath=${directoryManager.projectDirectory}/cache/test`, '--threads=4', '--cacheAllConnectionSets=true'], + 'ready.', + false, + undefined, + false + ); + }); }); diff --git a/packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts b/packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts index 2e54a0b05..9f5253111 100644 --- a/packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts +++ b/packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts @@ -8,9 +8,8 @@ import _get from 'lodash/get'; //TODO replace this fetch-retry library with one compatible with TS const fetch = require('@zeit/fetch-retry')(require('node-fetch')); -import Preferences from 'chaire-lib-common/lib/config/Preferences'; +import config from '../../config/server.config'; import * as TrRoutingApi from 'chaire-lib-common/lib/api/TrRouting'; -import TrRoutingProcessManager from '../processManagers/TrRoutingProcessManager'; import { _isBlank } from 'chaire-lib-common/lib/utils/LodashExtensions'; /** @@ -191,9 +190,11 @@ class TrRoutingServiceBackend { port: string | number | undefined, customRequestPath: string ): string { - const trRoutingConfig = Preferences.get('trRouting'); + // FIXME We should not get the port from here, as we assume here it is for the single instance and not the batch. + const trRoutingConfig = config.trRouting?.single || { port: 4000 }; if (host === undefined) { - host = process.env.TR_ROUTING_HOST_URL || trRoutingConfig.host || 'http://localhost'; + // There used to be a host in the config, but we do not support it, it either comes from the environment or is localhost + host = process.env.TR_ROUTING_HOST_URL || 'http://localhost'; } if (port === undefined) { port = process.env.TR_ROUTING_HOST_PORT || trRoutingConfig.port; diff --git a/packages/chaire-lib-common/src/config/defaultPreferences.config.ts b/packages/chaire-lib-common/src/config/defaultPreferences.config.ts index 76b996c7f..6336693a3 100644 --- a/packages/chaire-lib-common/src/config/defaultPreferences.config.ts +++ b/packages/chaire-lib-common/src/config/defaultPreferences.config.ts @@ -173,10 +173,6 @@ const defaultPreferences: PreferencesModel = { showAggregatedOdTripsLayer: true, socketUploadChunkSize: 10240000, defaultWalkingSpeedMetersPerSeconds: 5 / 3.6, - trRouting: { - port: 4000, - autoStart: false - }, geoNames: { host: 'http://api.geonames.org/findNearestIntersectionOSMJSON' }, diff --git a/packages/chaire-lib-common/src/config/shared/__tests__/project.config.test.ts b/packages/chaire-lib-common/src/config/shared/__tests__/project.config.test.ts index d01388895..dedd2ecd2 100644 --- a/packages/chaire-lib-common/src/config/shared/__tests__/project.config.test.ts +++ b/packages/chaire-lib-common/src/config/shared/__tests__/project.config.test.ts @@ -6,11 +6,16 @@ test('Expected default', () => { expect(projectConfig.mapDefaultCenter).toEqual({ lon: -73.6131, lat: 45.5041 }); expect(projectConfig.separateAdminLoginPage).toEqual(false); expect(projectConfig.projectShortname).toEqual('default'); + expect(projectConfig.trRouting.single).toEqual({ port: 4000, cacheAllScenarios: false }); + expect(projectConfig.trRouting.batch).toEqual({ port: 14000, cacheAllScenarios: false }); }); test('setProjectConfiguration', () => { - setProjectConfiguration({ projectShortname: 'newProject', mapDefaultCenter: { lon: -73, lat: 45 } }) + setProjectConfiguration({ projectShortname: 'newProject', mapDefaultCenter: { lon: -73, lat: 45 }, trRouting: { single: { port: 5000 } } as any }) expect(projectConfig.mapDefaultCenter).toEqual({ lon: -73, lat: 45 }); expect(projectConfig.separateAdminLoginPage).toEqual(false); expect(projectConfig.projectShortname).toEqual('newProject'); + // Make sure the deep merge works for object configs + expect(projectConfig.trRouting.single).toEqual({ port: 5000, cacheAllScenarios: false }); + expect(projectConfig.trRouting.batch).toEqual({ port: 14000, cacheAllScenarios: false }); }); \ No newline at end of file diff --git a/packages/chaire-lib-common/src/config/shared/project.config.ts b/packages/chaire-lib-common/src/config/shared/project.config.ts index c6f57b078..eeaec6c48 100644 --- a/packages/chaire-lib-common/src/config/shared/project.config.ts +++ b/packages/chaire-lib-common/src/config/shared/project.config.ts @@ -4,6 +4,22 @@ * This file is licensed under the MIT License. * License text available at https://opensource.org/licenses/MIT */ +import _merge from 'lodash/merge'; + +type TrRoutingConfig = { + /** + * Port to use for this trRouting instance + */ + port: number; + /** + * If set to `true`, enable caching of connections for all scenarios in + * trRouting. Will use more memory + */ + cacheAllScenarios: boolean; + // FIXME Do we need to configure a host here? If so, we need to properly + // support it in both batch and single calculation +}; + // TODO Some project config option depend on the application, so should not be // typed in chaire-lib. Each app (transition , evolution, etc) should add types // to the config and should have a project.config file which imports this one @@ -70,6 +86,19 @@ export type ProjectConfiguration = { */ projectDirectory: string; maxParallelCalculators: number; + /** + * TrRouting specific configurations + */ + trRouting: { + /** + * Configuration for the single calculation trRouting service + */ + single: TrRoutingConfig; + /** + * Configuration for the batch calculation trRouting service + */ + batch: TrRoutingConfig; + }; } & AdditionalConfig; // Initialize default configuration @@ -79,7 +108,17 @@ const projectConfig: ProjectConfiguration = { projectShortname: 'default', userDiskQuota: '1gb', maxFileUploadMB: 256, - maxParallelCalculators: 1 + maxParallelCalculators: 1, + trRouting: { + single: { + port: 4000, + cacheAllScenarios: false + }, + batch: { + port: 14000, + cacheAllScenarios: false + } + } }; /** @@ -91,6 +130,10 @@ const projectConfig: ProjectConfiguration = { * @returns */ export const setProjectConfiguration = (config: Partial>) => - Object.keys(config).forEach((configKey) => (projectConfig[configKey] = config[configKey])); + Object.keys(config).forEach((configKey) => + typeof projectConfig[configKey] === 'object' + ? _merge(projectConfig[configKey], config[configKey]) + : (projectConfig[configKey] = config[configKey]) + ); export default projectConfig; diff --git a/packages/transition-backend/src/api/services.socketRoutes.ts b/packages/transition-backend/src/api/services.socketRoutes.ts index 6fef2463f..2ff9b8267 100644 --- a/packages/transition-backend/src/api/services.socketRoutes.ts +++ b/packages/transition-backend/src/api/services.socketRoutes.ts @@ -15,7 +15,7 @@ import trRoutingProcessManager from 'chaire-lib-backend/lib/utils/processManager import trRoutingService from 'chaire-lib-backend/lib/utils/trRouting/TrRoutingServiceBackend'; import osrmProcessManager from 'chaire-lib-backend/lib/utils/processManagers/OSRMProcessManager'; import osrmService from 'chaire-lib-backend/lib/utils/osrm/OSRMService'; -import Preferences from 'chaire-lib-common/lib/config/Preferences'; +import config from 'chaire-lib-backend/lib/config/server.config'; import { TrRoutingConstants } from 'chaire-lib-common/lib/api/TrRouting'; import { transitionRouteOptions, transitionMatchOptions } from 'chaire-lib-common/lib/api/OSRMRouting'; import { AccessibilityMapAttributes } from 'transition-common/lib/services/accessibilityMap/TransitAccessibilityMapRouting'; @@ -135,7 +135,7 @@ export default function (socket: EventEmitter, userId?: number) { .updateCache( parameters, parameters.host || 'http://localhost', - parameters.port || Preferences.get('trRouting.port') + parameters.port || config.trRouting?.single?.port || 4000 ) .then((response) => { callback(response); diff --git a/packages/transition-backend/src/services/simulation/SimulationRun.ts b/packages/transition-backend/src/services/simulation/SimulationRun.ts index c8741e295..adc99dd2a 100644 --- a/packages/transition-backend/src/services/simulation/SimulationRun.ts +++ b/packages/transition-backend/src/services/simulation/SimulationRun.ts @@ -10,7 +10,6 @@ import pQueue from 'p-queue'; import _cloneDeep from 'lodash/cloneDeep'; import Scenario from 'transition-common/lib/services/scenario/Scenario'; import trRoutingService from 'chaire-lib-backend/lib/utils/trRouting/TrRoutingServiceBackend'; -import Preferences from 'chaire-lib-common/lib/config/Preferences'; import config from 'chaire-lib-backend/lib/config/server.config'; import Simulation, { SimulationDataAttributes } from 'transition-common/lib/services/simulation/Simulation'; @@ -132,7 +131,7 @@ export default class SimulationRunBackend extends SimulationRun { } public getBatchPort(): number { - return this.attributes.options.trRoutingStartingPort || Preferences.get('trRouting.batchPortStart', 14000); + return this.attributes.options.trRoutingStartingPort || config.trRouting?.batch?.port || 14000; } public getCacheDirectoryPath(): string {