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

@@ -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,

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<typeof startTrRoutingProcess>[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<typeof startTrRoutingProcess>[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 {

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
+        );
+    });
 });

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;

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'
     },

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 });
 });

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<AdditionalConfig> = {
      */
     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<any> = {
     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<any> = {
  * @returns
  */
 export const setProjectConfiguration = <AdditionalConfig>(config: Partial<ProjectConfiguration<AdditionalConfig>>) =>
-    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;

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);