From d088c7f0a3d98ead6101f27d1c017e639e0c3c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 23 Aug 2021 15:29:03 +0200 Subject: [PATCH 01/27] Implement the locks part of the task manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/knexfile.js | 26 ++++ .../migrations/20210928160613_init.js | 71 +++++++++ packages/backend-common/package.json | 5 + .../src/database/DatabaseManager.ts | 5 +- .../src/database/migrateBackendCommon.ts | 30 ++++ .../backend-common/src/database/tables.ts | 22 +++ packages/backend-common/src/index.ts | 1 + .../src/tasks/TaskManager.test.ts | 73 +++++++++ .../backend-common/src/tasks/TaskManager.ts | 144 ++++++++++++++++++ packages/backend-common/src/tasks/index.ts | 18 +++ packages/backend-common/src/tasks/types.ts | 45 ++++++ packages/backend-common/src/tasks/util.ts | 26 ++++ packages/backend/src/index.ts | 31 ++-- packages/backend/src/types.ts | 6 +- yarn.lock | 5 + 15 files changed, 490 insertions(+), 18 deletions(-) create mode 100644 packages/backend-common/knexfile.js create mode 100644 packages/backend-common/migrations/20210928160613_init.js create mode 100644 packages/backend-common/src/database/migrateBackendCommon.ts create mode 100644 packages/backend-common/src/database/tables.ts create mode 100644 packages/backend-common/src/tasks/TaskManager.test.ts create mode 100644 packages/backend-common/src/tasks/TaskManager.ts create mode 100644 packages/backend-common/src/tasks/index.ts create mode 100644 packages/backend-common/src/tasks/types.ts create mode 100644 packages/backend-common/src/tasks/util.ts diff --git a/packages/backend-common/knexfile.js b/packages/backend-common/knexfile.js new file mode 100644 index 0000000000..4c8be42673 --- /dev/null +++ b/packages/backend-common/knexfile.js @@ -0,0 +1,26 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// This file makes it possible to run "yarn knex migrate:make some_file_name" +// to assist in making new migrations +module.exports = { + client: 'sqlite3', + connection: ':memory:', + useNullAsDefault: true, + migrations: { + directory: './migrations', + }, +}; diff --git a/packages/backend-common/migrations/20210928160613_init.js b/packages/backend-common/migrations/20210928160613_init.js new file mode 100644 index 0000000000..663e2f8aeb --- /dev/null +++ b/packages/backend-common/migrations/20210928160613_init.js @@ -0,0 +1,71 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +/** + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + // + // locking + // + await knex.schema.createTable( + 'backstage_backend_common__task_locks', + table => { + table.comment('Locks used for mutual exclusion among multiple workers'); + table + .text('id') + .primary() + .notNullable() + .comment('The unique id of this particular lock'); + table + .text('acquired_ticket') + .nullable() + .comment('A unique ticket for the current lock acquiral, if any'); + table + .dateTime('acquired_at') + .nullable() + .comment('The time when the lock was acquired, if locked'); + table + .dateTime('expires_at') + .nullable() + .comment('The time when an acquired lock will time out and expire'); + table.index('id', 'task_locks_id_idx'); + }, + ); + // + // tasks + // + await knex.schema.createTable('backstage_backend_common__tasks', table => { + table.comment('Tasks used for scheduling work on multiple workers'); + table + .text('id') + .primary() + .notNullable() + .comment('The unique id of this particular task'); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + await knex.schema.alterTable('task_locks', table => { + table.dropIndex([], 'task_locks_id_idx'); + }); + await knex.schema.dropTable('task_locks'); +}; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 03ae1fbe62..c8e8d69ab5 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -59,14 +59,17 @@ "knex": "^0.95.1", "lodash": "^4.17.21", "logform": "^2.1.1", + "luxon": "^2.0.2", "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", + "node-abort-controller": "^3.0.0", "raw-body": "^2.4.1", "selfsigned": "^1.10.7", "stoppable": "^1.1.0", "tar": "^6.1.2", "unzipper": "^0.10.11", + "uuid": "^8.0.0", "winston": "^3.2.1", "yn": "^4.0.0" }, @@ -79,6 +82,7 @@ } }, "devDependencies": { + "@backstage/backend-test-utils": "^0.1.8", "@backstage/cli": "^0.8.2", "@backstage/test-utils": "^0.1.21", "@types/archiver": "^5.1.0", @@ -107,6 +111,7 @@ }, "files": [ "dist", + "migrations/**/*.{js,d.ts}", "config.d.ts" ], "configSchema": "config.d.ts" diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index c0511c6b78..f8cdab1413 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -13,14 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { Knex } from 'knex'; import { omit } from 'lodash'; import { Config, ConfigReader } from '@backstage/config'; import { JsonObject } from '@backstage/types'; import { createDatabaseClient, - ensureDatabaseExists, createNameOverride, + ensureDatabaseExists, normalizeConnection, } from './connection'; import { PluginDatabaseManager } from './types'; @@ -165,7 +166,7 @@ export class DatabaseManager { ); return { - // include base connection if client type has not been overriden + // include base connection if client type has not been overridden ...(overridden ? {} : baseConnection), ...connection, }; diff --git a/packages/backend-common/src/database/migrateBackendCommon.ts b/packages/backend-common/src/database/migrateBackendCommon.ts new file mode 100644 index 0000000000..6283f3a96c --- /dev/null +++ b/packages/backend-common/src/database/migrateBackendCommon.ts @@ -0,0 +1,30 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { resolvePackagePath } from '../paths'; + +const migrationsDir = resolvePackagePath( + '@backstage/backend-common', + 'migrations', +); + +export async function migrateBackendCommon(knex: Knex): Promise { + await knex.migrate.latest({ + directory: migrationsDir, + tableName: 'knex_migrations_backstage_backend_common', + }); +} diff --git a/packages/backend-common/src/database/tables.ts b/packages/backend-common/src/database/tables.ts new file mode 100644 index 0000000000..8efa393bb7 --- /dev/null +++ b/packages/backend-common/src/database/tables.ts @@ -0,0 +1,22 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type DbTaskLocksRow = { + id: string; + acquired_ticket?: string; + acquired_at?: Date; + expires_at?: Date; +}; diff --git a/packages/backend-common/src/index.ts b/packages/backend-common/src/index.ts index c214961a2e..a906d7bc61 100644 --- a/packages/backend-common/src/index.ts +++ b/packages/backend-common/src/index.ts @@ -31,4 +31,5 @@ export * from './paths'; export * from './reading'; export * from './scm'; export * from './service'; +export * from './tasks'; export * from './util'; diff --git a/packages/backend-common/src/tasks/TaskManager.test.ts b/packages/backend-common/src/tasks/TaskManager.test.ts new file mode 100644 index 0000000000..da9adb70f6 --- /dev/null +++ b/packages/backend-common/src/tasks/TaskManager.test.ts @@ -0,0 +1,73 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; +import { Duration } from 'luxon'; +import { DatabaseManager } from '../database'; +import { TaskManager } from './TaskManager'; + +describe('TaskManager', () => { + const databases = TestDatabases.create({ + ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], + }); + + async function createDatabase( + databaseId: TestDatabaseId, + ): Promise { + const knex = await databases.init(databaseId); + const databaseManager: Partial = { + forPlugin: () => ({ + getClient: async () => knex, + }), + }; + return databaseManager as DatabaseManager; + } + + describe('locking', () => { + it.each(databases.eachSupportedId())( + 'can run the happy path, %p', + async databaseId => { + const database = await createDatabase(databaseId); + const manager = new TaskManager(database).forPlugin('test'); + + const lock1 = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + const lock2 = await manager.acquireLock('lock2', { + timeout: Duration.fromMillis(5000), + }); + + expect(lock1.acquired).toBe(true); + expect(lock2.acquired).toBe(true); + + await expect( + manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }), + ).resolves.toEqual({ acquired: false }); + + await (lock1 as any).release(); + await (lock2 as any).release(); + + const lock1Again = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + expect(lock1Again.acquired).toBe(true); + await (lock1Again as any).release(); + }, + ); + }); +}); diff --git a/packages/backend-common/src/tasks/TaskManager.ts b/packages/backend-common/src/tasks/TaskManager.ts new file mode 100644 index 0000000000..29741e1594 --- /dev/null +++ b/packages/backend-common/src/tasks/TaskManager.ts @@ -0,0 +1,144 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Config } from '@backstage/config'; +import { Knex } from 'knex'; +import { memoize } from 'lodash'; +import { Duration } from 'luxon'; +import { v4 as uuid } from 'uuid'; +import { DatabaseManager } from '../database'; +import { migrateBackendCommon } from '../database/migrateBackendCommon'; +import { DbTaskLocksRow } from '../database/tables'; +import { PluginTaskManager } from './types'; +import { validateId } from './util'; + +export class PluginTaskManagerImpl implements PluginTaskManager { + constructor( + private readonly pluginId: string, + private readonly databaseFactory: () => Promise, + ) {} + + async acquireLock( + idWithoutPrefix: string, + options: { + timeout: Duration; + }, + ): Promise< + | { acquired: false } + | { acquired: true; release: () => void | Promise } + > { + validateId(idWithoutPrefix); + + const knex = await this.databaseFactory(); + const id = `plugin:${this.pluginId}:${idWithoutPrefix}`; + const ticket = uuid(); + const timeout = options.timeout.as('seconds'); + + const release = async () => { + try { + await knex('backstage_backend_common__task_locks') + .where('id', '=', id) + .where('acquired_ticket', '=', ticket) + .delete(); + } catch { + // fail silently + } + }; + + // First try to overwrite an existing lock, that has timed out + const stolen = await knex( + 'backstage_backend_common__task_locks', + ) + .where('id', '=', id) + .whereNotNull('acquired_ticket') + .where('expires_at', '<', knex.fn.now()) + .update({ + acquired_ticket: ticket, + acquired_at: knex.fn.now(), + expires_at: + knex.client.config.client === 'sqlite3' + ? knex.raw(`datetime('now', ?)`, [`${timeout} seconds`]) + : knex.raw(`now() + interval '${timeout} seconds'`), + }); + + if (stolen) { + return { acquired: true, release }; + } + + try { + await knex('backstage_backend_common__task_locks').insert( + { + id, + acquired_ticket: ticket, + acquired_at: knex.fn.now(), + expires_at: + knex.client.config.client === 'sqlite3' + ? knex.raw(`datetime('now', ?)`, [`${timeout} seconds`]) + : knex.raw(`now() + interval '${timeout} seconds'`), + }, + ); + return { acquired: true, release }; + } catch { + return { acquired: false }; + } + } + + async scheduleTask( + idWithoutPrefix: string, + options: { + timeout: Duration; + frequency: Duration; + initialDelay?: Duration; + }, + fn: () => Promise, + ): Promise<{ release: () => Promise }> { + validateId(idWithoutPrefix); + + const knex = await this.databaseFactory(); + const id = `plugin:${this.pluginId}:${idWithoutPrefix}`; + + return {}; + } +} + +/** + * Deals with management and locking related to distributed tasks. + * + * @public + */ +export class TaskManager { + static fromConfig( + config: Config, + options?: { databaseManager?: DatabaseManager }, + ): TaskManager { + const databaseManager = + options?.databaseManager ?? DatabaseManager.fromConfig(config); + return new TaskManager(databaseManager); + } + + constructor(private readonly databaseManager: DatabaseManager) {} + + forPlugin(pluginId: string): PluginTaskManager { + return new PluginTaskManagerImpl( + pluginId, + memoize(async () => { + const knex = await this.databaseManager.forPlugin(pluginId).getClient(); + await migrateBackendCommon(knex); + return knex; + }), + ); + } +} diff --git a/packages/backend-common/src/tasks/index.ts b/packages/backend-common/src/tasks/index.ts new file mode 100644 index 0000000000..60329788b3 --- /dev/null +++ b/packages/backend-common/src/tasks/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type { PluginTaskManager } from './types'; +export { TaskManager } from './TaskManager'; diff --git a/packages/backend-common/src/tasks/types.ts b/packages/backend-common/src/tasks/types.ts new file mode 100644 index 0000000000..4a4fcb474e --- /dev/null +++ b/packages/backend-common/src/tasks/types.ts @@ -0,0 +1,45 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Duration } from 'luxon'; + +/** + * Deals with management and locking related to distributed tasks, for a given + * plugin. + * + * @public + */ +export interface PluginTaskManager { + acquireLock( + id: string, + options: { + timeout: Duration; + }, + ): Promise< + | { acquired: false } + | { acquired: true; release: () => void | Promise } + >; + + scheduleTask( + id: string, + options: { + timeout: Duration; + frequency: Duration; + initialDelay?: Duration; + }, + fn: () => Promise, + ): Promise<{ release: () => Promise }>; +} diff --git a/packages/backend-common/src/tasks/util.ts b/packages/backend-common/src/tasks/util.ts new file mode 100644 index 0000000000..7bd4ff61b5 --- /dev/null +++ b/packages/backend-common/src/tasks/util.ts @@ -0,0 +1,26 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/errors'; + +// Keep the IDs compatible with e.g. Prometheus +export function validateId(id: string) { + if (typeof id !== 'string' || !/^[a-z0-9]+(?:_[a-z0-9]+)*$/.test(id)) { + throw new InputError( + `${id} is not a valid ID, expected string of lowercase characters and digits separated by underscores`, + ); + } +} diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index ffdce949b7..554c9a7d99 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -22,41 +22,42 @@ * Happy hacking! */ -import Router from 'express-promise-router'; import { CacheManager, createServiceBuilder, + DatabaseManager, getRootLogger, loadBackendConfig, notFoundHandler, - DatabaseManager, SingleHostDiscovery, + TaskManager, UrlReaders, useHotMemoize, } from '@backstage/backend-common'; import { Config } from '@backstage/config'; -import healthcheck from './plugins/healthcheck'; -import { metricsInit, metricsHandler } from './metrics'; +import Router from 'express-promise-router'; +import { metricsHandler, metricsInit } from './metrics'; +import app from './plugins/app'; import auth from './plugins/auth'; import azureDevOps from './plugins/azure-devops'; +import badges from './plugins/badges'; import catalog from './plugins/catalog'; import codeCoverage from './plugins/codecoverage'; -import kubernetes from './plugins/kubernetes'; +import graphql from './plugins/graphql'; +import healthcheck from './plugins/healthcheck'; +import jenkins from './plugins/jenkins'; import kafka from './plugins/kafka'; +import kubernetes from './plugins/kubernetes'; +import proxy from './plugins/proxy'; import rollbar from './plugins/rollbar'; import scaffolder from './plugins/scaffolder'; -import proxy from './plugins/proxy'; import search from './plugins/search'; import techdocs from './plugins/techdocs'; -import todo from './plugins/todo'; -import graphql from './plugins/graphql'; -import app from './plugins/app'; -import badges from './plugins/badges'; -import jenkins from './plugins/jenkins'; import techInsights from './plugins/techInsights'; +import todo from './plugins/todo'; import { PluginEnvironment } from './types'; -function makeCreateEnv(config: Config) { +async function makeCreateEnv(config: Config) { const root = getRootLogger(); const reader = UrlReaders.default({ logger: root, config }); const discovery = SingleHostDiscovery.fromConfig(config); @@ -64,13 +65,15 @@ function makeCreateEnv(config: Config) { root.info(`Created UrlReader ${reader}`); const databaseManager = DatabaseManager.fromConfig(config); + const taskManager = TaskManager.fromConfig(config); const cacheManager = CacheManager.fromConfig(config); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); const database = databaseManager.forPlugin(plugin); + const tasks = taskManager.forPlugin(plugin); const cache = cacheManager.forPlugin(plugin); - return { logger, cache, database, config, reader, discovery }; + return { logger, cache, database, tasks, config, reader, discovery }; }; } @@ -87,7 +90,7 @@ async function main() { argv: process.argv, logger, }); - const createEnv = makeCreateEnv(config); + const createEnv = await makeCreateEnv(config); const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck')); const catalogEnv = useHotMemoize(module, () => createEnv('catalog')); diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index 8290e569ef..aafd25bc3f 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -14,19 +14,21 @@ * limitations under the License. */ -import { Logger } from 'winston'; -import { Config } from '@backstage/config'; import { PluginCacheManager, PluginDatabaseManager, PluginEndpointDiscovery, + PluginTaskManager, UrlReader, } from '@backstage/backend-common'; +import { Config } from '@backstage/config'; +import { Logger } from 'winston'; export type PluginEnvironment = { logger: Logger; cache: PluginCacheManager; database: PluginDatabaseManager; + tasks: PluginTaskManager; config: Config; reader: UrlReader; discovery: PluginEndpointDiscovery; diff --git a/yarn.lock b/yarn.lock index 4c42941ad9..caba3db5fb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20895,6 +20895,11 @@ node-abi@^2.21.0: dependencies: semver "^5.4.1" +node-abort-controller@^3.0.0: + version "3.0.0" + resolved "https://registry.npmjs.org/node-abort-controller/-/node-abort-controller-3.0.0.tgz#e7b1f2b72f4c5a74b5594cc23fa8fe3b62010ab5" + integrity sha512-IqMCPbihDpbHV4bNws015hU0svIBGyzPjJearwXMGJyungWdblbBcboNojTz9bWOrrJD3zIwmcr5w5c+NH+2+A== + node-addon-api@^3.0.0: version "3.2.1" resolved "https://registry.npmjs.org/node-addon-api/-/node-addon-api-3.2.1.tgz#81325e0a2117789c0128dab65e7e38f07ceba161" From d4f412fcd3fbef498eeebfd2ea1647da56d5ced2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 30 Sep 2021 09:35:49 +0200 Subject: [PATCH 02/27] Get most of the task worker code into place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/api-report.md | 47 +++ .../migrations/20210928160613_init.js | 89 ++++-- packages/backend-common/package.json | 6 +- .../backend-common/src/database/tables.ts | 20 +- .../backend-common/src/tasks/CancelToken.ts | 48 +++ .../src/tasks/PluginTaskManagerImpl.test.ts | 67 +++++ .../src/tasks/PluginTaskManagerImpl.ts | 132 +++++++++ .../src/tasks/TaskManager.test.ts | 46 +-- .../backend-common/src/tasks/TaskManager.ts | 126 ++------ .../src/tasks/TaskWorker.test.ts | 257 ++++++++++++++++ .../backend-common/src/tasks/TaskWorker.ts | 277 ++++++++++++++++++ packages/backend-common/src/tasks/types.ts | 32 +- packages/backend-common/src/tasks/util.ts | 19 ++ packages/backend/src/index.ts | 4 +- yarn.lock | 5 - 15 files changed, 996 insertions(+), 179 deletions(-) create mode 100644 packages/backend-common/src/tasks/CancelToken.ts create mode 100644 packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts create mode 100644 packages/backend-common/src/tasks/PluginTaskManagerImpl.ts create mode 100644 packages/backend-common/src/tasks/TaskWorker.test.ts create mode 100644 packages/backend-common/src/tasks/TaskWorker.ts diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index a19447034e..93c25bca24 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -12,6 +12,7 @@ import { BitbucketIntegration } from '@backstage/integration'; import { Config } from '@backstage/config'; import cors from 'cors'; import Docker from 'dockerode'; +import { Duration } from 'luxon'; import { ErrorRequestHandler } from 'express'; import express from 'express'; import { GithubCredentialsProvider } from '@backstage/integration'; @@ -399,6 +400,37 @@ export type PluginEndpointDiscovery = { getExternalBaseUrl(pluginId: string): Promise; }; +// @public +export interface PluginTaskManager { + // (undocumented) + acquireLock( + id: string, + options: { + timeout: Duration; + }, + ): Promise< + | { + acquired: false; + } + | { + acquired: true; + release: () => void | Promise; + } + >; + // (undocumented) + scheduleTask( + id: string, + options: { + timeout: Duration; + frequency: Duration; + initialDelay?: Duration; + }, + fn: () => Promise, + ): Promise<{ + unschedule: () => Promise; + }>; +} + // @public export type ReaderFactory = (options: { config: Config; @@ -576,6 +608,21 @@ export interface StatusCheckHandlerOptions { statusCheck?: StatusCheck; } +// @public +export class TaskManager { + constructor(databaseManager: DatabaseManager, logger: Logger_2); + // (undocumented) + forPlugin(pluginId: string): PluginTaskManager; + // (undocumented) + static fromConfig( + config: Config, + options?: { + databaseManager?: DatabaseManager; + logger?: Logger_2; + }, + ): TaskManager; +} + // @public export type UrlReader = { read(url: string): Promise; diff --git a/packages/backend-common/migrations/20210928160613_init.js b/packages/backend-common/migrations/20210928160613_init.js index 663e2f8aeb..7a7590d903 100644 --- a/packages/backend-common/migrations/20210928160613_init.js +++ b/packages/backend-common/migrations/20210928160613_init.js @@ -21,32 +21,29 @@ */ exports.up = async function up(knex) { // - // locking + // mutexes // - await knex.schema.createTable( - 'backstage_backend_common__task_locks', - table => { - table.comment('Locks used for mutual exclusion among multiple workers'); - table - .text('id') - .primary() - .notNullable() - .comment('The unique id of this particular lock'); - table - .text('acquired_ticket') - .nullable() - .comment('A unique ticket for the current lock acquiral, if any'); - table - .dateTime('acquired_at') - .nullable() - .comment('The time when the lock was acquired, if locked'); - table - .dateTime('expires_at') - .nullable() - .comment('The time when an acquired lock will time out and expire'); - table.index('id', 'task_locks_id_idx'); - }, - ); + await knex.schema.createTable('backstage_backend_common__mutexes', table => { + table.comment('Locks used for mutual exclusion among multiple workers'); + table + .text('id') + .primary() + .notNullable() + .comment('The unique ID of this particular mutex'); + table + .text('current_lock_ticket') + .nullable() + .comment('A unique ticket for the current mutex lock'); + table + .dateTime('current_lock_acquired_at') + .nullable() + .comment('The time when the mutex was locked'); + table + .dateTime('current_lock_expires_at') + .nullable() + .comment('The time when a locked mutex will time out and auto-release'); + table.index(['id'], 'backstage_backend_common__mutexes__id_idx'); + }); // // tasks // @@ -56,7 +53,28 @@ exports.up = async function up(knex) { .text('id') .primary() .notNullable() - .comment('The unique id of this particular task'); + .comment('The unique ID of this particular task'); + table + .text('settings_json') + .notNullable() + .comment('JSON serialized object with properties for this task'); + table + .dateTime('next_run_start_at') + .nullable() + .comment('The next time that the task should be started'); + table + .text('current_run_ticket') + .nullable() + .comment('A unique ticket for the current task run'); + table + .dateTime('current_run_started_at') + .nullable() + .comment('The time that the current task run started'); + table + .dateTime('current_run_expires_at') + .nullable() + .comment('The time that the current task run will time out'); + table.index(['id'], 'backstage_backend_common__tasks__id_idx'); }); }; @@ -64,8 +82,21 @@ exports.up = async function up(knex) { * @param {import('knex').Knex} knex */ exports.down = async function down(knex) { - await knex.schema.alterTable('task_locks', table => { - table.dropIndex([], 'task_locks_id_idx'); + // + // tasks + // + await knex.schema.alterTable('backstage_backend_common__tasks', table => { + table.dropIndex([], 'backstage_backend_common__tasks__id_idx'); }); - await knex.schema.dropTable('task_locks'); + await knex.schema.dropTable('backstage_backend_common__tasks'); + // + // locks + // + await knex.schema.alterTable( + 'backstage_backend_common__task_locks', + table => { + table.dropIndex([], 'backstage_backend_common__task_locks__id_idx'); + }, + ); + await knex.schema.dropTable('backstage_backend_common__task_locks'); }; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index c8e8d69ab5..a2255926e9 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -71,7 +71,8 @@ "unzipper": "^0.10.11", "uuid": "^8.0.0", "winston": "^3.2.1", - "yn": "^4.0.0" + "yn": "^4.0.0", + "zod": "^3.9.5" }, "peerDependencies": { "pg-connection-string": "^2.3.0" @@ -107,7 +108,8 @@ "msw": "^0.35.0", "mysql2": "^2.2.5", "recursive-readdir": "^2.2.2", - "supertest": "^6.1.3" + "supertest": "^6.1.3", + "wait-for-expect": "^3.0.2" }, "files": [ "dist", diff --git a/packages/backend-common/src/database/tables.ts b/packages/backend-common/src/database/tables.ts index 8efa393bb7..e566272ea6 100644 --- a/packages/backend-common/src/database/tables.ts +++ b/packages/backend-common/src/database/tables.ts @@ -14,9 +14,21 @@ * limitations under the License. */ -export type DbTaskLocksRow = { +export const DB_MUTEXES_TABLE = 'backstage_backend_common__mutexes'; +export const DB_TASKS_TABLE = 'backstage_backend_common__tasks'; + +export type DbMutexesRow = { id: string; - acquired_ticket?: string; - acquired_at?: Date; - expires_at?: Date; + current_lock_ticket?: string; + current_lock_acquired_at?: Date | string; + current_lock_expires_at?: Date | string; +}; + +export type DbTasksRow = { + id: string; + settings_json: string; + next_run_start_at?: Date | string; + current_run_ticket?: string; + current_run_started_at?: Date | string; + current_run_expires_at?: Date | string; }; diff --git a/packages/backend-common/src/tasks/CancelToken.ts b/packages/backend-common/src/tasks/CancelToken.ts new file mode 100644 index 0000000000..9ea9802e82 --- /dev/null +++ b/packages/backend-common/src/tasks/CancelToken.ts @@ -0,0 +1,48 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export class CancelToken { + // @ts-ignore: is actually assigned by the Promise constructor + #cancel: () => void; + #isCancelled: boolean; + #cancelPromise: Promise; + + static create(): CancelToken { + return new CancelToken(); + } + + private constructor() { + this.#isCancelled = false; + this.#cancelPromise = new Promise(resolve => { + this.#cancel = () => { + this.#isCancelled = true; + resolve(); + }; + }); + } + + cancel(): void { + this.#cancel(); + } + + get isCancelled(): boolean { + return this.#isCancelled; + } + + get promise(): Promise { + return this.#cancelPromise; + } +} diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts new file mode 100644 index 0000000000..ed5bf0911e --- /dev/null +++ b/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts @@ -0,0 +1,67 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TestDatabases } from '@backstage/backend-test-utils'; +import { Duration } from 'luxon'; +import { migrateBackendCommon } from '../database/migrateBackendCommon'; +import { getVoidLogger } from '../logging'; +import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; + +describe('PluginTaskManagerImpl', () => { + const databases = TestDatabases.create({ + ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], + }); + + describe('locking', () => { + it.each(databases.eachSupportedId())( + 'can run the happy path, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + + const manager = new PluginTaskManagerImpl( + async () => knex, + getVoidLogger(), + ); + + const lock1 = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + const lock2 = await manager.acquireLock('lock2', { + timeout: Duration.fromMillis(5000), + }); + + expect(lock1.acquired).toBe(true); + expect(lock2.acquired).toBe(true); + + await expect( + manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }), + ).resolves.toEqual({ acquired: false }); + + await (lock1 as any).release(); + await (lock2 as any).release(); + + const lock1Again = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + expect(lock1Again.acquired).toBe(true); + await (lock1Again as any).release(); + }, + ); + }); +}); diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts new file mode 100644 index 0000000000..8b9aad4388 --- /dev/null +++ b/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts @@ -0,0 +1,132 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { Duration } from 'luxon'; +import { v4 as uuid } from 'uuid'; +import { Logger } from 'winston'; +import { DbMutexesRow, DB_MUTEXES_TABLE } from '../database/tables'; +import { PluginTaskManagerJanitor } from './PluginTaskManagerJanitor'; +import { TaskWorker } from './TaskWorker'; +import { PluginTaskManager } from './types'; +import { nowPlus, validateId } from './util'; + +/** + * Implements the actual task management. + */ +export class PluginTaskManagerImpl implements PluginTaskManager { + private janitor: PluginTaskManagerJanitor | undefined; + + constructor( + private readonly databaseFactory: () => Promise, + private readonly logger: Logger, + ) {} + + async acquireLock( + id: string, + options: { + timeout: Duration; + }, + ): Promise< + | { acquired: false } + | { acquired: true; release: () => void | Promise } + > { + validateId(id); + + const knex = await this.databaseFactory(); + await this.ensureJanitor(knex); + + const ticket = uuid(); + + async function release() { + try { + await knex(DB_MUTEXES_TABLE) + .where('id', '=', id) + .where('current_lock_ticket', '=', ticket) + .delete(); + } catch { + // fail silently + } + } + + // First try to overwrite an existing lock, that has timed out + const stolen = await knex(DB_MUTEXES_TABLE) + .where('id', '=', id) + .whereNotNull('current_lock_ticket') + .where('current_lock_expires_at', '<', knex.fn.now()) + .update({ + current_lock_ticket: ticket, + current_lock_acquired_at: knex.fn.now(), + current_lock_expires_at: nowPlus(options.timeout, knex), + }); + + if (stolen) { + return { acquired: true, release }; + } + + try { + await knex(DB_MUTEXES_TABLE).insert({ + id, + current_lock_ticket: ticket, + current_lock_acquired_at: knex.fn.now(), + current_lock_expires_at: nowPlus(options.timeout, knex), + }); + return { acquired: true, release }; + } catch { + return { acquired: false }; + } + } + + async scheduleTask( + id: string, + options: { + timeout?: Duration; + frequency?: Duration; + initialDelay?: Duration; + }, + fn: () => void | Promise, + ): Promise<{ unschedule: () => Promise }> { + validateId(id); + + const knex = await this.databaseFactory(); + await this.ensureJanitor(knex); + + const task = new TaskWorker(id, fn, knex, this.logger); + await task.start({ + version: 1, + initialDelayDuration: options.initialDelay?.toISO(), + recurringAtMostEveryDuration: options.frequency?.toISO(), + timeoutAfterDuration: options.timeout?.toISO(), + }); + + return { + async unschedule() { + await task.stop(); + }, + }; + } + + private async ensureJanitor(knex: Knex) { + if (!this.janitor) { + this.janitor = new PluginTaskManagerJanitor({ + knex, + waitBetweenRuns: Duration.fromObject({ minutes: 1 }), + logger: this.logger, + }); + this.janitor.start(); + } + } +} diff --git a/packages/backend-common/src/tasks/TaskManager.test.ts b/packages/backend-common/src/tasks/TaskManager.test.ts index da9adb70f6..e6a34e58c1 100644 --- a/packages/backend-common/src/tasks/TaskManager.test.ts +++ b/packages/backend-common/src/tasks/TaskManager.test.ts @@ -17,9 +17,11 @@ import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; import { DatabaseManager } from '../database'; +import { getVoidLogger } from '../logging'; import { TaskManager } from './TaskManager'; describe('TaskManager', () => { + const logger = getVoidLogger(); const databases = TestDatabases.create({ ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], }); @@ -36,38 +38,16 @@ describe('TaskManager', () => { return databaseManager as DatabaseManager; } - describe('locking', () => { - it.each(databases.eachSupportedId())( - 'can run the happy path, %p', - async databaseId => { - const database = await createDatabase(databaseId); - const manager = new TaskManager(database).forPlugin('test'); + it.each(databases.eachSupportedId())( + 'can return a working plugin impl, %p', + async databaseId => { + const database = await createDatabase(databaseId); + const manager = new TaskManager(database, logger).forPlugin('test'); - const lock1 = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - const lock2 = await manager.acquireLock('lock2', { - timeout: Duration.fromMillis(5000), - }); - - expect(lock1.acquired).toBe(true); - expect(lock2.acquired).toBe(true); - - await expect( - manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }), - ).resolves.toEqual({ acquired: false }); - - await (lock1 as any).release(); - await (lock2 as any).release(); - - const lock1Again = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - expect(lock1Again.acquired).toBe(true); - await (lock1Again as any).release(); - }, - ); - }); + const lock = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + expect(lock.acquired).toBe(true); + }, + ); }); diff --git a/packages/backend-common/src/tasks/TaskManager.ts b/packages/backend-common/src/tasks/TaskManager.ts index 29741e1594..edb8e46609 100644 --- a/packages/backend-common/src/tasks/TaskManager.ts +++ b/packages/backend-common/src/tasks/TaskManager.ts @@ -15,104 +15,13 @@ */ import { Config } from '@backstage/config'; -import { Knex } from 'knex'; import { memoize } from 'lodash'; -import { Duration } from 'luxon'; -import { v4 as uuid } from 'uuid'; +import { Logger } from 'winston'; import { DatabaseManager } from '../database'; import { migrateBackendCommon } from '../database/migrateBackendCommon'; -import { DbTaskLocksRow } from '../database/tables'; +import { getRootLogger } from '../logging'; +import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; import { PluginTaskManager } from './types'; -import { validateId } from './util'; - -export class PluginTaskManagerImpl implements PluginTaskManager { - constructor( - private readonly pluginId: string, - private readonly databaseFactory: () => Promise, - ) {} - - async acquireLock( - idWithoutPrefix: string, - options: { - timeout: Duration; - }, - ): Promise< - | { acquired: false } - | { acquired: true; release: () => void | Promise } - > { - validateId(idWithoutPrefix); - - const knex = await this.databaseFactory(); - const id = `plugin:${this.pluginId}:${idWithoutPrefix}`; - const ticket = uuid(); - const timeout = options.timeout.as('seconds'); - - const release = async () => { - try { - await knex('backstage_backend_common__task_locks') - .where('id', '=', id) - .where('acquired_ticket', '=', ticket) - .delete(); - } catch { - // fail silently - } - }; - - // First try to overwrite an existing lock, that has timed out - const stolen = await knex( - 'backstage_backend_common__task_locks', - ) - .where('id', '=', id) - .whereNotNull('acquired_ticket') - .where('expires_at', '<', knex.fn.now()) - .update({ - acquired_ticket: ticket, - acquired_at: knex.fn.now(), - expires_at: - knex.client.config.client === 'sqlite3' - ? knex.raw(`datetime('now', ?)`, [`${timeout} seconds`]) - : knex.raw(`now() + interval '${timeout} seconds'`), - }); - - if (stolen) { - return { acquired: true, release }; - } - - try { - await knex('backstage_backend_common__task_locks').insert( - { - id, - acquired_ticket: ticket, - acquired_at: knex.fn.now(), - expires_at: - knex.client.config.client === 'sqlite3' - ? knex.raw(`datetime('now', ?)`, [`${timeout} seconds`]) - : knex.raw(`now() + interval '${timeout} seconds'`), - }, - ); - return { acquired: true, release }; - } catch { - return { acquired: false }; - } - } - - async scheduleTask( - idWithoutPrefix: string, - options: { - timeout: Duration; - frequency: Duration; - initialDelay?: Duration; - }, - fn: () => Promise, - ): Promise<{ release: () => Promise }> { - validateId(idWithoutPrefix); - - const knex = await this.databaseFactory(); - const id = `plugin:${this.pluginId}:${idWithoutPrefix}`; - - return {}; - } -} /** * Deals with management and locking related to distributed tasks. @@ -122,23 +31,34 @@ export class PluginTaskManagerImpl implements PluginTaskManager { export class TaskManager { static fromConfig( config: Config, - options?: { databaseManager?: DatabaseManager }, + options?: { + databaseManager?: DatabaseManager; + logger?: Logger; + }, ): TaskManager { const databaseManager = options?.databaseManager ?? DatabaseManager.fromConfig(config); - return new TaskManager(databaseManager); + const logger = (options?.logger || getRootLogger()).child({ + type: 'taskManager', + }); + return new TaskManager(databaseManager, logger); } - constructor(private readonly databaseManager: DatabaseManager) {} + constructor( + private readonly databaseManager: DatabaseManager, + private readonly logger: Logger, + ) {} forPlugin(pluginId: string): PluginTaskManager { + const databaseFactory = memoize(async () => { + const knex = await this.databaseManager.forPlugin(pluginId).getClient(); + await migrateBackendCommon(knex); + return knex; + }); + return new PluginTaskManagerImpl( - pluginId, - memoize(async () => { - const knex = await this.databaseManager.forPlugin(pluginId).getClient(); - await migrateBackendCommon(knex); - return knex; - }), + databaseFactory, + this.logger.child({ plugin: pluginId }), ); } } diff --git a/packages/backend-common/src/tasks/TaskWorker.test.ts b/packages/backend-common/src/tasks/TaskWorker.test.ts new file mode 100644 index 0000000000..ca0d624560 --- /dev/null +++ b/packages/backend-common/src/tasks/TaskWorker.test.ts @@ -0,0 +1,257 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { TestDatabases } from '@backstage/backend-test-utils'; +import { Duration } from 'luxon'; +import waitForExpect from 'wait-for-expect'; +import { migrateBackendCommon } from '../database/migrateBackendCommon'; +import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; +import { getVoidLogger } from '../logging'; +import { TaskWorker } from './TaskWorker'; +import { TaskSettingsV1 } from './types'; + +describe('TaskWorker', () => { + const logger = getVoidLogger(); + const databases = TestDatabases.create({ + ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it.each(databases.eachSupportedId())( + 'can run a single task to completion, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + + const fn = jest.fn( + async () => new Promise(resolve => setTimeout(resolve, 50)), + ); + const settings: TaskSettingsV1 = { + version: 1, + }; + + const worker = new TaskWorker('task1', fn, knex, logger); + await worker.start(settings); + + waitForExpect(() => { + expect(fn).toBeCalledTimes(1); + }); + }, + 60_000, + ); + + it.each(databases.eachSupportedId())( + 'goes through the expected states for a single run, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + + const fn = jest.fn( + async () => new Promise(resolve => setTimeout(resolve, 50)), + ); + const settings: TaskSettingsV1 = { + version: 1, + initialDelayDuration: Duration.fromObject({ seconds: 1 }).toISO(), + recurringAtMostEveryDuration: undefined, + timeoutAfterDuration: undefined, + }; + + const worker = new TaskWorker('task1', fn, knex, logger); + await worker.persistTask(settings); + + let row = (await knex(DB_TASKS_TABLE))[0]; + expect(row).toEqual( + expect.objectContaining({ + id: 'task1', + current_run_ticket: null, + current_run_started_at: null, + current_run_expires_at: null, + }), + ); + expect(JSON.parse(row.settings_json)).toEqual({ + version: 1, + initialDelayDuration: 'PT1S', + }); + + await expect(worker.findReadyTask()).resolves.toEqual({ + result: 'not ready yet', + }); + + waitForExpect(async () => { + await expect(worker.findReadyTask()).resolves.toEqual({ + result: 'ready', + }); + }); + + row = (await knex(DB_TASKS_TABLE))[0]; + expect(row).toEqual( + expect.objectContaining({ + id: 'task1', + current_run_ticket: null, + current_run_started_at: null, + current_run_expires_at: null, + }), + ); + + await expect(worker.tryClaimTask('ticket', settings)).resolves.toBe(true); + + row = (await knex(DB_TASKS_TABLE))[0]; + expect(row).toEqual( + expect.objectContaining({ + id: 'task1', + current_run_ticket: 'ticket', + current_run_started_at: expect.anything(), + current_run_expires_at: null, + }), + ); + + await expect(worker.tryReleaseTask('ticket', settings)).resolves.toBe( + true, + ); + + row = (await knex(DB_TASKS_TABLE))[0]; + expect(row).toBeUndefined(); + }, + 60_000, + ); + + it.each(databases.eachSupportedId())( + 'runs tasks more than once even when the task throws, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + + const fn = jest.fn().mockRejectedValue(new Error('failed')); + const settings: TaskSettingsV1 = { + version: 1, + initialDelayDuration: undefined, + recurringAtMostEveryDuration: Duration.fromMillis(0).toISO(), + timeoutAfterDuration: undefined, + }; + + const worker = new TaskWorker('task1', fn, knex, logger); + worker.start(settings); + + waitForExpect(() => { + expect(fn).toBeCalledTimes(3); + }); + }, + 60_000, + ); + + it.each(databases.eachSupportedId())( + 'does not clobber ticket lock when stolen, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + + const fn = jest.fn( + async () => new Promise(resolve => setTimeout(resolve, 50)), + ); + const settings: TaskSettingsV1 = { + version: 1, + recurringAtMostEveryDuration: Duration.fromMillis(0).toISO(), + }; + + const worker = new TaskWorker('task1', fn, knex, logger); + await worker.persistTask(settings); + await expect(worker.findReadyTask()).resolves.toEqual({ + result: 'ready', + settings, + }); + await expect(worker.tryClaimTask('ticket', settings)).resolves.toBe(true); + + let row = (await knex(DB_TASKS_TABLE))[0]; + expect(row).toEqual( + expect.objectContaining({ + id: 'task1', + current_run_ticket: 'ticket', + current_run_started_at: expect.anything(), + current_run_expires_at: null, + }), + ); + + await knex(DB_TASKS_TABLE) + .where('id', '=', 'task1') + .update({ current_run_ticket: 'stolen' }); + + await expect(worker.tryReleaseTask('ticket', settings)).resolves.toBe( + false, + ); + + row = (await knex(DB_TASKS_TABLE))[0]; + expect(row).toEqual( + expect.objectContaining({ + id: 'task1', + current_run_ticket: 'stolen', + current_run_started_at: expect.anything(), + current_run_expires_at: null, + }), + ); + }, + 60_000, + ); + + it.each(databases.eachSupportedId())( + 'gracefully handles a disappeared task row, %p', + async databaseId => { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + + const fn = jest.fn(async () => {}); + const settings: TaskSettingsV1 = { + version: 1, + recurringAtMostEveryDuration: Duration.fromMillis(0).toISO(), + }; + + const worker1 = new TaskWorker('task1', fn, knex, logger); + await worker1.persistTask(settings); + await knex(DB_TASKS_TABLE).where('id', '=', 'task1').delete(); + await expect(worker1.findReadyTask()).resolves.toEqual({ + result: 'abort', + }); + + const worker2 = new TaskWorker('task2', fn, knex, logger); + await worker2.persistTask(settings); + await expect(worker2.findReadyTask()).resolves.toEqual({ + result: 'ready', + settings, + }); + await knex(DB_TASKS_TABLE).where('id', '=', 'task2').delete(); + await expect(worker2.tryClaimTask('ticket', settings)).resolves.toBe( + false, + ); + + const worker3 = new TaskWorker('task3', fn, knex, logger); + await worker3.persistTask(settings); + await expect(worker3.findReadyTask()).resolves.toEqual({ + result: 'ready', + settings, + }); + await expect(worker3.tryClaimTask('ticket', settings)).resolves.toBe( + true, + ); + await knex(DB_TASKS_TABLE).where('id', '=', 'task3').delete(); + await expect(worker3.tryReleaseTask('ticket', settings)).resolves.toBe( + false, + ); + }, + 60_000, + ); +}); diff --git a/packages/backend-common/src/tasks/TaskWorker.ts b/packages/backend-common/src/tasks/TaskWorker.ts new file mode 100644 index 0000000000..3fd1f30716 --- /dev/null +++ b/packages/backend-common/src/tasks/TaskWorker.ts @@ -0,0 +1,277 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { Duration } from 'luxon'; +import { v4 as uuid } from 'uuid'; +import { Logger } from 'winston'; +import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; +import { CancelToken } from './CancelToken'; +import { TaskSettingsV1, taskSettingsV1Schema } from './types'; +import { nowPlus } from './util'; + +const WORK_CHECK_FREQUENCY = Duration.fromObject({ seconds: 5 }); + +/** + * Performs the actual work of a task. + * + * @private + */ +export class TaskWorker { + private readonly taskId: string; + private readonly fn: () => void | Promise; + private readonly knex: Knex; + private readonly logger: Logger; + private readonly cancelToken: CancelToken; + + constructor( + taskId: string, + fn: () => void | Promise, + knex: Knex, + logger: Logger, + ) { + this.taskId = taskId; + this.fn = fn; + this.knex = knex; + this.logger = logger; + this.cancelToken = CancelToken.create(); + } + + async start(settings: TaskSettingsV1) { + try { + await this.persistTask(settings); + } catch (e) { + throw new Error(`Failed to persist task, ${e}`); + } + + this.logger.info( + `Task worker starting: ${this.taskId}, ${JSON.stringify(settings)}`, + ); + + (async () => { + try { + while (!this.cancelToken.isCancelled) { + const runResult = await this.runOnce(); + if (runResult.result === 'abort') { + break; + } + if (!settings.recurringAtMostEveryDuration) { + break; + } + + await this.sleep(WORK_CHECK_FREQUENCY); + } + this.logger.info(`Task worker finished: ${this.taskId}`); + } catch (e) { + this.logger.warn(`Task worker failed unexpectedly, ${e}`); + } + })(); + } + + stop() { + this.cancelToken.cancel(); + } + + /** + * Makes a single attempt at running the task to completion, if ready. + * + * @returns The outcome of the attempt + */ + async runOnce(): Promise< + | { result: 'not ready yet' } + | { result: 'abort' } + | { result: 'failed' } + | { result: 'completed' } + > { + const findResult = await this.findReadyTask(); + if ( + findResult.result === 'not ready yet' || + findResult.result === 'abort' + ) { + return findResult; + } + + const taskSettings = findResult.settings; + const ticket = uuid(); + + const claimed = await this.tryClaimTask(ticket, taskSettings); + if (!claimed) { + return { result: 'not ready yet' }; + } + + try { + await this.fn(); + } catch (e) { + await this.tryReleaseTask(ticket, taskSettings); + return { result: 'failed' }; + } + + await this.tryReleaseTask(ticket, taskSettings); + return { result: 'completed' }; + } + + /** + * Sleep for the given duration, but abort sooner if the cancel token + * triggers. + * + * @param duration - The amount of time to sleep, at most + */ + private async sleep(duration: Duration): Promise { + await Promise.race([ + new Promise(resolve => setTimeout(resolve, duration.as('milliseconds'))), + this.cancelToken.promise, + ]); + } + + /** + * Perform the initial store of the task info + */ + async persistTask(settings: TaskSettingsV1) { + // Perform an initial parse to ensure that we will definitely be able to + // read it back again. + taskSettingsV1Schema.parse(settings); + + const settingsJson = JSON.stringify(settings); + const startAt = settings.initialDelayDuration + ? nowPlus(Duration.fromISO(settings.initialDelayDuration), this.knex) + : this.knex.fn.now(); + + // It's OK if the task already exists; if it does, just replace its + // settings with the new value and start the loop as usual. + await this.knex(DB_TASKS_TABLE) + .insert({ + id: this.taskId, + settings_json: settingsJson, + next_run_start_at: startAt, + }) + .onConflict('id') + .merge(['settings_json']); + } + + /** + * Check if the task is ready to run + */ + async findReadyTask(): Promise< + | { result: 'not ready yet' } + | { result: 'abort' } + | { result: 'ready'; settings: TaskSettingsV1 } + > { + const [row] = await this.knex(DB_TASKS_TABLE) + .where('id', '=', this.taskId) + .select({ + settingsJson: 'settings_json', + ready: this.knex.raw( + ` + CASE + WHEN next_run_start_at <= ? AND current_run_ticket IS NULL THEN TRUE + ELSE FALSE + END`, + [this.knex.fn.now()], + ), + }); + + if (!row) { + this.logger.info( + 'No longer able to find task; aborting and assuming that it has been unregistered or expired', + ); + return { result: 'abort' }; + } else if (!row.ready) { + return { result: 'not ready yet' }; + } + + try { + const settings = taskSettingsV1Schema.parse(JSON.parse(row.settingsJson)); + return { result: 'ready', settings }; + } catch (e) { + this.logger.info( + 'No longer able to parse task settings; aborting and assuming that a ' + + 'newer version of the task has been issued and being handled by ' + + `other workers, ${e}`, + ); + return { result: 'abort' }; + } + } + + /** + * Attempts to claim a task that's ready for execution, on this worker's + * behalf. We should not attempt to perform the work unless the claim really + * goes through. + * + * @param ticket - A globally unique string that changes for each invocation + * @param settings - The settings of the task to claim + * @returns True if it was successfully claimed + */ + async tryClaimTask( + ticket: string, + settings: TaskSettingsV1, + ): Promise { + const startedAt = this.knex.fn.now(); + const expiresAt = settings.timeoutAfterDuration + ? nowPlus(Duration.fromISO(settings.timeoutAfterDuration), this.knex) + : this.knex.raw('null'); + + const rows = await this.knex(DB_TASKS_TABLE) + .where('id', '=', this.taskId) + .whereNull('current_run_ticket') + .update({ + current_run_ticket: ticket, + current_run_started_at: startedAt, + current_run_expires_at: expiresAt, + }); + + return rows === 1; + } + + async tryReleaseTask( + ticket: string, + settings: TaskSettingsV1, + ): Promise { + const { recurringAtMostEveryDuration } = settings; + + // If this is not a recurring task, and we still have the current run + // ticket, delete it from the table + if (recurringAtMostEveryDuration === undefined) { + const rows = await this.knex(DB_TASKS_TABLE) + .where('id', '=', this.taskId) + .where('current_run_ticket', '=', ticket) + .delete(); + + return rows === 1; + } + + // We make an effort to keep the datetime calculations in the database + // layer, making sure to not have to perform conversions back and forth and + // leaning on the database as a central clock source + const dbNull = this.knex.raw('null'); + const dt = Duration.fromISO(recurringAtMostEveryDuration).as('seconds'); + const nextRun = + this.knex.client.config.client === 'sqlite3' + ? this.knex.raw('datetime(next_run_start_at, ?)', [`+${dt} seconds`]) + : this.knex.raw(`next_run_start_at + interval '${dt} seconds'`); + + const rows = await this.knex(DB_TASKS_TABLE) + .where('id', '=', this.taskId) + .where('current_run_ticket', '=', ticket) + .update({ + next_run_start_at: nextRun, + current_run_ticket: dbNull, + current_run_started_at: dbNull, + current_run_expires_at: dbNull, + }); + + return rows === 1; + } +} diff --git a/packages/backend-common/src/tasks/types.ts b/packages/backend-common/src/tasks/types.ts index 4a4fcb474e..4b061f9069 100644 --- a/packages/backend-common/src/tasks/types.ts +++ b/packages/backend-common/src/tasks/types.ts @@ -15,6 +15,7 @@ */ import { Duration } from 'luxon'; +import { z } from 'zod'; /** * Deals with management and locking related to distributed tasks, for a given @@ -41,5 +42,34 @@ export interface PluginTaskManager { initialDelay?: Duration; }, fn: () => Promise, - ): Promise<{ release: () => Promise }>; + ): Promise<{ unschedule: () => Promise }>; } + +function isValidOptionalDurationString(d: string | undefined): boolean { + try { + return !d || Duration.fromISO(d).isValid === true; + } catch { + return false; + } +} + +export const taskSettingsV1Schema = z.object({ + version: z.literal(1), + initialDelayDuration: z + .string() + .optional() + .refine(isValidOptionalDurationString, { message: 'Invalid duration' }), + recurringAtMostEveryDuration: z + .string() + .optional() + .refine(isValidOptionalDurationString, { message: 'Invalid duration' }), + timeoutAfterDuration: z + .string() + .optional() + .refine(isValidOptionalDurationString, { message: 'Invalid duration' }), +}); + +/** + * The properties that control a scheduled task (version 1). + */ +export type TaskSettingsV1 = z.infer; diff --git a/packages/backend-common/src/tasks/util.ts b/packages/backend-common/src/tasks/util.ts index 7bd4ff61b5..dd5c2d4b7c 100644 --- a/packages/backend-common/src/tasks/util.ts +++ b/packages/backend-common/src/tasks/util.ts @@ -15,6 +15,8 @@ */ import { InputError } from '@backstage/errors'; +import { Knex } from 'knex'; +import { DateTime, Duration } from 'luxon'; // Keep the IDs compatible with e.g. Prometheus export function validateId(id: string) { @@ -24,3 +26,20 @@ export function validateId(id: string) { ); } } + +export function dbTime(t: Date | string): DateTime { + if (typeof t === 'string') { + return DateTime.fromSQL(t); + } + return DateTime.fromJSDate(t); +} + +export function nowPlus(duration: Duration | undefined, knex: Knex) { + const seconds = duration?.as('seconds') ?? 0; + if (!seconds) { + return knex.fn.now(); + } + return knex.client.config.client === 'sqlite3' + ? knex.raw(`datetime('now', ?)`, [`${seconds} seconds`]) + : knex.raw(`now() + interval '${seconds} seconds'`); +} diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 554c9a7d99..4e44729200 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -57,7 +57,7 @@ import techInsights from './plugins/techInsights'; import todo from './plugins/todo'; import { PluginEnvironment } from './types'; -async function makeCreateEnv(config: Config) { +function makeCreateEnv(config: Config) { const root = getRootLogger(); const reader = UrlReaders.default({ logger: root, config }); const discovery = SingleHostDiscovery.fromConfig(config); @@ -90,7 +90,7 @@ async function main() { argv: process.argv, logger, }); - const createEnv = await makeCreateEnv(config); + const createEnv = makeCreateEnv(config); const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck')); const catalogEnv = useHotMemoize(module, () => createEnv('catalog')); diff --git a/yarn.lock b/yarn.lock index caba3db5fb..4c42941ad9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20895,11 +20895,6 @@ node-abi@^2.21.0: dependencies: semver "^5.4.1" -node-abort-controller@^3.0.0: - version "3.0.0" - resolved "https://registry.npmjs.org/node-abort-controller/-/node-abort-controller-3.0.0.tgz#e7b1f2b72f4c5a74b5594cc23fa8fe3b62010ab5" - integrity sha512-IqMCPbihDpbHV4bNws015hU0svIBGyzPjJearwXMGJyungWdblbBcboNojTz9bWOrrJD3zIwmcr5w5c+NH+2+A== - node-addon-api@^3.0.0: version "3.2.1" resolved "https://registry.npmjs.org/node-addon-api/-/node-addon-api-3.2.1.tgz#81325e0a2117789c0128dab65e7e38f07ceba161" From 751317dbf3faccb05188f480773a1702b8b9da37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 14 Oct 2021 17:04:24 +0200 Subject: [PATCH 03/27] finishing touches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/api-report.md | 3 + packages/backend-common/package.json | 1 - packages/backend-common/src/database/index.ts | 1 + .../src/database/migrateBackendCommon.ts | 3 +- .../backend-common/src/database/tables.ts | 1 + packages/backend-common/src/database/util.ts | 33 +++++ .../src/tasks/PluginTaskManagerImpl.test.ts | 89 ++++++++++++-- .../src/tasks/PluginTaskManagerImpl.ts | 44 +++---- .../src/tasks/PluginTaskManagerJanitor.ts | 114 ++++++++++++++++++ .../backend-common/src/tasks/TaskManager.ts | 11 ++ 10 files changed, 260 insertions(+), 40 deletions(-) create mode 100644 packages/backend-common/src/database/util.ts create mode 100644 packages/backend-common/src/tasks/PluginTaskManagerJanitor.ts diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 93c25bca24..480b6419fa 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -375,6 +375,9 @@ export class GitlabUrlReader implements UrlReader { export { isChildPath }; +// @public +export function isDatabaseConflictError(e: unknown): boolean; + // @public export function loadBackendConfig(options: { logger: Logger_2; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index a2255926e9..e0d4be1242 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -63,7 +63,6 @@ "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", - "node-abort-controller": "^3.0.0", "raw-body": "^2.4.1", "selfsigned": "^1.10.7", "stoppable": "^1.1.0", diff --git a/packages/backend-common/src/database/index.ts b/packages/backend-common/src/database/index.ts index 7fcb8bf930..2a820f7367 100644 --- a/packages/backend-common/src/database/index.ts +++ b/packages/backend-common/src/database/index.ts @@ -28,3 +28,4 @@ export { } from './connection'; export type { PluginDatabaseManager } from './types'; +export { isDatabaseConflictError } from './util'; diff --git a/packages/backend-common/src/database/migrateBackendCommon.ts b/packages/backend-common/src/database/migrateBackendCommon.ts index 6283f3a96c..104e757e0b 100644 --- a/packages/backend-common/src/database/migrateBackendCommon.ts +++ b/packages/backend-common/src/database/migrateBackendCommon.ts @@ -16,6 +16,7 @@ import { Knex } from 'knex'; import { resolvePackagePath } from '../paths'; +import { DB_MIGRATIONS_TABLE } from './tables'; const migrationsDir = resolvePackagePath( '@backstage/backend-common', @@ -25,6 +26,6 @@ const migrationsDir = resolvePackagePath( export async function migrateBackendCommon(knex: Knex): Promise { await knex.migrate.latest({ directory: migrationsDir, - tableName: 'knex_migrations_backstage_backend_common', + tableName: DB_MIGRATIONS_TABLE, }); } diff --git a/packages/backend-common/src/database/tables.ts b/packages/backend-common/src/database/tables.ts index e566272ea6..90e8aeca40 100644 --- a/packages/backend-common/src/database/tables.ts +++ b/packages/backend-common/src/database/tables.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +export const DB_MIGRATIONS_TABLE = 'backstage_backend_common__knex_migrations'; export const DB_MUTEXES_TABLE = 'backstage_backend_common__mutexes'; export const DB_TASKS_TABLE = 'backstage_backend_common__tasks'; diff --git a/packages/backend-common/src/database/util.ts b/packages/backend-common/src/database/util.ts new file mode 100644 index 0000000000..607554f294 --- /dev/null +++ b/packages/backend-common/src/database/util.ts @@ -0,0 +1,33 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Tries to deduce whether a thrown error is a database conflict. + * + * @public + * @param e - A thrown error + * @returns True if the error looks like it was a conflict error thrown by a + * known database engine + */ +export function isDatabaseConflictError(e: unknown) { + const message = (e as any)?.message; + + return ( + typeof message === 'string' && + (/SQLITE_CONSTRAINT: UNIQUE/.test(message) || + /unique constraint/.test(message)) + ); +} diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts index ed5bf0911e..ef5f6db96f 100644 --- a/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts @@ -14,8 +14,9 @@ * limitations under the License. */ -import { TestDatabases } from '@backstage/backend-test-utils'; +import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; +import waitForExpect from 'wait-for-expect'; import { migrateBackendCommon } from '../database/migrateBackendCommon'; import { getVoidLogger } from '../logging'; import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; @@ -25,17 +26,21 @@ describe('PluginTaskManagerImpl', () => { ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], }); - describe('locking', () => { + async function init(databaseId: TestDatabaseId) { + const knex = await databases.init(databaseId); + await migrateBackendCommon(knex); + const manager = new PluginTaskManagerImpl( + async () => knex, + getVoidLogger(), + ); + return { knex, manager }; + } + + describe('acquireLock', () => { it.each(databases.eachSupportedId())( 'can run the happy path, %p', async databaseId => { - const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); - - const manager = new PluginTaskManagerImpl( - async () => knex, - getVoidLogger(), - ); + const { manager } = await init(databaseId); const lock1 = await manager.acquireLock('lock1', { timeout: Duration.fromMillis(5000), @@ -63,5 +68,71 @@ describe('PluginTaskManagerImpl', () => { await (lock1Again as any).release(); }, ); + + it.each(databases.eachSupportedId())( + 'rejects double lock attempts, %p', + async databaseId => { + const { manager } = await init(databaseId); + + const lock1 = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + const lock2 = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + + expect(lock1.acquired).toBe(true); + expect(lock2.acquired).toBe(false); + + await (lock1 as any).release(); + + const lock1Again = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + expect(lock1Again.acquired).toBe(true); + await (lock1Again as any).release(); + }, + ); + + it.each(databases.eachSupportedId())( + 'times out locks, %p', + async databaseId => { + const { manager } = await init(databaseId); + + const lock1 = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(200), + }); + + expect(lock1.acquired).toBe(true); + + await new Promise(resolve => setTimeout(resolve, 1000)); + + const lock2 = await manager.acquireLock('lock1', { + timeout: Duration.fromMillis(5000), + }); + expect(lock2.acquired).toBe(true); + await (lock2 as any).release(); + }, + ); + }); + + // This is just to test the wrapper code; most of the actual tests are in + // TaskWorker.test.ts + describe('scheduleTask', () => { + it.each(databases.eachSupportedId())( + 'can run the happy path, %p', + async databaseId => { + const { manager } = await init(databaseId); + + const fn = jest.fn(); + const { unschedule } = await manager.scheduleTask('task1', {}, fn); + + await waitForExpect(() => { + expect(fn).toBeCalled(); + }); + + await unschedule(); + }, + ); }); }); diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts index 8b9aad4388..2d1f192fe6 100644 --- a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts @@ -18,8 +18,8 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; +import { isDatabaseConflictError } from '../database'; import { DbMutexesRow, DB_MUTEXES_TABLE } from '../database/tables'; -import { PluginTaskManagerJanitor } from './PluginTaskManagerJanitor'; import { TaskWorker } from './TaskWorker'; import { PluginTaskManager } from './types'; import { nowPlus, validateId } from './util'; @@ -28,8 +28,6 @@ import { nowPlus, validateId } from './util'; * Implements the actual task management. */ export class PluginTaskManagerImpl implements PluginTaskManager { - private janitor: PluginTaskManagerJanitor | undefined; - constructor( private readonly databaseFactory: () => Promise, private readonly logger: Logger, @@ -47,8 +45,6 @@ export class PluginTaskManagerImpl implements PluginTaskManager { validateId(id); const knex = await this.databaseFactory(); - await this.ensureJanitor(knex); - const ticket = uuid(); async function release() { @@ -62,30 +58,32 @@ export class PluginTaskManagerImpl implements PluginTaskManager { } } + const record: Knex.DbRecord = { + current_lock_ticket: ticket, + current_lock_acquired_at: knex.fn.now(), + current_lock_expires_at: options.timeout + ? nowPlus(options.timeout, knex) + : knex.raw('null'), + }; + // First try to overwrite an existing lock, that has timed out const stolen = await knex(DB_MUTEXES_TABLE) .where('id', '=', id) .whereNotNull('current_lock_ticket') .where('current_lock_expires_at', '<', knex.fn.now()) - .update({ - current_lock_ticket: ticket, - current_lock_acquired_at: knex.fn.now(), - current_lock_expires_at: nowPlus(options.timeout, knex), - }); + .update(record); if (stolen) { return { acquired: true, release }; } try { - await knex(DB_MUTEXES_TABLE).insert({ - id, - current_lock_ticket: ticket, - current_lock_acquired_at: knex.fn.now(), - current_lock_expires_at: nowPlus(options.timeout, knex), - }); + await knex(DB_MUTEXES_TABLE).insert({ id, ...record }); return { acquired: true, release }; - } catch { + } catch (e) { + if (!isDatabaseConflictError(e)) { + this.logger.warn(`Failed to acquire lock, ${e}`); + } return { acquired: false }; } } @@ -102,7 +100,6 @@ export class PluginTaskManagerImpl implements PluginTaskManager { validateId(id); const knex = await this.databaseFactory(); - await this.ensureJanitor(knex); const task = new TaskWorker(id, fn, knex, this.logger); await task.start({ @@ -118,15 +115,4 @@ export class PluginTaskManagerImpl implements PluginTaskManager { }, }; } - - private async ensureJanitor(knex: Knex) { - if (!this.janitor) { - this.janitor = new PluginTaskManagerJanitor({ - knex, - waitBetweenRuns: Duration.fromObject({ minutes: 1 }), - logger: this.logger, - }); - this.janitor.start(); - } - } } diff --git a/packages/backend-common/src/tasks/PluginTaskManagerJanitor.ts b/packages/backend-common/src/tasks/PluginTaskManagerJanitor.ts new file mode 100644 index 0000000000..26af986025 --- /dev/null +++ b/packages/backend-common/src/tasks/PluginTaskManagerJanitor.ts @@ -0,0 +1,114 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { Duration } from 'luxon'; +import { Logger } from 'winston'; +import { + DbMutexesRow, + DbTasksRow, + DB_MUTEXES_TABLE, + DB_TASKS_TABLE, +} from '../database/tables'; +import { CancelToken } from './CancelToken'; + +/** + * Makes sure to auto-expire and clean up things that time out or for other + * reasons should not be left lingering. + */ +export class PluginTaskManagerJanitor { + private readonly knex: Knex; + private readonly waitBetweenRuns: Duration; + private readonly logger: Logger; + private readonly cancelToken: CancelToken; + + constructor(options: { + knex: Knex; + waitBetweenRuns: Duration; + logger: Logger; + }) { + this.knex = options.knex; + this.waitBetweenRuns = options.waitBetweenRuns; + this.logger = options.logger; + this.cancelToken = CancelToken.create(); + } + + async start() { + while (!this.cancelToken.isCancelled) { + try { + await this.runOnce(); + } catch (e) { + this.logger.warn(`Error while performing janitorial tasks, ${e}`); + } + + await this.sleep(this.waitBetweenRuns); + } + } + + async stop() { + this.cancelToken.cancel(); + } + + private async runOnce() { + // SQLite currently (Oct 1 2021) returns a number for returning() + // statements, effectively ignoring them and instead returning the outcome + // of the delete() - and knex also emits a warning about that fact, which + // is why we avoid that entirely for the sqlite3 driver. + // https://github.com/knex/knex/issues/4370 + // https://github.com/mapbox/node-sqlite3/issues/1453 + + const mutexesQuery = this.knex(DB_MUTEXES_TABLE) + .where('current_lock_expires_at', '<', this.knex.fn.now()) + .delete(); + + if (this.knex.client.config.client === 'sqlite3') { + const mutexes = await mutexesQuery; + if (mutexes > 0) { + this.logger.warn(`${mutexes} mutex locks timed out and were lost`); + } + } else { + const mutexes = await mutexesQuery.returning(['id']); + for (const { id } of mutexes) { + this.logger.warn(`Mutex lock timed out and was lost: ${id}`); + } + } + + const tasksQuery = this.knex(DB_TASKS_TABLE) + .where('current_run_expires_at', '<', this.knex.fn.now()) + .delete(); + + if (this.knex.client.config.client === 'sqlite3') { + const tasks = await tasksQuery; + this.logger.warn(`${tasks} tasks timed out and were lost`); + } else { + const tasks = await tasksQuery.returning(['id']); + for (const { id } of tasks) { + this.logger.warn(`Task timed out and was lost: ${id}`); + } + } + } + + /** + * Sleeps for the given duration, but aborts sooner if the cancel token + * triggers. + */ + private async sleep(duration: Duration) { + await Promise.race([ + new Promise(resolve => setTimeout(resolve, duration.as('milliseconds'))), + this.cancelToken.promise, + ]); + } +} diff --git a/packages/backend-common/src/tasks/TaskManager.ts b/packages/backend-common/src/tasks/TaskManager.ts index edb8e46609..ea49c2383e 100644 --- a/packages/backend-common/src/tasks/TaskManager.ts +++ b/packages/backend-common/src/tasks/TaskManager.ts @@ -16,11 +16,13 @@ import { Config } from '@backstage/config'; import { memoize } from 'lodash'; +import { Duration } from 'luxon'; import { Logger } from 'winston'; import { DatabaseManager } from '../database'; import { migrateBackendCommon } from '../database/migrateBackendCommon'; import { getRootLogger } from '../logging'; import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; +import { PluginTaskManagerJanitor } from './PluginTaskManagerJanitor'; import { PluginTaskManager } from './types'; /** @@ -52,7 +54,16 @@ export class TaskManager { forPlugin(pluginId: string): PluginTaskManager { const databaseFactory = memoize(async () => { const knex = await this.databaseManager.forPlugin(pluginId).getClient(); + await migrateBackendCommon(knex); + + const janitor = new PluginTaskManagerJanitor({ + knex, + waitBetweenRuns: Duration.fromObject({ minutes: 1 }), + logger: this.logger, + }); + janitor.start(); + return knex; }); From d7c1e0e34acf52172e879dce7a81c95ec8baf435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 15 Oct 2021 10:42:59 +0200 Subject: [PATCH 04/27] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/happy-rice-tickle.md | 5 + .github/styles/vocab.txt | 3 + packages/backend-common/api-report.md | 29 ++--- packages/backend-common/package.json | 1 + .../src/tasks/PluginTaskManagerImpl.ts | 16 +-- .../backend-common/src/tasks/TaskManager.ts | 6 ++ packages/backend-common/src/tasks/index.ts | 2 +- packages/backend-common/src/tasks/types.ts | 101 ++++++++++++++++-- 8 files changed, 126 insertions(+), 37 deletions(-) create mode 100644 .changeset/happy-rice-tickle.md diff --git a/.changeset/happy-rice-tickle.md b/.changeset/happy-rice-tickle.md new file mode 100644 index 0000000000..475f03501c --- /dev/null +++ b/.changeset/happy-rice-tickle.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Add support for distributed mutexes and scheduled tasks, through the `TaskManager` class. This class can be particularly useful for coordinating things across many deployed instances of a given backend plugin. An example of this is catalog entity providers - with this facility you can register tasks similar to a cron job, and make sure that only one host at a time tries to execute the job, and that the timing (call frequency, timeouts etc) are retained as a global concern, letting you scale your workload safely without affecting the task behavior. diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index 7e478953bc..c14047cd4d 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -52,6 +52,7 @@ configmaps configs const cookiecutter +cron css Datadog dataflow @@ -162,6 +163,8 @@ Monorepo monorepos msgraph msw +mutex +mutexes mysql namespace namespaced diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 480b6419fa..688c74159e 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -384,6 +384,11 @@ export function loadBackendConfig(options: { argv: string[]; }): Promise; +// @public +export interface LockOptions { + timeout: Duration; +} + // @public export function notFoundHandler(): RequestHandler; @@ -405,30 +410,22 @@ export type PluginEndpointDiscovery = { // @public export interface PluginTaskManager { - // (undocumented) acquireLock( id: string, - options: { - timeout: Duration; - }, + options: LockOptions, ): Promise< | { acquired: false; } | { acquired: true; - release: () => void | Promise; + release(): Promise; } >; - // (undocumented) scheduleTask( id: string, - options: { - timeout: Duration; - frequency: Duration; - initialDelay?: Duration; - }, - fn: () => Promise, + options: TaskOptions, + fn: () => void | Promise, ): Promise<{ unschedule: () => Promise; }>; @@ -614,7 +611,6 @@ export interface StatusCheckHandlerOptions { // @public export class TaskManager { constructor(databaseManager: DatabaseManager, logger: Logger_2); - // (undocumented) forPlugin(pluginId: string): PluginTaskManager; // (undocumented) static fromConfig( @@ -626,6 +622,13 @@ export class TaskManager { ): TaskManager; } +// @public +export interface TaskOptions { + frequency?: Duration; + initialDelay?: Duration; + timeout?: Duration; +} + // @public export type UrlReader = { read(url: string): Promise; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index e0d4be1242..555c2004ee 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -90,6 +90,7 @@ "@types/concat-stream": "^1.6.0", "@types/fs-extra": "^9.0.3", "@types/http-errors": "^1.6.3", + "@types/luxon": "^2.0.4", "@types/minimist": "^1.2.0", "@types/mock-fs": "^4.13.0", "@types/morgan": "^1.9.0", diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts index 2d1f192fe6..540c7d501a 100644 --- a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts @@ -15,13 +15,12 @@ */ import { Knex } from 'knex'; -import { Duration } from 'luxon'; import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; import { isDatabaseConflictError } from '../database'; import { DbMutexesRow, DB_MUTEXES_TABLE } from '../database/tables'; import { TaskWorker } from './TaskWorker'; -import { PluginTaskManager } from './types'; +import { LockOptions, PluginTaskManager, TaskOptions } from './types'; import { nowPlus, validateId } from './util'; /** @@ -35,12 +34,9 @@ export class PluginTaskManagerImpl implements PluginTaskManager { async acquireLock( id: string, - options: { - timeout: Duration; - }, + options: LockOptions, ): Promise< - | { acquired: false } - | { acquired: true; release: () => void | Promise } + { acquired: false } | { acquired: true; release(): Promise } > { validateId(id); @@ -90,11 +86,7 @@ export class PluginTaskManagerImpl implements PluginTaskManager { async scheduleTask( id: string, - options: { - timeout?: Duration; - frequency?: Duration; - initialDelay?: Duration; - }, + options: TaskOptions, fn: () => void | Promise, ): Promise<{ unschedule: () => Promise }> { validateId(id); diff --git a/packages/backend-common/src/tasks/TaskManager.ts b/packages/backend-common/src/tasks/TaskManager.ts index ea49c2383e..db8228561f 100644 --- a/packages/backend-common/src/tasks/TaskManager.ts +++ b/packages/backend-common/src/tasks/TaskManager.ts @@ -51,6 +51,12 @@ export class TaskManager { private readonly logger: Logger, ) {} + /** + * Instantiates a task manager instance for the given plugin. + * + * @param pluginId - The unique ID of the plugin, for example "catalog" + * @returns A {@link PluginTaskManager} instance + */ forPlugin(pluginId: string): PluginTaskManager { const databaseFactory = memoize(async () => { const knex = await this.databaseManager.forPlugin(pluginId).getClient(); diff --git a/packages/backend-common/src/tasks/index.ts b/packages/backend-common/src/tasks/index.ts index 60329788b3..9f86f8fc64 100644 --- a/packages/backend-common/src/tasks/index.ts +++ b/packages/backend-common/src/tasks/index.ts @@ -14,5 +14,5 @@ * limitations under the License. */ -export type { PluginTaskManager } from './types'; export { TaskManager } from './TaskManager'; +export type { LockOptions, PluginTaskManager, TaskOptions } from './types'; diff --git a/packages/backend-common/src/tasks/types.ts b/packages/backend-common/src/tasks/types.ts index 4b061f9069..a8d356bed9 100644 --- a/packages/backend-common/src/tasks/types.ts +++ b/packages/backend-common/src/tasks/types.ts @@ -17,6 +17,64 @@ import { Duration } from 'luxon'; import { z } from 'zod'; +/** + * Options that apply to the acquiral of a given lock. + * + * @public + */ +export interface LockOptions { + /** + * The maximum amount of time that the lock can be held, before it's + * considered timed out and gets auto-released by the framework. + */ + timeout: Duration; +} + +/** + * Options that apply to the invocation of a given task. + * + * @public + */ +export interface TaskOptions { + /** + * The maximum amount of time that a single task invocation can take, before + * it's considered timed out and gets "released" such that a new invocation + * is permitted to take place (possibly, then, on a different worker). + * + * If no value is given for this field then there is no timeout. This is + * potentially dangerous. + */ + timeout?: Duration; + + /** + * The amount of time that should pass between task invocation starts. + * Essentially, this equals roughly how often you want the task to run. + * + * This is a best effort value; under some circumstances there can be + * deviations. For example, if the task runtime is longer than the frequency + * and the timeout has not been given or not been exceeded yet, the next + * invocation of this task will be delayed until after the previous one + * finishes. + * + * The system does its best to avoid overlapping invocations. + * + * If no value is given for this field then the task will only be invoked + * once (on any worker) and then unscheduled automatically. + */ + frequency?: Duration; + + /** + * The amount of time that should pass before the first invocation happens. + * + * This can be useful in cold start scenarios to stagger or delay some heavy + * compute jobs. + * + * If no value is given for this field then the first invocation will happen + * as soon as possible. + */ + initialDelay?: Duration; +} + /** * Deals with management and locking related to distributed tasks, for a given * plugin. @@ -24,24 +82,45 @@ import { z } from 'zod'; * @public */ export interface PluginTaskManager { + /** + * Attempts to acquire an exclusive lock. + * + * A lock can only be held by one party at a time. Any subsequent attempts to + * acquire the lock will fail, unless the timeout period has been exceeded or + * the lock was released by the previous holder. + * + * @param id - A unique ID (within the scope of the plugin) for a lock + * @param options - Options for the lock + * @returns The result of the lock attempt. If it was successfully acquired, + * you should remember to call its `release` method as soon as you + * are done with the lock. + */ acquireLock( id: string, - options: { - timeout: Duration; - }, + options: LockOptions, ): Promise< - | { acquired: false } - | { acquired: true; release: () => void | Promise } + { acquired: false } | { acquired: true; release(): Promise } >; + /** + * Schedules a task function for coordinated exclusive invocation across + * workers. + * + * If the task was already scheduled since before by us or by another party, + * its options are just overwritten with the given options, and things + * continue from there. + * + * @param id - A unique ID (within the scope of the plugin) for the task + * @param options - Options for the task + * @param fn - The actual task function to be invoked + * @returns An `unschedule` function that can be used to stop the task + * invocations later on. This removes the task entirely from storage + * and stops its invocations across all workers. + */ scheduleTask( id: string, - options: { - timeout: Duration; - frequency: Duration; - initialDelay?: Duration; - }, - fn: () => Promise, + options: TaskOptions, + fn: () => void | Promise, ): Promise<{ unschedule: () => Promise }>; } From 222793c849958e613f2fcae3a482e05368e20a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 15 Oct 2021 11:23:05 +0200 Subject: [PATCH 05/27] do not modify the backend yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/package.json | 2 +- packages/backend/src/index.ts | 25 +++++++++++-------------- packages/backend/src/types.ts | 6 ++---- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 555c2004ee..47345c198d 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -41,6 +41,7 @@ "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", + "@types/luxon": "^2.0.4", "archiver": "^5.0.2", "aws-sdk": "^2.840.0", "compression": "^1.7.4", @@ -90,7 +91,6 @@ "@types/concat-stream": "^1.6.0", "@types/fs-extra": "^9.0.3", "@types/http-errors": "^1.6.3", - "@types/luxon": "^2.0.4", "@types/minimist": "^1.2.0", "@types/mock-fs": "^4.13.0", "@types/morgan": "^1.9.0", diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 4e44729200..f978e84da9 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -22,39 +22,38 @@ * Happy hacking! */ +import Router from 'express-promise-router'; import { CacheManager, createServiceBuilder, - DatabaseManager, getRootLogger, loadBackendConfig, notFoundHandler, + DatabaseManager, SingleHostDiscovery, - TaskManager, UrlReaders, useHotMemoize, } from '@backstage/backend-common'; import { Config } from '@backstage/config'; -import Router from 'express-promise-router'; -import { metricsHandler, metricsInit } from './metrics'; -import app from './plugins/app'; +import healthcheck from './plugins/healthcheck'; +import { metricsInit, metricsHandler } from './metrics'; import auth from './plugins/auth'; import azureDevOps from './plugins/azure-devops'; -import badges from './plugins/badges'; import catalog from './plugins/catalog'; import codeCoverage from './plugins/codecoverage'; -import graphql from './plugins/graphql'; -import healthcheck from './plugins/healthcheck'; -import jenkins from './plugins/jenkins'; -import kafka from './plugins/kafka'; import kubernetes from './plugins/kubernetes'; -import proxy from './plugins/proxy'; +import kafka from './plugins/kafka'; import rollbar from './plugins/rollbar'; import scaffolder from './plugins/scaffolder'; +import proxy from './plugins/proxy'; import search from './plugins/search'; import techdocs from './plugins/techdocs'; import techInsights from './plugins/techInsights'; import todo from './plugins/todo'; +import graphql from './plugins/graphql'; +import app from './plugins/app'; +import badges from './plugins/badges'; +import jenkins from './plugins/jenkins'; import { PluginEnvironment } from './types'; function makeCreateEnv(config: Config) { @@ -65,15 +64,13 @@ function makeCreateEnv(config: Config) { root.info(`Created UrlReader ${reader}`); const databaseManager = DatabaseManager.fromConfig(config); - const taskManager = TaskManager.fromConfig(config); const cacheManager = CacheManager.fromConfig(config); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); const database = databaseManager.forPlugin(plugin); - const tasks = taskManager.forPlugin(plugin); const cache = cacheManager.forPlugin(plugin); - return { logger, cache, database, tasks, config, reader, discovery }; + return { logger, cache, database, config, reader, discovery }; }; } diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index aafd25bc3f..8290e569ef 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -14,21 +14,19 @@ * limitations under the License. */ +import { Logger } from 'winston'; +import { Config } from '@backstage/config'; import { PluginCacheManager, PluginDatabaseManager, PluginEndpointDiscovery, - PluginTaskManager, UrlReader, } from '@backstage/backend-common'; -import { Config } from '@backstage/config'; -import { Logger } from 'winston'; export type PluginEnvironment = { logger: Logger; cache: PluginCacheManager; database: PluginDatabaseManager; - tasks: PluginTaskManager; config: Config; reader: UrlReader; discovery: PluginEndpointDiscovery; From e09bf604cd6989a463a326bbae228ccc736b3ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 24 Oct 2021 19:08:50 +0200 Subject: [PATCH 06/27] move to a separate package instead and address comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/happy-rice-tickle.md | 2 +- packages/backend-common/api-report.md | 50 ---------------- packages/backend-common/package.json | 11 +--- packages/backend-common/src/index.ts | 1 - packages/backend-tasks/.eslintrc.js | 3 + packages/backend-tasks/README.md | 36 +++++++++++ packages/backend-tasks/api-report.md | 59 +++++++++++++++++++ .../knexfile.js | 0 .../migrations/20210928160613_init.js | 27 ++++----- packages/backend-tasks/package.json | 54 +++++++++++++++++ .../src/database/migrateBackendTasks.ts} | 6 +- .../src/database/tables.ts | 8 +-- packages/backend-tasks/src/index.ts | 23 ++++++++ packages/backend-tasks/src/setupTests.ts | 17 ++++++ .../src/tasks/CancelToken.ts | 0 .../src/tasks/PluginTaskManagerImpl.test.ts | 6 +- .../src/tasks/PluginTaskManagerImpl.ts | 3 +- .../src/tasks/PluginTaskManagerJanitor.ts | 0 .../src/tasks/TaskManager.test.ts | 3 +- .../src/tasks/TaskManager.ts | 7 +-- .../src/tasks/TaskWorker.test.ts | 14 ++--- .../src/tasks/TaskWorker.ts | 0 .../src/tasks/index.ts | 0 .../src/tasks/types.ts | 0 .../src/tasks/util.ts | 0 25 files changed, 229 insertions(+), 101 deletions(-) create mode 100644 packages/backend-tasks/.eslintrc.js create mode 100644 packages/backend-tasks/README.md create mode 100644 packages/backend-tasks/api-report.md rename packages/{backend-common => backend-tasks}/knexfile.js (100%) rename packages/{backend-common => backend-tasks}/migrations/20210928160613_init.js (75%) create mode 100644 packages/backend-tasks/package.json rename packages/{backend-common/src/database/migrateBackendCommon.ts => backend-tasks/src/database/migrateBackendTasks.ts} (83%) rename packages/{backend-common => backend-tasks}/src/database/tables.ts (79%) create mode 100644 packages/backend-tasks/src/index.ts create mode 100644 packages/backend-tasks/src/setupTests.ts rename packages/{backend-common => backend-tasks}/src/tasks/CancelToken.ts (100%) rename packages/{backend-common => backend-tasks}/src/tasks/PluginTaskManagerImpl.test.ts (96%) rename packages/{backend-common => backend-tasks}/src/tasks/PluginTaskManagerImpl.ts (96%) rename packages/{backend-common => backend-tasks}/src/tasks/PluginTaskManagerJanitor.ts (100%) rename packages/{backend-common => backend-tasks}/src/tasks/TaskManager.test.ts (94%) rename packages/{backend-common => backend-tasks}/src/tasks/TaskManager.ts (91%) rename packages/{backend-common => backend-tasks}/src/tasks/TaskWorker.test.ts (95%) rename packages/{backend-common => backend-tasks}/src/tasks/TaskWorker.ts (100%) rename packages/{backend-common => backend-tasks}/src/tasks/index.ts (100%) rename packages/{backend-common => backend-tasks}/src/tasks/types.ts (100%) rename packages/{backend-common => backend-tasks}/src/tasks/util.ts (100%) diff --git a/.changeset/happy-rice-tickle.md b/.changeset/happy-rice-tickle.md index 475f03501c..1e39dc2e9c 100644 --- a/.changeset/happy-rice-tickle.md +++ b/.changeset/happy-rice-tickle.md @@ -2,4 +2,4 @@ '@backstage/backend-common': patch --- -Add support for distributed mutexes and scheduled tasks, through the `TaskManager` class. This class can be particularly useful for coordinating things across many deployed instances of a given backend plugin. An example of this is catalog entity providers - with this facility you can register tasks similar to a cron job, and make sure that only one host at a time tries to execute the job, and that the timing (call frequency, timeouts etc) are retained as a global concern, letting you scale your workload safely without affecting the task behavior. +Added the `isDatabaseConflictError` function. diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 688c74159e..7242c32a84 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -12,7 +12,6 @@ import { BitbucketIntegration } from '@backstage/integration'; import { Config } from '@backstage/config'; import cors from 'cors'; import Docker from 'dockerode'; -import { Duration } from 'luxon'; import { ErrorRequestHandler } from 'express'; import express from 'express'; import { GithubCredentialsProvider } from '@backstage/integration'; @@ -384,11 +383,6 @@ export function loadBackendConfig(options: { argv: string[]; }): Promise; -// @public -export interface LockOptions { - timeout: Duration; -} - // @public export function notFoundHandler(): RequestHandler; @@ -408,29 +402,6 @@ export type PluginEndpointDiscovery = { getExternalBaseUrl(pluginId: string): Promise; }; -// @public -export interface PluginTaskManager { - acquireLock( - id: string, - options: LockOptions, - ): Promise< - | { - acquired: false; - } - | { - acquired: true; - release(): Promise; - } - >; - scheduleTask( - id: string, - options: TaskOptions, - fn: () => void | Promise, - ): Promise<{ - unschedule: () => Promise; - }>; -} - // @public export type ReaderFactory = (options: { config: Config; @@ -608,27 +579,6 @@ export interface StatusCheckHandlerOptions { statusCheck?: StatusCheck; } -// @public -export class TaskManager { - constructor(databaseManager: DatabaseManager, logger: Logger_2); - forPlugin(pluginId: string): PluginTaskManager; - // (undocumented) - static fromConfig( - config: Config, - options?: { - databaseManager?: DatabaseManager; - logger?: Logger_2; - }, - ): TaskManager; -} - -// @public -export interface TaskOptions { - frequency?: Duration; - initialDelay?: Duration; - timeout?: Duration; -} - // @public export type UrlReader = { read(url: string): Promise; diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 47345c198d..446b3b5afe 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -41,7 +41,6 @@ "@types/cors": "^2.8.6", "@types/dockerode": "^3.2.1", "@types/express": "^4.17.6", - "@types/luxon": "^2.0.4", "archiver": "^5.0.2", "aws-sdk": "^2.840.0", "compression": "^1.7.4", @@ -60,7 +59,6 @@ "knex": "^0.95.1", "lodash": "^4.17.21", "logform": "^2.1.1", - "luxon": "^2.0.2", "minimatch": "^3.0.4", "minimist": "^1.2.5", "morgan": "^1.10.0", @@ -69,10 +67,8 @@ "stoppable": "^1.1.0", "tar": "^6.1.2", "unzipper": "^0.10.11", - "uuid": "^8.0.0", "winston": "^3.2.1", - "yn": "^4.0.0", - "zod": "^3.9.5" + "yn": "^4.0.0" }, "peerDependencies": { "pg-connection-string": "^2.3.0" @@ -85,7 +81,6 @@ "devDependencies": { "@backstage/backend-test-utils": "^0.1.8", "@backstage/cli": "^0.8.2", - "@backstage/test-utils": "^0.1.21", "@types/archiver": "^5.1.0", "@types/compression": "^1.7.0", "@types/concat-stream": "^1.6.0", @@ -108,12 +103,10 @@ "msw": "^0.35.0", "mysql2": "^2.2.5", "recursive-readdir": "^2.2.2", - "supertest": "^6.1.3", - "wait-for-expect": "^3.0.2" + "supertest": "^6.1.3" }, "files": [ "dist", - "migrations/**/*.{js,d.ts}", "config.d.ts" ], "configSchema": "config.d.ts" diff --git a/packages/backend-common/src/index.ts b/packages/backend-common/src/index.ts index a906d7bc61..c214961a2e 100644 --- a/packages/backend-common/src/index.ts +++ b/packages/backend-common/src/index.ts @@ -31,5 +31,4 @@ export * from './paths'; export * from './reading'; export * from './scm'; export * from './service'; -export * from './tasks'; export * from './util'; diff --git a/packages/backend-tasks/.eslintrc.js b/packages/backend-tasks/.eslintrc.js new file mode 100644 index 0000000000..16a033dbc6 --- /dev/null +++ b/packages/backend-tasks/.eslintrc.js @@ -0,0 +1,3 @@ +module.exports = { + extends: [require.resolve('@backstage/cli/config/eslint.backend')], +}; diff --git a/packages/backend-tasks/README.md b/packages/backend-tasks/README.md new file mode 100644 index 0000000000..71c54b0a7f --- /dev/null +++ b/packages/backend-tasks/README.md @@ -0,0 +1,36 @@ +# @backstage/backend-tasks + +Common distributed task management / locking library for Backstage backends. + +## Usage + +Add the library to your backend package: + +```sh +# From your Backstage root directory +cd packages/backend +yarn add @backstage/backend-tasks +``` + +then make use of its facilities as necessary: + +```typescript +import { TaskManager } from '@backstage/backend-tasks'; + +const manager = TaskManager.fromConfig(rootConfig).forPlugin('my-plugin'); + +const { unschedule } = await manager.scheduleTask( + 'refresh-things', + { + frequency: Duration.fromObject({ minutes: 10 }), + }, + async () => { + await entityProvider.run(); + }, +); +``` + +## Documentation + +- [Backstage Readme](https://github.com/backstage/backstage/blob/master/README.md) +- [Backstage Documentation](https://github.com/backstage/backstage/blob/master/docs/README.md) diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md new file mode 100644 index 0000000000..4734e8b502 --- /dev/null +++ b/packages/backend-tasks/api-report.md @@ -0,0 +1,59 @@ +## API Report File for "@backstage/backend-tasks" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts +import { Config } from '@backstage/config'; +import { DatabaseManager } from '@backstage/backend-common'; +import { Duration } from 'luxon'; +import { Logger as Logger_2 } from 'winston'; + +// @public +export interface LockOptions { + timeout: Duration; +} + +// @public +export interface PluginTaskManager { + acquireLock( + id: string, + options: LockOptions, + ): Promise< + | { + acquired: false; + } + | { + acquired: true; + release(): Promise; + } + >; + scheduleTask( + id: string, + options: TaskOptions, + fn: () => void | Promise, + ): Promise<{ + unschedule: () => Promise; + }>; +} + +// @public +export class TaskManager { + constructor(databaseManager: DatabaseManager, logger: Logger_2); + forPlugin(pluginId: string): PluginTaskManager; + // (undocumented) + static fromConfig( + config: Config, + options?: { + databaseManager?: DatabaseManager; + logger?: Logger_2; + }, + ): TaskManager; +} + +// @public +export interface TaskOptions { + frequency?: Duration; + initialDelay?: Duration; + timeout?: Duration; +} +``` diff --git a/packages/backend-common/knexfile.js b/packages/backend-tasks/knexfile.js similarity index 100% rename from packages/backend-common/knexfile.js rename to packages/backend-tasks/knexfile.js diff --git a/packages/backend-common/migrations/20210928160613_init.js b/packages/backend-tasks/migrations/20210928160613_init.js similarity index 75% rename from packages/backend-common/migrations/20210928160613_init.js rename to packages/backend-tasks/migrations/20210928160613_init.js index 7a7590d903..77c8f2b945 100644 --- a/packages/backend-common/migrations/20210928160613_init.js +++ b/packages/backend-tasks/migrations/20210928160613_init.js @@ -23,7 +23,7 @@ exports.up = async function up(knex) { // // mutexes // - await knex.schema.createTable('backstage_backend_common__mutexes', table => { + await knex.schema.createTable('backstage_backend_tasks__mutexes', table => { table.comment('Locks used for mutual exclusion among multiple workers'); table .text('id') @@ -32,7 +32,7 @@ exports.up = async function up(knex) { .comment('The unique ID of this particular mutex'); table .text('current_lock_ticket') - .nullable() + .notNullable() .comment('A unique ticket for the current mutex lock'); table .dateTime('current_lock_acquired_at') @@ -42,12 +42,12 @@ exports.up = async function up(knex) { .dateTime('current_lock_expires_at') .nullable() .comment('The time when a locked mutex will time out and auto-release'); - table.index(['id'], 'backstage_backend_common__mutexes__id_idx'); + table.index(['id'], 'backstage_backend_tasks__mutexes__id_idx'); }); // // tasks // - await knex.schema.createTable('backstage_backend_common__tasks', table => { + await knex.schema.createTable('backstage_backend_tasks__tasks', table => { table.comment('Tasks used for scheduling work on multiple workers'); table .text('id') @@ -74,7 +74,7 @@ exports.up = async function up(knex) { .dateTime('current_run_expires_at') .nullable() .comment('The time that the current task run will time out'); - table.index(['id'], 'backstage_backend_common__tasks__id_idx'); + table.index(['id'], 'backstage_backend_tasks__tasks__id_idx'); }); }; @@ -85,18 +85,15 @@ exports.down = async function down(knex) { // // tasks // - await knex.schema.alterTable('backstage_backend_common__tasks', table => { - table.dropIndex([], 'backstage_backend_common__tasks__id_idx'); + await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { + table.dropIndex([], 'backstage_backend_tasks__tasks__id_idx'); }); - await knex.schema.dropTable('backstage_backend_common__tasks'); + await knex.schema.dropTable('backstage_backend_tasks__tasks'); // // locks // - await knex.schema.alterTable( - 'backstage_backend_common__task_locks', - table => { - table.dropIndex([], 'backstage_backend_common__task_locks__id_idx'); - }, - ); - await knex.schema.dropTable('backstage_backend_common__task_locks'); + await knex.schema.alterTable('backstage_backend_tasks__task_locks', table => { + table.dropIndex([], 'backstage_backend_tasks__task_locks__id_idx'); + }); + await knex.schema.dropTable('backstage_backend_tasks__task_locks'); }; diff --git a/packages/backend-tasks/package.json b/packages/backend-tasks/package.json new file mode 100644 index 0000000000..a5374d7b6e --- /dev/null +++ b/packages/backend-tasks/package.json @@ -0,0 +1,54 @@ +{ + "name": "@backstage/backend-tasks", + "description": "Common distributed task management / locking library for Backstage backends", + "version": "0.1.0", + "main": "src/index.ts", + "types": "src/index.ts", + "private": false, + "publishConfig": { + "access": "public", + "main": "dist/index.cjs.js", + "types": "dist/index.d.ts" + }, + "homepage": "https://backstage.io", + "repository": { + "type": "git", + "url": "https://github.com/backstage/backstage", + "directory": "packages/backend-tasks" + }, + "keywords": [ + "backstage" + ], + "license": "Apache-2.0", + "scripts": { + "build": "backstage-cli build --outputs cjs,types", + "lint": "backstage-cli lint", + "test": "backstage-cli test", + "prepack": "backstage-cli prepack", + "postpack": "backstage-cli postpack", + "clean": "backstage-cli clean" + }, + "dependencies": { + "@backstage/backend-common": "^0.9.7", + "@backstage/config": "^0.1.10", + "@backstage/errors": "^0.1.3", + "@backstage/types": "^0.1.1", + "@types/luxon": "^2.0.4", + "knex": "^0.95.1", + "lodash": "^4.17.21", + "luxon": "^2.0.2", + "uuid": "^8.0.0", + "winston": "^3.2.1", + "zod": "^3.9.5" + }, + "devDependencies": { + "@backstage/backend-test-utils": "^0.1.7", + "@backstage/cli": "^0.8.0", + "jest": "^26.0.1", + "wait-for-expect": "^3.0.2" + }, + "files": [ + "dist", + "migrations/**/*.{js,d.ts}" + ] +} diff --git a/packages/backend-common/src/database/migrateBackendCommon.ts b/packages/backend-tasks/src/database/migrateBackendTasks.ts similarity index 83% rename from packages/backend-common/src/database/migrateBackendCommon.ts rename to packages/backend-tasks/src/database/migrateBackendTasks.ts index 104e757e0b..cc0f4349ac 100644 --- a/packages/backend-common/src/database/migrateBackendCommon.ts +++ b/packages/backend-tasks/src/database/migrateBackendTasks.ts @@ -14,16 +14,16 @@ * limitations under the License. */ +import { resolvePackagePath } from '@backstage/backend-common'; import { Knex } from 'knex'; -import { resolvePackagePath } from '../paths'; import { DB_MIGRATIONS_TABLE } from './tables'; const migrationsDir = resolvePackagePath( - '@backstage/backend-common', + '@backstage/backend-tasks', 'migrations', ); -export async function migrateBackendCommon(knex: Knex): Promise { +export async function migrateBackendTasks(knex: Knex): Promise { await knex.migrate.latest({ directory: migrationsDir, tableName: DB_MIGRATIONS_TABLE, diff --git a/packages/backend-common/src/database/tables.ts b/packages/backend-tasks/src/database/tables.ts similarity index 79% rename from packages/backend-common/src/database/tables.ts rename to packages/backend-tasks/src/database/tables.ts index 90e8aeca40..b93dcf4faa 100644 --- a/packages/backend-common/src/database/tables.ts +++ b/packages/backend-tasks/src/database/tables.ts @@ -14,13 +14,13 @@ * limitations under the License. */ -export const DB_MIGRATIONS_TABLE = 'backstage_backend_common__knex_migrations'; -export const DB_MUTEXES_TABLE = 'backstage_backend_common__mutexes'; -export const DB_TASKS_TABLE = 'backstage_backend_common__tasks'; +export const DB_MIGRATIONS_TABLE = 'backstage_backend_tasks__knex_migrations'; +export const DB_MUTEXES_TABLE = 'backstage_backend_tasks__mutexes'; +export const DB_TASKS_TABLE = 'backstage_backend_tasks__tasks'; export type DbMutexesRow = { id: string; - current_lock_ticket?: string; + current_lock_ticket: string; current_lock_acquired_at?: Date | string; current_lock_expires_at?: Date | string; }; diff --git a/packages/backend-tasks/src/index.ts b/packages/backend-tasks/src/index.ts new file mode 100644 index 0000000000..00a8aa2803 --- /dev/null +++ b/packages/backend-tasks/src/index.ts @@ -0,0 +1,23 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Common distributed task management / locking library for Backstage backends + * + * @packageDocumentation + */ + +export * from './tasks'; diff --git a/packages/backend-tasks/src/setupTests.ts b/packages/backend-tasks/src/setupTests.ts new file mode 100644 index 0000000000..d3232290a7 --- /dev/null +++ b/packages/backend-tasks/src/setupTests.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export {}; diff --git a/packages/backend-common/src/tasks/CancelToken.ts b/packages/backend-tasks/src/tasks/CancelToken.ts similarity index 100% rename from packages/backend-common/src/tasks/CancelToken.ts rename to packages/backend-tasks/src/tasks/CancelToken.ts diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts similarity index 96% rename from packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts rename to packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts index ef5f6db96f..f27a00cac6 100644 --- a/packages/backend-common/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts @@ -14,11 +14,11 @@ * limitations under the License. */ +import { getVoidLogger } from '@backstage/backend-common'; import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; import waitForExpect from 'wait-for-expect'; -import { migrateBackendCommon } from '../database/migrateBackendCommon'; -import { getVoidLogger } from '../logging'; +import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; describe('PluginTaskManagerImpl', () => { @@ -28,7 +28,7 @@ describe('PluginTaskManagerImpl', () => { async function init(databaseId: TestDatabaseId) { const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const manager = new PluginTaskManagerImpl( async () => knex, getVoidLogger(), diff --git a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts similarity index 96% rename from packages/backend-common/src/tasks/PluginTaskManagerImpl.ts rename to packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts index 540c7d501a..c30c915353 100644 --- a/packages/backend-common/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts @@ -14,10 +14,10 @@ * limitations under the License. */ +import { isDatabaseConflictError } from '@backstage/backend-common'; import { Knex } from 'knex'; import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; -import { isDatabaseConflictError } from '../database'; import { DbMutexesRow, DB_MUTEXES_TABLE } from '../database/tables'; import { TaskWorker } from './TaskWorker'; import { LockOptions, PluginTaskManager, TaskOptions } from './types'; @@ -65,7 +65,6 @@ export class PluginTaskManagerImpl implements PluginTaskManager { // First try to overwrite an existing lock, that has timed out const stolen = await knex(DB_MUTEXES_TABLE) .where('id', '=', id) - .whereNotNull('current_lock_ticket') .where('current_lock_expires_at', '<', knex.fn.now()) .update(record); diff --git a/packages/backend-common/src/tasks/PluginTaskManagerJanitor.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts similarity index 100% rename from packages/backend-common/src/tasks/PluginTaskManagerJanitor.ts rename to packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts diff --git a/packages/backend-common/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskManager.test.ts similarity index 94% rename from packages/backend-common/src/tasks/TaskManager.test.ts rename to packages/backend-tasks/src/tasks/TaskManager.test.ts index e6a34e58c1..87efb5cfed 100644 --- a/packages/backend-common/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.test.ts @@ -14,10 +14,9 @@ * limitations under the License. */ +import { DatabaseManager, getVoidLogger } from '@backstage/backend-common'; import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; -import { DatabaseManager } from '../database'; -import { getVoidLogger } from '../logging'; import { TaskManager } from './TaskManager'; describe('TaskManager', () => { diff --git a/packages/backend-common/src/tasks/TaskManager.ts b/packages/backend-tasks/src/tasks/TaskManager.ts similarity index 91% rename from packages/backend-common/src/tasks/TaskManager.ts rename to packages/backend-tasks/src/tasks/TaskManager.ts index db8228561f..d065447fb2 100644 --- a/packages/backend-common/src/tasks/TaskManager.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.ts @@ -14,13 +14,12 @@ * limitations under the License. */ +import { DatabaseManager, getRootLogger } from '@backstage/backend-common'; import { Config } from '@backstage/config'; import { memoize } from 'lodash'; import { Duration } from 'luxon'; import { Logger } from 'winston'; -import { DatabaseManager } from '../database'; -import { migrateBackendCommon } from '../database/migrateBackendCommon'; -import { getRootLogger } from '../logging'; +import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; import { PluginTaskManagerJanitor } from './PluginTaskManagerJanitor'; import { PluginTaskManager } from './types'; @@ -61,7 +60,7 @@ export class TaskManager { const databaseFactory = memoize(async () => { const knex = await this.databaseManager.forPlugin(pluginId).getClient(); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const janitor = new PluginTaskManagerJanitor({ knex, diff --git a/packages/backend-common/src/tasks/TaskWorker.test.ts b/packages/backend-tasks/src/tasks/TaskWorker.test.ts similarity index 95% rename from packages/backend-common/src/tasks/TaskWorker.test.ts rename to packages/backend-tasks/src/tasks/TaskWorker.test.ts index ca0d624560..2a7de2220a 100644 --- a/packages/backend-common/src/tasks/TaskWorker.test.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.test.ts @@ -14,12 +14,12 @@ * limitations under the License. */ +import { getVoidLogger } from '@backstage/backend-common'; import { TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; import waitForExpect from 'wait-for-expect'; -import { migrateBackendCommon } from '../database/migrateBackendCommon'; +import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; -import { getVoidLogger } from '../logging'; import { TaskWorker } from './TaskWorker'; import { TaskSettingsV1 } from './types'; @@ -37,7 +37,7 @@ describe('TaskWorker', () => { 'can run a single task to completion, %p', async databaseId => { const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const fn = jest.fn( async () => new Promise(resolve => setTimeout(resolve, 50)), @@ -60,7 +60,7 @@ describe('TaskWorker', () => { 'goes through the expected states for a single run, %p', async databaseId => { const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const fn = jest.fn( async () => new Promise(resolve => setTimeout(resolve, 50)), @@ -135,7 +135,7 @@ describe('TaskWorker', () => { 'runs tasks more than once even when the task throws, %p', async databaseId => { const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const fn = jest.fn().mockRejectedValue(new Error('failed')); const settings: TaskSettingsV1 = { @@ -159,7 +159,7 @@ describe('TaskWorker', () => { 'does not clobber ticket lock when stolen, %p', async databaseId => { const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const fn = jest.fn( async () => new Promise(resolve => setTimeout(resolve, 50)), @@ -212,7 +212,7 @@ describe('TaskWorker', () => { 'gracefully handles a disappeared task row, %p', async databaseId => { const knex = await databases.init(databaseId); - await migrateBackendCommon(knex); + await migrateBackendTasks(knex); const fn = jest.fn(async () => {}); const settings: TaskSettingsV1 = { diff --git a/packages/backend-common/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts similarity index 100% rename from packages/backend-common/src/tasks/TaskWorker.ts rename to packages/backend-tasks/src/tasks/TaskWorker.ts diff --git a/packages/backend-common/src/tasks/index.ts b/packages/backend-tasks/src/tasks/index.ts similarity index 100% rename from packages/backend-common/src/tasks/index.ts rename to packages/backend-tasks/src/tasks/index.ts diff --git a/packages/backend-common/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts similarity index 100% rename from packages/backend-common/src/tasks/types.ts rename to packages/backend-tasks/src/tasks/types.ts diff --git a/packages/backend-common/src/tasks/util.ts b/packages/backend-tasks/src/tasks/util.ts similarity index 100% rename from packages/backend-common/src/tasks/util.ts rename to packages/backend-tasks/src/tasks/util.ts From c1ac8dd174c25fb15245420813506f72a29d2c05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 24 Oct 2021 19:22:41 +0200 Subject: [PATCH 07/27] log on failed release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts index c30c915353..f3d453e89b 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts @@ -43,16 +43,16 @@ export class PluginTaskManagerImpl implements PluginTaskManager { const knex = await this.databaseFactory(); const ticket = uuid(); - async function release() { + const release = async () => { try { await knex(DB_MUTEXES_TABLE) .where('id', '=', id) .where('current_lock_ticket', '=', ticket) .delete(); - } catch { - // fail silently + } catch (e) { + this.logger.warn(`Failed to release lock, ${e}`); } - } + }; const record: Knex.DbRecord = { current_lock_ticket: ticket, From 67456f7fc9994894b1ae231fa09d4d438d307bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 16:46:10 +0200 Subject: [PATCH 08/27] remove knexfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/knexfile.js | 26 -------------------------- 1 file changed, 26 deletions(-) delete mode 100644 packages/backend-tasks/knexfile.js diff --git a/packages/backend-tasks/knexfile.js b/packages/backend-tasks/knexfile.js deleted file mode 100644 index 4c8be42673..0000000000 --- a/packages/backend-tasks/knexfile.js +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// This file makes it possible to run "yarn knex migrate:make some_file_name" -// to assist in making new migrations -module.exports = { - client: 'sqlite3', - connection: ':memory:', - useNullAsDefault: true, - migrations: { - directory: './migrations', - }, -}; From 3f1237147fbf5cfb33f931403ac657de8eeb37f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 16:54:36 +0200 Subject: [PATCH 09/27] remove the lock functionality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/api-report.md | 17 ---- .../migrations/20210928160613_init.js | 31 ------- packages/backend-tasks/src/database/tables.ts | 8 -- .../src/tasks/PluginTaskManagerImpl.test.ts | 81 ------------------- .../src/tasks/PluginTaskManagerImpl.ts | 58 +------------ .../src/tasks/PluginTaskManagerJanitor.ts | 23 +----- .../src/tasks/TaskManager.test.ts | 12 ++- packages/backend-tasks/src/tasks/index.ts | 2 +- packages/backend-tasks/src/tasks/types.ts | 33 -------- 9 files changed, 12 insertions(+), 253 deletions(-) diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 4734e8b502..e9d7884b43 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -8,25 +8,8 @@ import { DatabaseManager } from '@backstage/backend-common'; import { Duration } from 'luxon'; import { Logger as Logger_2 } from 'winston'; -// @public -export interface LockOptions { - timeout: Duration; -} - // @public export interface PluginTaskManager { - acquireLock( - id: string, - options: LockOptions, - ): Promise< - | { - acquired: false; - } - | { - acquired: true; - release(): Promise; - } - >; scheduleTask( id: string, options: TaskOptions, diff --git a/packages/backend-tasks/migrations/20210928160613_init.js b/packages/backend-tasks/migrations/20210928160613_init.js index 77c8f2b945..c5028e2e93 100644 --- a/packages/backend-tasks/migrations/20210928160613_init.js +++ b/packages/backend-tasks/migrations/20210928160613_init.js @@ -20,30 +20,6 @@ * @param {import('knex').Knex} knex */ exports.up = async function up(knex) { - // - // mutexes - // - await knex.schema.createTable('backstage_backend_tasks__mutexes', table => { - table.comment('Locks used for mutual exclusion among multiple workers'); - table - .text('id') - .primary() - .notNullable() - .comment('The unique ID of this particular mutex'); - table - .text('current_lock_ticket') - .notNullable() - .comment('A unique ticket for the current mutex lock'); - table - .dateTime('current_lock_acquired_at') - .nullable() - .comment('The time when the mutex was locked'); - table - .dateTime('current_lock_expires_at') - .nullable() - .comment('The time when a locked mutex will time out and auto-release'); - table.index(['id'], 'backstage_backend_tasks__mutexes__id_idx'); - }); // // tasks // @@ -89,11 +65,4 @@ exports.down = async function down(knex) { table.dropIndex([], 'backstage_backend_tasks__tasks__id_idx'); }); await knex.schema.dropTable('backstage_backend_tasks__tasks'); - // - // locks - // - await knex.schema.alterTable('backstage_backend_tasks__task_locks', table => { - table.dropIndex([], 'backstage_backend_tasks__task_locks__id_idx'); - }); - await knex.schema.dropTable('backstage_backend_tasks__task_locks'); }; diff --git a/packages/backend-tasks/src/database/tables.ts b/packages/backend-tasks/src/database/tables.ts index b93dcf4faa..4777c4e60c 100644 --- a/packages/backend-tasks/src/database/tables.ts +++ b/packages/backend-tasks/src/database/tables.ts @@ -15,16 +15,8 @@ */ export const DB_MIGRATIONS_TABLE = 'backstage_backend_tasks__knex_migrations'; -export const DB_MUTEXES_TABLE = 'backstage_backend_tasks__mutexes'; export const DB_TASKS_TABLE = 'backstage_backend_tasks__tasks'; -export type DbMutexesRow = { - id: string; - current_lock_ticket: string; - current_lock_acquired_at?: Date | string; - current_lock_expires_at?: Date | string; -}; - export type DbTasksRow = { id: string; settings_json: string; diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts index f27a00cac6..1d1c89cb6b 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts @@ -16,7 +16,6 @@ import { getVoidLogger } from '@backstage/backend-common'; import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; -import { Duration } from 'luxon'; import waitForExpect from 'wait-for-expect'; import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; @@ -36,86 +35,6 @@ describe('PluginTaskManagerImpl', () => { return { knex, manager }; } - describe('acquireLock', () => { - it.each(databases.eachSupportedId())( - 'can run the happy path, %p', - async databaseId => { - const { manager } = await init(databaseId); - - const lock1 = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - const lock2 = await manager.acquireLock('lock2', { - timeout: Duration.fromMillis(5000), - }); - - expect(lock1.acquired).toBe(true); - expect(lock2.acquired).toBe(true); - - await expect( - manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }), - ).resolves.toEqual({ acquired: false }); - - await (lock1 as any).release(); - await (lock2 as any).release(); - - const lock1Again = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - expect(lock1Again.acquired).toBe(true); - await (lock1Again as any).release(); - }, - ); - - it.each(databases.eachSupportedId())( - 'rejects double lock attempts, %p', - async databaseId => { - const { manager } = await init(databaseId); - - const lock1 = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - const lock2 = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - - expect(lock1.acquired).toBe(true); - expect(lock2.acquired).toBe(false); - - await (lock1 as any).release(); - - const lock1Again = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - expect(lock1Again.acquired).toBe(true); - await (lock1Again as any).release(); - }, - ); - - it.each(databases.eachSupportedId())( - 'times out locks, %p', - async databaseId => { - const { manager } = await init(databaseId); - - const lock1 = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(200), - }); - - expect(lock1.acquired).toBe(true); - - await new Promise(resolve => setTimeout(resolve, 1000)); - - const lock2 = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - expect(lock2.acquired).toBe(true); - await (lock2 as any).release(); - }, - ); - }); - // This is just to test the wrapper code; most of the actual tests are in // TaskWorker.test.ts describe('scheduleTask', () => { diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts index f3d453e89b..7859f4ee23 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts @@ -14,14 +14,11 @@ * limitations under the License. */ -import { isDatabaseConflictError } from '@backstage/backend-common'; import { Knex } from 'knex'; -import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; -import { DbMutexesRow, DB_MUTEXES_TABLE } from '../database/tables'; import { TaskWorker } from './TaskWorker'; -import { LockOptions, PluginTaskManager, TaskOptions } from './types'; -import { nowPlus, validateId } from './util'; +import { PluginTaskManager, TaskOptions } from './types'; +import { validateId } from './util'; /** * Implements the actual task management. @@ -32,57 +29,6 @@ export class PluginTaskManagerImpl implements PluginTaskManager { private readonly logger: Logger, ) {} - async acquireLock( - id: string, - options: LockOptions, - ): Promise< - { acquired: false } | { acquired: true; release(): Promise } - > { - validateId(id); - - const knex = await this.databaseFactory(); - const ticket = uuid(); - - const release = async () => { - try { - await knex(DB_MUTEXES_TABLE) - .where('id', '=', id) - .where('current_lock_ticket', '=', ticket) - .delete(); - } catch (e) { - this.logger.warn(`Failed to release lock, ${e}`); - } - }; - - const record: Knex.DbRecord = { - current_lock_ticket: ticket, - current_lock_acquired_at: knex.fn.now(), - current_lock_expires_at: options.timeout - ? nowPlus(options.timeout, knex) - : knex.raw('null'), - }; - - // First try to overwrite an existing lock, that has timed out - const stolen = await knex(DB_MUTEXES_TABLE) - .where('id', '=', id) - .where('current_lock_expires_at', '<', knex.fn.now()) - .update(record); - - if (stolen) { - return { acquired: true, release }; - } - - try { - await knex(DB_MUTEXES_TABLE).insert({ id, ...record }); - return { acquired: true, release }; - } catch (e) { - if (!isDatabaseConflictError(e)) { - this.logger.warn(`Failed to acquire lock, ${e}`); - } - return { acquired: false }; - } - } - async scheduleTask( id: string, options: TaskOptions, diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts index 26af986025..2912041b3f 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts @@ -17,12 +17,7 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; import { Logger } from 'winston'; -import { - DbMutexesRow, - DbTasksRow, - DB_MUTEXES_TABLE, - DB_TASKS_TABLE, -} from '../database/tables'; +import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; import { CancelToken } from './CancelToken'; /** @@ -70,22 +65,6 @@ export class PluginTaskManagerJanitor { // https://github.com/knex/knex/issues/4370 // https://github.com/mapbox/node-sqlite3/issues/1453 - const mutexesQuery = this.knex(DB_MUTEXES_TABLE) - .where('current_lock_expires_at', '<', this.knex.fn.now()) - .delete(); - - if (this.knex.client.config.client === 'sqlite3') { - const mutexes = await mutexesQuery; - if (mutexes > 0) { - this.logger.warn(`${mutexes} mutex locks timed out and were lost`); - } - } else { - const mutexes = await mutexesQuery.returning(['id']); - for (const { id } of mutexes) { - this.logger.warn(`Mutex lock timed out and was lost: ${id}`); - } - } - const tasksQuery = this.knex(DB_TASKS_TABLE) .where('current_run_expires_at', '<', this.knex.fn.now()) .delete(); diff --git a/packages/backend-tasks/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskManager.test.ts index 87efb5cfed..cf6e808ee3 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.test.ts @@ -43,10 +43,14 @@ describe('TaskManager', () => { const database = await createDatabase(databaseId); const manager = new TaskManager(database, logger).forPlugin('test'); - const lock = await manager.acquireLock('lock1', { - timeout: Duration.fromMillis(5000), - }); - expect(lock.acquired).toBe(true); + const task = await manager.scheduleTask( + 'task1', + { + timeout: Duration.fromMillis(5000), + }, + () => {}, + ); + expect(task.unschedule).toBeDefined(); }, ); }); diff --git a/packages/backend-tasks/src/tasks/index.ts b/packages/backend-tasks/src/tasks/index.ts index 9f86f8fc64..d16a30b6c9 100644 --- a/packages/backend-tasks/src/tasks/index.ts +++ b/packages/backend-tasks/src/tasks/index.ts @@ -15,4 +15,4 @@ */ export { TaskManager } from './TaskManager'; -export type { LockOptions, PluginTaskManager, TaskOptions } from './types'; +export type { PluginTaskManager, TaskOptions } from './types'; diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index a8d356bed9..eed652093d 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -17,19 +17,6 @@ import { Duration } from 'luxon'; import { z } from 'zod'; -/** - * Options that apply to the acquiral of a given lock. - * - * @public - */ -export interface LockOptions { - /** - * The maximum amount of time that the lock can be held, before it's - * considered timed out and gets auto-released by the framework. - */ - timeout: Duration; -} - /** * Options that apply to the invocation of a given task. * @@ -82,26 +69,6 @@ export interface TaskOptions { * @public */ export interface PluginTaskManager { - /** - * Attempts to acquire an exclusive lock. - * - * A lock can only be held by one party at a time. Any subsequent attempts to - * acquire the lock will fail, unless the timeout period has been exceeded or - * the lock was released by the previous holder. - * - * @param id - A unique ID (within the scope of the plugin) for a lock - * @param options - Options for the lock - * @returns The result of the lock attempt. If it was successfully acquired, - * you should remember to call its `release` method as soon as you - * are done with the lock. - */ - acquireLock( - id: string, - options: LockOptions, - ): Promise< - { acquired: false } | { acquired: true; release(): Promise } - >; - /** * Schedules a task function for coordinated exclusive invocation across * workers. From 5d602f8e292e8717dafecfccefc2ea4377f72c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 16:56:26 +0200 Subject: [PATCH 10/27] get rid of the ts-ignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/src/tasks/CancelToken.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/backend-tasks/src/tasks/CancelToken.ts b/packages/backend-tasks/src/tasks/CancelToken.ts index 9ea9802e82..190a5d6562 100644 --- a/packages/backend-tasks/src/tasks/CancelToken.ts +++ b/packages/backend-tasks/src/tasks/CancelToken.ts @@ -15,7 +15,6 @@ */ export class CancelToken { - // @ts-ignore: is actually assigned by the Promise constructor #cancel: () => void; #isCancelled: boolean; #cancelPromise: Promise; @@ -26,6 +25,8 @@ export class CancelToken { private constructor() { this.#isCancelled = false; + + this.#cancel = () => {}; // Avoids a TS warning this.#cancelPromise = new Promise(resolve => { this.#cancel = () => { this.#isCancelled = true; From d252c983b575e7e8808b16ac2b39cfa9389dd2b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 16:59:20 +0200 Subject: [PATCH 11/27] use dashes in not-ready-yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/src/tasks/TaskWorker.test.ts | 2 +- packages/backend-tasks/src/tasks/TaskWorker.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/backend-tasks/src/tasks/TaskWorker.test.ts b/packages/backend-tasks/src/tasks/TaskWorker.test.ts index 2a7de2220a..5e0152f776 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.test.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.test.ts @@ -90,7 +90,7 @@ describe('TaskWorker', () => { }); await expect(worker.findReadyTask()).resolves.toEqual({ - result: 'not ready yet', + result: 'not-ready-yet', }); waitForExpect(async () => { diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index 3fd1f30716..2289975230 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -91,14 +91,14 @@ export class TaskWorker { * @returns The outcome of the attempt */ async runOnce(): Promise< - | { result: 'not ready yet' } + | { result: 'not-ready-yet' } | { result: 'abort' } | { result: 'failed' } | { result: 'completed' } > { const findResult = await this.findReadyTask(); if ( - findResult.result === 'not ready yet' || + findResult.result === 'not-ready-yet' || findResult.result === 'abort' ) { return findResult; @@ -109,7 +109,7 @@ export class TaskWorker { const claimed = await this.tryClaimTask(ticket, taskSettings); if (!claimed) { - return { result: 'not ready yet' }; + return { result: 'not-ready-yet' }; } try { @@ -165,7 +165,7 @@ export class TaskWorker { * Check if the task is ready to run */ async findReadyTask(): Promise< - | { result: 'not ready yet' } + | { result: 'not-ready-yet' } | { result: 'abort' } | { result: 'ready'; settings: TaskSettingsV1 } > { @@ -189,7 +189,7 @@ export class TaskWorker { ); return { result: 'abort' }; } else if (!row.ready) { - return { result: 'not ready yet' }; + return { result: 'not-ready-yet' }; } try { From aeaa2fe3e6fe7b29334c54d8a4e27c708b18a844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 17:01:24 +0200 Subject: [PATCH 12/27] log the task id as well when unable to parse settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/src/tasks/TaskWorker.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index 2289975230..dc48f843aa 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -197,9 +197,8 @@ export class TaskWorker { return { result: 'ready', settings }; } catch (e) { this.logger.info( - 'No longer able to parse task settings; aborting and assuming that a ' + - 'newer version of the task has been issued and being handled by ' + - `other workers, ${e}`, + `Task "${this.taskId}" is no longer able to parse task settings; aborting and assuming that a ` + + `newer version of the task has been issued and being handled by other workers, ${e}`, ); return { result: 'abort' }; } From cc3b6c25aa60e1ea6bee1cc20a456e581d944ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 19:08:59 +0200 Subject: [PATCH 13/27] make timeout and frequency mandatory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/tasks/PluginTaskManagerImpl.test.ts | 10 +++- .../src/tasks/PluginTaskManagerImpl.ts | 4 +- .../src/tasks/TaskManager.test.ts | 1 + .../src/tasks/TaskWorker.test.ts | 52 +++++++------------ .../backend-tasks/src/tasks/TaskWorker.ts | 14 ----- packages/backend-tasks/src/tasks/types.ts | 6 +-- 6 files changed, 34 insertions(+), 53 deletions(-) diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts index 1d1c89cb6b..193dd2a95d 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts @@ -16,6 +16,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; +import { Duration } from 'luxon'; import waitForExpect from 'wait-for-expect'; import { migrateBackendTasks } from '../database/migrateBackendTasks'; import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; @@ -44,7 +45,14 @@ describe('PluginTaskManagerImpl', () => { const { manager } = await init(databaseId); const fn = jest.fn(); - const { unschedule } = await manager.scheduleTask('task1', {}, fn); + const { unschedule } = await manager.scheduleTask( + 'task1', + { + timeout: Duration.fromMillis(5000), + frequency: Duration.fromMillis(5000), + }, + fn, + ); await waitForExpect(() => { expect(fn).toBeCalled(); diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts index 7859f4ee23..1b890a69c8 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts @@ -42,8 +42,8 @@ export class PluginTaskManagerImpl implements PluginTaskManager { await task.start({ version: 1, initialDelayDuration: options.initialDelay?.toISO(), - recurringAtMostEveryDuration: options.frequency?.toISO(), - timeoutAfterDuration: options.timeout?.toISO(), + recurringAtMostEveryDuration: options.frequency.toISO(), + timeoutAfterDuration: options.timeout.toISO(), }); return { diff --git a/packages/backend-tasks/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskManager.test.ts index cf6e808ee3..002db0b3de 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.test.ts @@ -47,6 +47,7 @@ describe('TaskManager', () => { 'task1', { timeout: Duration.fromMillis(5000), + frequency: Duration.fromMillis(5000), }, () => {}, ); diff --git a/packages/backend-tasks/src/tasks/TaskWorker.test.ts b/packages/backend-tasks/src/tasks/TaskWorker.test.ts index 5e0152f776..2f977ca639 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.test.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.test.ts @@ -34,7 +34,7 @@ describe('TaskWorker', () => { }); it.each(databases.eachSupportedId())( - 'can run a single task to completion, %p', + 'goes through the expected states, %p', async databaseId => { const knex = await databases.init(databaseId); await migrateBackendTasks(knex); @@ -44,32 +44,9 @@ describe('TaskWorker', () => { ); const settings: TaskSettingsV1 = { version: 1, - }; - - const worker = new TaskWorker('task1', fn, knex, logger); - await worker.start(settings); - - waitForExpect(() => { - expect(fn).toBeCalledTimes(1); - }); - }, - 60_000, - ); - - it.each(databases.eachSupportedId())( - 'goes through the expected states for a single run, %p', - async databaseId => { - const knex = await databases.init(databaseId); - await migrateBackendTasks(knex); - - const fn = jest.fn( - async () => new Promise(resolve => setTimeout(resolve, 50)), - ); - const settings: TaskSettingsV1 = { - version: 1, - initialDelayDuration: Duration.fromObject({ seconds: 1 }).toISO(), - recurringAtMostEveryDuration: undefined, - timeoutAfterDuration: undefined, + initialDelayDuration: Duration.fromMillis(1000).toISO(), + recurringAtMostEveryDuration: Duration.fromMillis(2000).toISO(), + timeoutAfterDuration: Duration.fromMillis(60000).toISO(), }; const worker = new TaskWorker('task1', fn, knex, logger); @@ -87,6 +64,8 @@ describe('TaskWorker', () => { expect(JSON.parse(row.settings_json)).toEqual({ version: 1, initialDelayDuration: 'PT1S', + recurringAtMostEveryDuration: 'PT2S', + timeoutAfterDuration: 'PT60S', }); await expect(worker.findReadyTask()).resolves.toEqual({ @@ -117,7 +96,7 @@ describe('TaskWorker', () => { id: 'task1', current_run_ticket: 'ticket', current_run_started_at: expect.anything(), - current_run_expires_at: null, + current_run_expires_at: expect.anything(), }), ); @@ -126,7 +105,14 @@ describe('TaskWorker', () => { ); row = (await knex(DB_TASKS_TABLE))[0]; - expect(row).toBeUndefined(); + expect(row).toEqual( + expect.objectContaining({ + id: 'task1', + current_run_ticket: null, + current_run_started_at: null, + current_run_expires_at: null, + }), + ); }, 60_000, ); @@ -142,7 +128,7 @@ describe('TaskWorker', () => { version: 1, initialDelayDuration: undefined, recurringAtMostEveryDuration: Duration.fromMillis(0).toISO(), - timeoutAfterDuration: undefined, + timeoutAfterDuration: Duration.fromMillis(60000).toISO(), }; const worker = new TaskWorker('task1', fn, knex, logger); @@ -167,6 +153,7 @@ describe('TaskWorker', () => { const settings: TaskSettingsV1 = { version: 1, recurringAtMostEveryDuration: Duration.fromMillis(0).toISO(), + timeoutAfterDuration: Duration.fromMillis(60000).toISO(), }; const worker = new TaskWorker('task1', fn, knex, logger); @@ -183,7 +170,7 @@ describe('TaskWorker', () => { id: 'task1', current_run_ticket: 'ticket', current_run_started_at: expect.anything(), - current_run_expires_at: null, + current_run_expires_at: expect.anything(), }), ); @@ -201,7 +188,7 @@ describe('TaskWorker', () => { id: 'task1', current_run_ticket: 'stolen', current_run_started_at: expect.anything(), - current_run_expires_at: null, + current_run_expires_at: expect.anything(), }), ); }, @@ -218,6 +205,7 @@ describe('TaskWorker', () => { const settings: TaskSettingsV1 = { version: 1, recurringAtMostEveryDuration: Duration.fromMillis(0).toISO(), + timeoutAfterDuration: Duration.fromMillis(60000).toISO(), }; const worker1 = new TaskWorker('task1', fn, knex, logger); diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index dc48f843aa..881a013360 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -68,9 +68,6 @@ export class TaskWorker { if (runResult.result === 'abort') { break; } - if (!settings.recurringAtMostEveryDuration) { - break; - } await this.sleep(WORK_CHECK_FREQUENCY); } @@ -240,17 +237,6 @@ export class TaskWorker { ): Promise { const { recurringAtMostEveryDuration } = settings; - // If this is not a recurring task, and we still have the current run - // ticket, delete it from the table - if (recurringAtMostEveryDuration === undefined) { - const rows = await this.knex(DB_TASKS_TABLE) - .where('id', '=', this.taskId) - .where('current_run_ticket', '=', ticket) - .delete(); - - return rows === 1; - } - // We make an effort to keep the datetime calculations in the database // layer, making sure to not have to perform conversions back and forth and // leaning on the database as a central clock source diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index eed652093d..989414ec12 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -31,7 +31,7 @@ export interface TaskOptions { * If no value is given for this field then there is no timeout. This is * potentially dangerous. */ - timeout?: Duration; + timeout: Duration; /** * The amount of time that should pass between task invocation starts. @@ -48,7 +48,7 @@ export interface TaskOptions { * If no value is given for this field then the task will only be invoked * once (on any worker) and then unscheduled automatically. */ - frequency?: Duration; + frequency: Duration; /** * The amount of time that should pass before the first invocation happens. @@ -107,11 +107,9 @@ export const taskSettingsV1Schema = z.object({ .refine(isValidOptionalDurationString, { message: 'Invalid duration' }), recurringAtMostEveryDuration: z .string() - .optional() .refine(isValidOptionalDurationString, { message: 'Invalid duration' }), timeoutAfterDuration: z .string() - .optional() .refine(isValidOptionalDurationString, { message: 'Invalid duration' }), }); From b91fe07309954fa6a18fa1934e3a5e9d9dffd233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 21:00:07 +0200 Subject: [PATCH 14/27] use a proper AbortController instead of the home baked one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/package.json | 1 + .../backend-tasks/src/tasks/CancelToken.ts | 49 ------------------- .../src/tasks/PluginTaskManagerJanitor.ts | 26 ++++------ .../backend-tasks/src/tasks/TaskWorker.ts | 29 ++++------- packages/backend-tasks/src/tasks/util.ts | 32 ++++++++++++ 5 files changed, 51 insertions(+), 86 deletions(-) delete mode 100644 packages/backend-tasks/src/tasks/CancelToken.ts diff --git a/packages/backend-tasks/package.json b/packages/backend-tasks/package.json index a5374d7b6e..dcb5922cf1 100644 --- a/packages/backend-tasks/package.json +++ b/packages/backend-tasks/package.json @@ -37,6 +37,7 @@ "knex": "^0.95.1", "lodash": "^4.17.21", "luxon": "^2.0.2", + "node-abort-controller": "^3.0.1", "uuid": "^8.0.0", "winston": "^3.2.1", "zod": "^3.9.5" diff --git a/packages/backend-tasks/src/tasks/CancelToken.ts b/packages/backend-tasks/src/tasks/CancelToken.ts deleted file mode 100644 index 190a5d6562..0000000000 --- a/packages/backend-tasks/src/tasks/CancelToken.ts +++ /dev/null @@ -1,49 +0,0 @@ -/* - * Copyright 2021 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -export class CancelToken { - #cancel: () => void; - #isCancelled: boolean; - #cancelPromise: Promise; - - static create(): CancelToken { - return new CancelToken(); - } - - private constructor() { - this.#isCancelled = false; - - this.#cancel = () => {}; // Avoids a TS warning - this.#cancelPromise = new Promise(resolve => { - this.#cancel = () => { - this.#isCancelled = true; - resolve(); - }; - }); - } - - cancel(): void { - this.#cancel(); - } - - get isCancelled(): boolean { - return this.#isCancelled; - } - - get promise(): Promise { - return this.#cancelPromise; - } -} diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts index 2912041b3f..ec65eadf1e 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts @@ -16,9 +16,10 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; +import { AbortController, AbortSignal } from 'node-abort-controller'; import { Logger } from 'winston'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; -import { CancelToken } from './CancelToken'; +import { sleep } from './util'; /** * Makes sure to auto-expire and clean up things that time out or for other @@ -28,7 +29,8 @@ export class PluginTaskManagerJanitor { private readonly knex: Knex; private readonly waitBetweenRuns: Duration; private readonly logger: Logger; - private readonly cancelToken: CancelToken; + private readonly abortController: AbortController; + private readonly abortSignal: AbortSignal; constructor(options: { knex: Knex; @@ -38,23 +40,24 @@ export class PluginTaskManagerJanitor { this.knex = options.knex; this.waitBetweenRuns = options.waitBetweenRuns; this.logger = options.logger; - this.cancelToken = CancelToken.create(); + this.abortController = new AbortController(); + this.abortSignal = this.abortController.signal; } async start() { - while (!this.cancelToken.isCancelled) { + while (!this.abortSignal.aborted) { try { await this.runOnce(); } catch (e) { this.logger.warn(`Error while performing janitorial tasks, ${e}`); } - await this.sleep(this.waitBetweenRuns); + await sleep(this.waitBetweenRuns, this.abortSignal); } } async stop() { - this.cancelToken.cancel(); + this.abortController.abort(); } private async runOnce() { @@ -79,15 +82,4 @@ export class PluginTaskManagerJanitor { } } } - - /** - * Sleeps for the given duration, but aborts sooner if the cancel token - * triggers. - */ - private async sleep(duration: Duration) { - await Promise.race([ - new Promise(resolve => setTimeout(resolve, duration.as('milliseconds'))), - this.cancelToken.promise, - ]); - } } diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index 881a013360..4b190c69c6 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -16,12 +16,12 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; +import { AbortController, AbortSignal } from 'node-abort-controller'; import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; -import { CancelToken } from './CancelToken'; import { TaskSettingsV1, taskSettingsV1Schema } from './types'; -import { nowPlus } from './util'; +import { nowPlus, sleep } from './util'; const WORK_CHECK_FREQUENCY = Duration.fromObject({ seconds: 5 }); @@ -35,7 +35,8 @@ export class TaskWorker { private readonly fn: () => void | Promise; private readonly knex: Knex; private readonly logger: Logger; - private readonly cancelToken: CancelToken; + private readonly abortController: AbortController; + private readonly abortSignal: AbortSignal; constructor( taskId: string, @@ -47,7 +48,8 @@ export class TaskWorker { this.fn = fn; this.knex = knex; this.logger = logger; - this.cancelToken = CancelToken.create(); + this.abortController = new AbortController(); + this.abortSignal = this.abortController.signal; } async start(settings: TaskSettingsV1) { @@ -63,13 +65,13 @@ export class TaskWorker { (async () => { try { - while (!this.cancelToken.isCancelled) { + while (!this.abortSignal.aborted) { const runResult = await this.runOnce(); if (runResult.result === 'abort') { break; } - await this.sleep(WORK_CHECK_FREQUENCY); + await sleep(WORK_CHECK_FREQUENCY, this.abortSignal); } this.logger.info(`Task worker finished: ${this.taskId}`); } catch (e) { @@ -79,7 +81,7 @@ export class TaskWorker { } stop() { - this.cancelToken.cancel(); + this.abortController.abort(); } /** @@ -120,19 +122,6 @@ export class TaskWorker { return { result: 'completed' }; } - /** - * Sleep for the given duration, but abort sooner if the cancel token - * triggers. - * - * @param duration - The amount of time to sleep, at most - */ - private async sleep(duration: Duration): Promise { - await Promise.race([ - new Promise(resolve => setTimeout(resolve, duration.as('milliseconds'))), - this.cancelToken.promise, - ]); - } - /** * Perform the initial store of the task info */ diff --git a/packages/backend-tasks/src/tasks/util.ts b/packages/backend-tasks/src/tasks/util.ts index dd5c2d4b7c..72753a46b2 100644 --- a/packages/backend-tasks/src/tasks/util.ts +++ b/packages/backend-tasks/src/tasks/util.ts @@ -17,6 +17,7 @@ import { InputError } from '@backstage/errors'; import { Knex } from 'knex'; import { DateTime, Duration } from 'luxon'; +import { AbortSignal } from 'node-abort-controller'; // Keep the IDs compatible with e.g. Prometheus export function validateId(id: string) { @@ -43,3 +44,34 @@ export function nowPlus(duration: Duration | undefined, knex: Knex) { ? knex.raw(`datetime('now', ?)`, [`${seconds} seconds`]) : knex.raw(`now() + interval '${seconds} seconds'`); } + +/** + * Sleep for the given duration, but return sooner if the abort signal + * triggers. + * + * @param duration - The amount of time to sleep, at most + * @param abortSignal - An optional abort signal that short circuits the wait + */ +export async function sleep( + duration: Duration, + abortSignal?: AbortSignal, +): Promise { + if (abortSignal?.aborted) { + return; + } + + await new Promise(resolve => { + let timeoutHandle: NodeJS.Timeout | undefined = undefined; + + const done = () => { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } + abortSignal?.removeEventListener('abort', done); + resolve(); + }; + + timeoutHandle = setTimeout(done, duration.as('milliseconds')); + abortSignal?.addEventListener('abort', done); + }); +} From d8c28d99cf597a45ad757b1bf40ba3077b514753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 21:11:19 +0200 Subject: [PATCH 15/27] make the original task definition into just one object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/README.md | 12 +++++----- packages/backend-tasks/api-report.md | 22 +++++++++---------- .../src/tasks/PluginTaskManagerImpl.test.ts | 12 +++++----- .../src/tasks/PluginTaskManagerImpl.ts | 20 ++++++++--------- .../src/tasks/TaskManager.test.ts | 14 +++++------- packages/backend-tasks/src/tasks/index.ts | 2 +- packages/backend-tasks/src/tasks/types.ts | 20 +++++++++++------ 7 files changed, 49 insertions(+), 53 deletions(-) diff --git a/packages/backend-tasks/README.md b/packages/backend-tasks/README.md index 71c54b0a7f..abb25079e3 100644 --- a/packages/backend-tasks/README.md +++ b/packages/backend-tasks/README.md @@ -19,15 +19,13 @@ import { TaskManager } from '@backstage/backend-tasks'; const manager = TaskManager.fromConfig(rootConfig).forPlugin('my-plugin'); -const { unschedule } = await manager.scheduleTask( - 'refresh-things', - { - frequency: Duration.fromObject({ minutes: 10 }), - }, - async () => { +const { unschedule } = await manager.scheduleTask({ + id: 'refresh-things', + frequency: Duration.fromObject({ minutes: 10 }), + fn: async () => { await entityProvider.run(); }, -); +}); ``` ## Documentation diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index e9d7884b43..f3aaf0e9bf 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -10,15 +10,20 @@ import { Logger as Logger_2 } from 'winston'; // @public export interface PluginTaskManager { - scheduleTask( - id: string, - options: TaskOptions, - fn: () => void | Promise, - ): Promise<{ + scheduleTask(task: TaskDefinition): Promise<{ unschedule: () => Promise; }>; } +// @public +export interface TaskDefinition { + fn: () => void | Promise; + frequency: Duration; + id: string; + initialDelay?: Duration; + timeout: Duration; +} + // @public export class TaskManager { constructor(databaseManager: DatabaseManager, logger: Logger_2); @@ -32,11 +37,4 @@ export class TaskManager { }, ): TaskManager; } - -// @public -export interface TaskOptions { - frequency?: Duration; - initialDelay?: Duration; - timeout?: Duration; -} ``` diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts index 193dd2a95d..bbb54a2aa0 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts @@ -45,14 +45,12 @@ describe('PluginTaskManagerImpl', () => { const { manager } = await init(databaseId); const fn = jest.fn(); - const { unschedule } = await manager.scheduleTask( - 'task1', - { - timeout: Duration.fromMillis(5000), - frequency: Duration.fromMillis(5000), - }, + const { unschedule } = await manager.scheduleTask({ + id: 'task1', + timeout: Duration.fromMillis(5000), + frequency: Duration.fromMillis(5000), fn, - ); + }); await waitForExpect(() => { expect(fn).toBeCalled(); diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts index 1b890a69c8..fd543cf828 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts @@ -17,7 +17,7 @@ import { Knex } from 'knex'; import { Logger } from 'winston'; import { TaskWorker } from './TaskWorker'; -import { PluginTaskManager, TaskOptions } from './types'; +import { PluginTaskManager, TaskDefinition } from './types'; import { validateId } from './util'; /** @@ -30,25 +30,23 @@ export class PluginTaskManagerImpl implements PluginTaskManager { ) {} async scheduleTask( - id: string, - options: TaskOptions, - fn: () => void | Promise, + task: TaskDefinition, ): Promise<{ unschedule: () => Promise }> { - validateId(id); + validateId(task.id); const knex = await this.databaseFactory(); - const task = new TaskWorker(id, fn, knex, this.logger); - await task.start({ + const worker = new TaskWorker(task.id, task.fn, knex, this.logger); + await worker.start({ version: 1, - initialDelayDuration: options.initialDelay?.toISO(), - recurringAtMostEveryDuration: options.frequency.toISO(), - timeoutAfterDuration: options.timeout.toISO(), + initialDelayDuration: task.initialDelay?.toISO(), + recurringAtMostEveryDuration: task.frequency.toISO(), + timeoutAfterDuration: task.timeout.toISO(), }); return { async unschedule() { - await task.stop(); + await worker.stop(); }, }; } diff --git a/packages/backend-tasks/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskManager.test.ts index 002db0b3de..c81cfc8ee9 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.test.ts @@ -43,14 +43,12 @@ describe('TaskManager', () => { const database = await createDatabase(databaseId); const manager = new TaskManager(database, logger).forPlugin('test'); - const task = await manager.scheduleTask( - 'task1', - { - timeout: Duration.fromMillis(5000), - frequency: Duration.fromMillis(5000), - }, - () => {}, - ); + const task = await manager.scheduleTask({ + id: 'task1', + timeout: Duration.fromMillis(5000), + frequency: Duration.fromMillis(5000), + fn: () => {}, + }); expect(task.unschedule).toBeDefined(); }, ); diff --git a/packages/backend-tasks/src/tasks/index.ts b/packages/backend-tasks/src/tasks/index.ts index d16a30b6c9..e4fe1ad2a3 100644 --- a/packages/backend-tasks/src/tasks/index.ts +++ b/packages/backend-tasks/src/tasks/index.ts @@ -15,4 +15,4 @@ */ export { TaskManager } from './TaskManager'; -export type { PluginTaskManager, TaskOptions } from './types'; +export type { PluginTaskManager, TaskDefinition } from './types'; diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index 989414ec12..e2dfbea0a1 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -22,7 +22,17 @@ import { z } from 'zod'; * * @public */ -export interface TaskOptions { +export interface TaskDefinition { + /** + * A unique ID (within the scope of the plugin) for the task. + */ + id: string; + + /** + * The actual task function to be invoked regularly. + */ + fn: () => void | Promise; + /** * The maximum amount of time that a single task invocation can take, before * it's considered timed out and gets "released" such that a new invocation @@ -77,17 +87,13 @@ export interface PluginTaskManager { * its options are just overwritten with the given options, and things * continue from there. * - * @param id - A unique ID (within the scope of the plugin) for the task - * @param options - Options for the task - * @param fn - The actual task function to be invoked + * @param definition - The task definition * @returns An `unschedule` function that can be used to stop the task * invocations later on. This removes the task entirely from storage * and stops its invocations across all workers. */ scheduleTask( - id: string, - options: TaskOptions, - fn: () => void | Promise, + task: TaskDefinition, ): Promise<{ unschedule: () => Promise }>; } From 9bee0b9f719abb758cf58107aecbe1738edaa5c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 21:55:55 +0200 Subject: [PATCH 16/27] make it possible to abort tasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/api-report.md | 10 ++++- .../backend-tasks/src/tasks/TaskWorker.ts | 34 ++++++++------- packages/backend-tasks/src/tasks/index.ts | 2 +- packages/backend-tasks/src/tasks/types.ts | 7 +++- packages/backend-tasks/src/tasks/util.test.ts | 42 +++++++++++++++++++ packages/backend-tasks/src/tasks/util.ts | 30 ++++++++++++- 6 files changed, 106 insertions(+), 19 deletions(-) create mode 100644 packages/backend-tasks/src/tasks/util.test.ts diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index f3aaf0e9bf..68c4358348 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -3,6 +3,7 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +import { AbortSignal as AbortSignal_2 } from 'node-abort-controller'; import { Config } from '@backstage/config'; import { DatabaseManager } from '@backstage/backend-common'; import { Duration } from 'luxon'; @@ -17,13 +18,20 @@ export interface PluginTaskManager { // @public export interface TaskDefinition { - fn: () => void | Promise; + fn: TaskFunction; frequency: Duration; id: string; initialDelay?: Duration; timeout: Duration; } +// Warning: (ae-missing-release-tag) "TaskFunction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export type TaskFunction = + | ((abortSignal: AbortSignal_2) => void | Promise) + | (() => void | Promise); + // @public export class TaskManager { constructor(databaseManager: DatabaseManager, logger: Logger_2); diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index 4b190c69c6..9be1f2c1dc 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -16,12 +16,12 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; -import { AbortController, AbortSignal } from 'node-abort-controller'; +import { AbortController } from 'node-abort-controller'; import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; -import { TaskSettingsV1, taskSettingsV1Schema } from './types'; -import { nowPlus, sleep } from './util'; +import { TaskFunction, TaskSettingsV1, taskSettingsV1Schema } from './types'; +import { delegateAbortController, nowPlus, sleep } from './util'; const WORK_CHECK_FREQUENCY = Duration.fromObject({ seconds: 5 }); @@ -32,24 +32,17 @@ const WORK_CHECK_FREQUENCY = Duration.fromObject({ seconds: 5 }); */ export class TaskWorker { private readonly taskId: string; - private readonly fn: () => void | Promise; + private readonly fn: TaskFunction; private readonly knex: Knex; private readonly logger: Logger; private readonly abortController: AbortController; - private readonly abortSignal: AbortSignal; - constructor( - taskId: string, - fn: () => void | Promise, - knex: Knex, - logger: Logger, - ) { + constructor(taskId: string, fn: TaskFunction, knex: Knex, logger: Logger) { this.taskId = taskId; this.fn = fn; this.knex = knex; this.logger = logger; this.abortController = new AbortController(); - this.abortSignal = this.abortController.signal; } async start(settings: TaskSettingsV1) { @@ -65,13 +58,13 @@ export class TaskWorker { (async () => { try { - while (!this.abortSignal.aborted) { + while (!this.abortController.signal.aborted) { const runResult = await this.runOnce(); if (runResult.result === 'abort') { break; } - await sleep(WORK_CHECK_FREQUENCY, this.abortSignal); + await sleep(WORK_CHECK_FREQUENCY, this.abortController.signal); } this.logger.info(`Task worker finished: ${this.taskId}`); } catch (e) { @@ -111,11 +104,22 @@ export class TaskWorker { return { result: 'not-ready-yet' }; } + // Abort the task execution either if the worker is stopped, or if the + // task timeout is hit + const taskAbortController = delegateAbortController( + this.abortController.signal, + ); + const timeoutHandle = setTimeout(() => { + taskAbortController.abort(); + }, Duration.fromISO(taskSettings.timeoutAfterDuration).as('milliseconds')); + try { - await this.fn(); + await this.fn(taskAbortController.signal); } catch (e) { await this.tryReleaseTask(ticket, taskSettings); return { result: 'failed' }; + } finally { + clearTimeout(timeoutHandle); } await this.tryReleaseTask(ticket, taskSettings); diff --git a/packages/backend-tasks/src/tasks/index.ts b/packages/backend-tasks/src/tasks/index.ts index e4fe1ad2a3..bb59b3a3e2 100644 --- a/packages/backend-tasks/src/tasks/index.ts +++ b/packages/backend-tasks/src/tasks/index.ts @@ -15,4 +15,4 @@ */ export { TaskManager } from './TaskManager'; -export type { PluginTaskManager, TaskDefinition } from './types'; +export type { PluginTaskManager, TaskDefinition, TaskFunction } from './types'; diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index e2dfbea0a1..3be6591887 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -15,8 +15,13 @@ */ import { Duration } from 'luxon'; +import { AbortSignal } from 'node-abort-controller'; import { z } from 'zod'; +export type TaskFunction = + | ((abortSignal: AbortSignal) => void | Promise) + | (() => void | Promise); + /** * Options that apply to the invocation of a given task. * @@ -31,7 +36,7 @@ export interface TaskDefinition { /** * The actual task function to be invoked regularly. */ - fn: () => void | Promise; + fn: TaskFunction; /** * The maximum amount of time that a single task invocation can take, before diff --git a/packages/backend-tasks/src/tasks/util.test.ts b/packages/backend-tasks/src/tasks/util.test.ts new file mode 100644 index 0000000000..75fd9f3673 --- /dev/null +++ b/packages/backend-tasks/src/tasks/util.test.ts @@ -0,0 +1,42 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { AbortController } from 'node-abort-controller'; +import { delegateAbortController } from './util'; + +describe('util', () => { + describe('delegateAbortController', () => { + it('inherits parent abort state', () => { + const parent = new AbortController(); + const child = delegateAbortController(parent.signal); + expect(parent.signal.aborted).toBe(false); + expect(child.signal.aborted).toBe(false); + parent.abort(); + expect(parent.signal.aborted).toBe(true); + expect(child.signal.aborted).toBe(true); + }); + + it('does not inherit from child to parent', () => { + const parent = new AbortController(); + const child = delegateAbortController(parent.signal); + expect(parent.signal.aborted).toBe(false); + expect(child.signal.aborted).toBe(false); + child.abort(); + expect(parent.signal.aborted).toBe(false); + expect(child.signal.aborted).toBe(true); + }); + }); +}); diff --git a/packages/backend-tasks/src/tasks/util.ts b/packages/backend-tasks/src/tasks/util.ts index 72753a46b2..378224ed57 100644 --- a/packages/backend-tasks/src/tasks/util.ts +++ b/packages/backend-tasks/src/tasks/util.ts @@ -17,7 +17,7 @@ import { InputError } from '@backstage/errors'; import { Knex } from 'knex'; import { DateTime, Duration } from 'luxon'; -import { AbortSignal } from 'node-abort-controller'; +import { AbortController, AbortSignal } from 'node-abort-controller'; // Keep the IDs compatible with e.g. Prometheus export function validateId(id: string) { @@ -75,3 +75,31 @@ export async function sleep( abortSignal?.addEventListener('abort', done); }); } + +/** + * Creates a new AbortController that, in addition to working as a regular + * standalone controller, also gets aborted if the given parent signal + * reaches aborted state. + * + * @param parent - The "parent" signal that can trigger the delegate + */ +export function delegateAbortController(parent: AbortSignal): AbortController { + const delegate = new AbortController(); + + if (parent.aborted) { + delegate.abort(); + } else { + const onParentAborted = () => { + delegate.abort(); + }; + + const onChildAborted = () => { + parent.removeEventListener('abort', onParentAborted); + }; + + parent.addEventListener('abort', onParentAborted, { once: true }); + delegate.signal.addEventListener('abort', onChildAborted, { once: true }); + } + + return delegate; +} From 26cf7f631d7a61004c76806b762b624555c80be7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 22:40:52 +0200 Subject: [PATCH 17/27] final cleanup, removing unschedule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/README.md | 2 +- packages/backend-tasks/api-report.md | 5 ++-- .../migrations/20210928160613_init.js | 2 +- packages/backend-tasks/src/database/tables.ts | 2 +- .../src/tasks/PluginTaskManagerImpl.test.ts | 4 +-- .../src/tasks/PluginTaskManagerImpl.ts | 25 ++++++++----------- .../src/tasks/PluginTaskManagerJanitor.ts | 24 ++++++++---------- .../src/tasks/TaskManager.test.ts | 11 +++++--- .../backend-tasks/src/tasks/TaskWorker.ts | 24 +++++++----------- packages/backend-tasks/src/tasks/types.ts | 13 +++++----- packages/backend-tasks/src/tasks/util.ts | 24 ++++++++++-------- 11 files changed, 65 insertions(+), 71 deletions(-) diff --git a/packages/backend-tasks/README.md b/packages/backend-tasks/README.md index abb25079e3..e98c997af0 100644 --- a/packages/backend-tasks/README.md +++ b/packages/backend-tasks/README.md @@ -19,7 +19,7 @@ import { TaskManager } from '@backstage/backend-tasks'; const manager = TaskManager.fromConfig(rootConfig).forPlugin('my-plugin'); -const { unschedule } = await manager.scheduleTask({ +await manager.scheduleTask({ id: 'refresh-things', frequency: Duration.fromObject({ minutes: 10 }), fn: async () => { diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 68c4358348..8f7209b9ed 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -11,9 +11,7 @@ import { Logger as Logger_2 } from 'winston'; // @public export interface PluginTaskManager { - scheduleTask(task: TaskDefinition): Promise<{ - unschedule: () => Promise; - }>; + scheduleTask(task: TaskDefinition): Promise; } // @public @@ -22,6 +20,7 @@ export interface TaskDefinition { frequency: Duration; id: string; initialDelay?: Duration; + signal?: AbortSignal_2; timeout: Duration; } diff --git a/packages/backend-tasks/migrations/20210928160613_init.js b/packages/backend-tasks/migrations/20210928160613_init.js index c5028e2e93..cd812b59e8 100644 --- a/packages/backend-tasks/migrations/20210928160613_init.js +++ b/packages/backend-tasks/migrations/20210928160613_init.js @@ -36,7 +36,7 @@ exports.up = async function up(knex) { .comment('JSON serialized object with properties for this task'); table .dateTime('next_run_start_at') - .nullable() + .notNullable() .comment('The next time that the task should be started'); table .text('current_run_ticket') diff --git a/packages/backend-tasks/src/database/tables.ts b/packages/backend-tasks/src/database/tables.ts index 4777c4e60c..63aad6e42a 100644 --- a/packages/backend-tasks/src/database/tables.ts +++ b/packages/backend-tasks/src/database/tables.ts @@ -20,7 +20,7 @@ export const DB_TASKS_TABLE = 'backstage_backend_tasks__tasks'; export type DbTasksRow = { id: string; settings_json: string; - next_run_start_at?: Date | string; + next_run_start_at: Date; current_run_ticket?: string; current_run_started_at?: Date | string; current_run_expires_at?: Date | string; diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts index bbb54a2aa0..27fd1f9f3b 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts @@ -45,7 +45,7 @@ describe('PluginTaskManagerImpl', () => { const { manager } = await init(databaseId); const fn = jest.fn(); - const { unschedule } = await manager.scheduleTask({ + await manager.scheduleTask({ id: 'task1', timeout: Duration.fromMillis(5000), frequency: Duration.fromMillis(5000), @@ -55,8 +55,6 @@ describe('PluginTaskManagerImpl', () => { await waitForExpect(() => { expect(fn).toBeCalled(); }); - - await unschedule(); }, ); }); diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts index fd543cf828..497a58b348 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts @@ -29,25 +29,22 @@ export class PluginTaskManagerImpl implements PluginTaskManager { private readonly logger: Logger, ) {} - async scheduleTask( - task: TaskDefinition, - ): Promise<{ unschedule: () => Promise }> { + async scheduleTask(task: TaskDefinition): Promise { validateId(task.id); const knex = await this.databaseFactory(); const worker = new TaskWorker(task.id, task.fn, knex, this.logger); - await worker.start({ - version: 1, - initialDelayDuration: task.initialDelay?.toISO(), - recurringAtMostEveryDuration: task.frequency.toISO(), - timeoutAfterDuration: task.timeout.toISO(), - }); - - return { - async unschedule() { - await worker.stop(); + await worker.start( + { + version: 1, + initialDelayDuration: task.initialDelay?.toISO(), + recurringAtMostEveryDuration: task.frequency.toISO(), + timeoutAfterDuration: task.timeout.toISO(), }, - }; + { + signal: task.signal, + }, + ); } } diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts index ec65eadf1e..6594243243 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts @@ -16,7 +16,7 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; -import { AbortController, AbortSignal } from 'node-abort-controller'; +import { AbortSignal } from 'node-abort-controller'; import { Logger } from 'winston'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; import { sleep } from './util'; @@ -29,8 +29,6 @@ export class PluginTaskManagerJanitor { private readonly knex: Knex; private readonly waitBetweenRuns: Duration; private readonly logger: Logger; - private readonly abortController: AbortController; - private readonly abortSignal: AbortSignal; constructor(options: { knex: Knex; @@ -40,26 +38,20 @@ export class PluginTaskManagerJanitor { this.knex = options.knex; this.waitBetweenRuns = options.waitBetweenRuns; this.logger = options.logger; - this.abortController = new AbortController(); - this.abortSignal = this.abortController.signal; } - async start() { - while (!this.abortSignal.aborted) { + async start(abortSignal?: AbortSignal) { + while (!abortSignal?.aborted) { try { await this.runOnce(); } catch (e) { this.logger.warn(`Error while performing janitorial tasks, ${e}`); } - await sleep(this.waitBetweenRuns, this.abortSignal); + await sleep(this.waitBetweenRuns, abortSignal); } } - async stop() { - this.abortController.abort(); - } - private async runOnce() { // SQLite currently (Oct 1 2021) returns a number for returning() // statements, effectively ignoring them and instead returning the outcome @@ -68,9 +60,15 @@ export class PluginTaskManagerJanitor { // https://github.com/knex/knex/issues/4370 // https://github.com/mapbox/node-sqlite3/issues/1453 + const dbNull = this.knex.raw('null'); + const tasksQuery = this.knex(DB_TASKS_TABLE) .where('current_run_expires_at', '<', this.knex.fn.now()) - .delete(); + .update({ + current_run_ticket: dbNull, + current_run_started_at: dbNull, + current_run_expires_at: dbNull, + }); if (this.knex.client.config.client === 'sqlite3') { const tasks = await tasksQuery; diff --git a/packages/backend-tasks/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskManager.test.ts index c81cfc8ee9..43fe79709c 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.test.ts @@ -18,6 +18,7 @@ import { DatabaseManager, getVoidLogger } from '@backstage/backend-common'; import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; import { TaskManager } from './TaskManager'; +import waitForExpect from 'wait-for-expect'; describe('TaskManager', () => { const logger = getVoidLogger(); @@ -42,14 +43,18 @@ describe('TaskManager', () => { async databaseId => { const database = await createDatabase(databaseId); const manager = new TaskManager(database, logger).forPlugin('test'); + const fn = jest.fn(); - const task = await manager.scheduleTask({ + await manager.scheduleTask({ id: 'task1', timeout: Duration.fromMillis(5000), frequency: Duration.fromMillis(5000), - fn: () => {}, + fn, + }); + + await waitForExpect(() => { + expect(fn).toBeCalled(); }); - expect(task.unschedule).toBeDefined(); }, ); }); diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts index 9be1f2c1dc..991d97b956 100644 --- a/packages/backend-tasks/src/tasks/TaskWorker.ts +++ b/packages/backend-tasks/src/tasks/TaskWorker.ts @@ -16,7 +16,7 @@ import { Knex } from 'knex'; import { Duration } from 'luxon'; -import { AbortController } from 'node-abort-controller'; +import { AbortSignal } from 'node-abort-controller'; import { v4 as uuid } from 'uuid'; import { Logger } from 'winston'; import { DbTasksRow, DB_TASKS_TABLE } from '../database/tables'; @@ -35,17 +35,15 @@ export class TaskWorker { private readonly fn: TaskFunction; private readonly knex: Knex; private readonly logger: Logger; - private readonly abortController: AbortController; constructor(taskId: string, fn: TaskFunction, knex: Knex, logger: Logger) { this.taskId = taskId; this.fn = fn; this.knex = knex; this.logger = logger; - this.abortController = new AbortController(); } - async start(settings: TaskSettingsV1) { + async start(settings: TaskSettingsV1, options?: { signal?: AbortSignal }) { try { await this.persistTask(settings); } catch (e) { @@ -58,13 +56,13 @@ export class TaskWorker { (async () => { try { - while (!this.abortController.signal.aborted) { - const runResult = await this.runOnce(); + while (!options?.signal?.aborted) { + const runResult = await this.runOnce(options?.signal); if (runResult.result === 'abort') { break; } - await sleep(WORK_CHECK_FREQUENCY, this.abortController.signal); + await sleep(WORK_CHECK_FREQUENCY, options?.signal); } this.logger.info(`Task worker finished: ${this.taskId}`); } catch (e) { @@ -73,16 +71,14 @@ export class TaskWorker { })(); } - stop() { - this.abortController.abort(); - } - /** * Makes a single attempt at running the task to completion, if ready. * * @returns The outcome of the attempt */ - async runOnce(): Promise< + async runOnce( + signal?: AbortSignal, + ): Promise< | { result: 'not-ready-yet' } | { result: 'abort' } | { result: 'failed' } @@ -106,9 +102,7 @@ export class TaskWorker { // Abort the task execution either if the worker is stopped, or if the // task timeout is hit - const taskAbortController = delegateAbortController( - this.abortController.signal, - ); + const taskAbortController = delegateAbortController(signal); const timeoutHandle = setTimeout(() => { taskAbortController.abort(); }, Duration.fromISO(taskSettings.timeoutAfterDuration).as('milliseconds')); diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index 3be6591887..4cd33fd3e1 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -38,6 +38,12 @@ export interface TaskDefinition { */ fn: TaskFunction; + /** + * An abort signal that, when triggered, will stop the recurring execution of + * the task. + */ + signal?: AbortSignal; + /** * The maximum amount of time that a single task invocation can take, before * it's considered timed out and gets "released" such that a new invocation @@ -93,13 +99,8 @@ export interface PluginTaskManager { * continue from there. * * @param definition - The task definition - * @returns An `unschedule` function that can be used to stop the task - * invocations later on. This removes the task entirely from storage - * and stops its invocations across all workers. */ - scheduleTask( - task: TaskDefinition, - ): Promise<{ unschedule: () => Promise }>; + scheduleTask(task: TaskDefinition): Promise; } function isValidOptionalDurationString(d: string | undefined): boolean { diff --git a/packages/backend-tasks/src/tasks/util.ts b/packages/backend-tasks/src/tasks/util.ts index 378224ed57..0509f29363 100644 --- a/packages/backend-tasks/src/tasks/util.ts +++ b/packages/backend-tasks/src/tasks/util.ts @@ -83,22 +83,24 @@ export async function sleep( * * @param parent - The "parent" signal that can trigger the delegate */ -export function delegateAbortController(parent: AbortSignal): AbortController { +export function delegateAbortController(parent?: AbortSignal): AbortController { const delegate = new AbortController(); - if (parent.aborted) { - delegate.abort(); - } else { - const onParentAborted = () => { + if (parent) { + if (parent.aborted) { delegate.abort(); - }; + } else { + const onParentAborted = () => { + delegate.abort(); + }; - const onChildAborted = () => { - parent.removeEventListener('abort', onParentAborted); - }; + const onChildAborted = () => { + parent.removeEventListener('abort', onParentAborted); + }; - parent.addEventListener('abort', onParentAborted, { once: true }); - delegate.signal.addEventListener('abort', onChildAborted, { once: true }); + parent.addEventListener('abort', onParentAborted, { once: true }); + delegate.signal.addEventListener('abort', onChildAborted, { once: true }); + } } return delegate; From b5c106ac4cdbe141c25b7ba6782f5ea32a7f96e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 27 Oct 2021 22:49:23 +0200 Subject: [PATCH 18/27] fix api export and test timeouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/api-report.md | 4 +--- .../backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts | 1 + packages/backend-tasks/src/tasks/TaskManager.test.ts | 1 + packages/backend-tasks/src/tasks/types.ts | 8 ++++++++ 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 8f7209b9ed..2be69f8788 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -24,9 +24,7 @@ export interface TaskDefinition { timeout: Duration; } -// Warning: (ae-missing-release-tag) "TaskFunction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export type TaskFunction = | ((abortSignal: AbortSignal_2) => void | Promise) | (() => void | Promise); diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts index 27fd1f9f3b..cbf754d5cf 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts @@ -56,6 +56,7 @@ describe('PluginTaskManagerImpl', () => { expect(fn).toBeCalled(); }); }, + 60_000, ); }); }); diff --git a/packages/backend-tasks/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskManager.test.ts index 43fe79709c..3f2d9dbc54 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskManager.test.ts @@ -56,5 +56,6 @@ describe('TaskManager', () => { expect(fn).toBeCalled(); }); }, + 60_000, ); }); diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index 4cd33fd3e1..c717a17c3b 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -18,6 +18,14 @@ import { Duration } from 'luxon'; import { AbortSignal } from 'node-abort-controller'; import { z } from 'zod'; +/** + * A function that can be called as a scheduled task. + * + * It may optionally accept an abort signal argument. When the signal triggers, + * processing should abort and return as quickly as possible. + * + * @public + */ export type TaskFunction = | ((abortSignal: AbortSignal) => void | Promise) | (() => void | Promise); From 92d85b80e92edaba95732aac8b64993f2f6290f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 28 Oct 2021 11:08:19 +0200 Subject: [PATCH 19/27] more tests and a review comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/README.md | 1 + packages/backend-tasks/src/tasks/util.test.ts | 36 ++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/backend-tasks/README.md b/packages/backend-tasks/README.md index e98c997af0..f8a2009af0 100644 --- a/packages/backend-tasks/README.md +++ b/packages/backend-tasks/README.md @@ -16,6 +16,7 @@ then make use of its facilities as necessary: ```typescript import { TaskManager } from '@backstage/backend-tasks'; +import { Duration } from 'luxon'; const manager = TaskManager.fromConfig(rootConfig).forPlugin('my-plugin'); diff --git a/packages/backend-tasks/src/tasks/util.test.ts b/packages/backend-tasks/src/tasks/util.test.ts index 75fd9f3673..f74669a7f4 100644 --- a/packages/backend-tasks/src/tasks/util.test.ts +++ b/packages/backend-tasks/src/tasks/util.test.ts @@ -14,10 +14,44 @@ * limitations under the License. */ +import { Duration } from 'luxon'; import { AbortController } from 'node-abort-controller'; -import { delegateAbortController } from './util'; +import { delegateAbortController, sleep, validateId } from './util'; describe('util', () => { + describe('validateId', () => { + it.each(['a', 'a_b', 'ab123c_2'])( + 'accepts valid inputs, %p', + async input => { + expect(validateId(input)).toBeUndefined(); + }, + ); + + it.each(['', 'a!', 'A', 'a-b', 'a.b', '_a', 'a_', null, Symbol('a')])( + 'rejects invalid inputs, %p', + async input => { + expect(() => validateId(input as any)).toThrow(); + }, + ); + }); + + describe('sleep', () => { + it('finishes the wait as expected with no signal', async () => { + const ac = new AbortController(); + const start = Date.now(); + await sleep(Duration.fromObject({ seconds: 1 }), ac.signal); + expect(Date.now() - start).toBeGreaterThan(800); + }, 5_000); + + it('aborts properly on the signal', async () => { + const ac = new AbortController(); + const promise = sleep(Duration.fromObject({ seconds: 10 }), ac.signal); + ac.abort(); + await promise; + expect(true).toBe(true); + }, 1_000); + }); + describe('delegateAbortController', () => { it('inherits parent abort state', () => { const parent = new AbortController(); From 85cc6bd6aaac7524e7d62d1463c80ecde7b9eaca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 28 Oct 2021 15:25:37 +0200 Subject: [PATCH 20/27] rename to TaskScheduler to reduce collision risk with other concepts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/README.md | 8 ++++---- packages/backend-tasks/api-report.md | 9 +++++---- packages/backend-tasks/package.json | 2 +- packages/backend-tasks/src/index.ts | 2 +- ...est.ts => PluginTaskSchedulerImpl.test.ts} | 4 ++-- ...agerImpl.ts => PluginTaskSchedulerImpl.ts} | 4 ++-- ...nitor.ts => PluginTaskSchedulerJanitor.ts} | 2 +- ...kManager.test.ts => TaskScheduler.test.ts} | 6 +++--- .../{TaskManager.ts => TaskScheduler.ts} | 20 +++++++++---------- packages/backend-tasks/src/tasks/index.ts | 8 ++++++-- packages/backend-tasks/src/tasks/types.ts | 5 ++--- 11 files changed, 37 insertions(+), 33 deletions(-) rename packages/backend-tasks/src/tasks/{PluginTaskManagerImpl.test.ts => PluginTaskSchedulerImpl.test.ts} (94%) rename packages/backend-tasks/src/tasks/{PluginTaskManagerImpl.ts => PluginTaskSchedulerImpl.ts} (91%) rename packages/backend-tasks/src/tasks/{PluginTaskManagerJanitor.ts => PluginTaskSchedulerJanitor.ts} (98%) rename packages/backend-tasks/src/tasks/{TaskManager.test.ts => TaskScheduler.test.ts} (91%) rename packages/backend-tasks/src/tasks/{TaskManager.ts => TaskScheduler.ts} (80%) diff --git a/packages/backend-tasks/README.md b/packages/backend-tasks/README.md index f8a2009af0..d3c361c02c 100644 --- a/packages/backend-tasks/README.md +++ b/packages/backend-tasks/README.md @@ -1,6 +1,6 @@ # @backstage/backend-tasks -Common distributed task management / locking library for Backstage backends. +Common distributed task management for Backstage backends. ## Usage @@ -15,12 +15,12 @@ yarn add @backstage/backend-tasks then make use of its facilities as necessary: ```typescript -import { TaskManager } from '@backstage/backend-tasks'; +import { TaskScheduler } from '@backstage/backend-tasks'; import { Duration } from 'luxon'; -const manager = TaskManager.fromConfig(rootConfig).forPlugin('my-plugin'); +const scheduler = TaskScheduler.fromConfig(rootConfig).forPlugin('my-plugin'); -await manager.scheduleTask({ +await scheduler.scheduleTask({ id: 'refresh-things', frequency: Duration.fromObject({ minutes: 10 }), fn: async () => { diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 2be69f8788..27259304c8 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -10,7 +10,7 @@ import { Duration } from 'luxon'; import { Logger as Logger_2 } from 'winston'; // @public -export interface PluginTaskManager { +export interface PluginTaskScheduler { scheduleTask(task: TaskDefinition): Promise; } @@ -30,9 +30,10 @@ export type TaskFunction = | (() => void | Promise); // @public -export class TaskManager { +export class TaskScheduler { constructor(databaseManager: DatabaseManager, logger: Logger_2); - forPlugin(pluginId: string): PluginTaskManager; + // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "@backstage/backend-tasks" does not have an export "PluginTaskManager" + forPlugin(pluginId: string): PluginTaskScheduler; // (undocumented) static fromConfig( config: Config, @@ -40,6 +41,6 @@ export class TaskManager { databaseManager?: DatabaseManager; logger?: Logger_2; }, - ): TaskManager; + ): TaskScheduler; } ``` diff --git a/packages/backend-tasks/package.json b/packages/backend-tasks/package.json index dcb5922cf1..01671638e9 100644 --- a/packages/backend-tasks/package.json +++ b/packages/backend-tasks/package.json @@ -1,6 +1,6 @@ { "name": "@backstage/backend-tasks", - "description": "Common distributed task management / locking library for Backstage backends", + "description": "Common distributed task management library for Backstage backends", "version": "0.1.0", "main": "src/index.ts", "types": "src/index.ts", diff --git a/packages/backend-tasks/src/index.ts b/packages/backend-tasks/src/index.ts index 00a8aa2803..dd75aca68c 100644 --- a/packages/backend-tasks/src/index.ts +++ b/packages/backend-tasks/src/index.ts @@ -15,7 +15,7 @@ */ /** - * Common distributed task management / locking library for Backstage backends + * Common distributed task management library for Backstage backends * * @packageDocumentation */ diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts similarity index 94% rename from packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts rename to packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts index cbf754d5cf..e387b85413 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.test.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.test.ts @@ -19,7 +19,7 @@ import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; import waitForExpect from 'wait-for-expect'; import { migrateBackendTasks } from '../database/migrateBackendTasks'; -import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; +import { PluginTaskSchedulerImpl } from './PluginTaskSchedulerImpl'; describe('PluginTaskManagerImpl', () => { const databases = TestDatabases.create({ @@ -29,7 +29,7 @@ describe('PluginTaskManagerImpl', () => { async function init(databaseId: TestDatabaseId) { const knex = await databases.init(databaseId); await migrateBackendTasks(knex); - const manager = new PluginTaskManagerImpl( + const manager = new PluginTaskSchedulerImpl( async () => knex, getVoidLogger(), ); diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts similarity index 91% rename from packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts rename to packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts index 497a58b348..93975bc327 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerImpl.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerImpl.ts @@ -17,13 +17,13 @@ import { Knex } from 'knex'; import { Logger } from 'winston'; import { TaskWorker } from './TaskWorker'; -import { PluginTaskManager, TaskDefinition } from './types'; +import { PluginTaskScheduler, TaskDefinition } from './types'; import { validateId } from './util'; /** * Implements the actual task management. */ -export class PluginTaskManagerImpl implements PluginTaskManager { +export class PluginTaskSchedulerImpl implements PluginTaskScheduler { constructor( private readonly databaseFactory: () => Promise, private readonly logger: Logger, diff --git a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts b/packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.ts similarity index 98% rename from packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts rename to packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.ts index 6594243243..8b90afff42 100644 --- a/packages/backend-tasks/src/tasks/PluginTaskManagerJanitor.ts +++ b/packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.ts @@ -25,7 +25,7 @@ import { sleep } from './util'; * Makes sure to auto-expire and clean up things that time out or for other * reasons should not be left lingering. */ -export class PluginTaskManagerJanitor { +export class PluginTaskSchedulerJanitor { private readonly knex: Knex; private readonly waitBetweenRuns: Duration; private readonly logger: Logger; diff --git a/packages/backend-tasks/src/tasks/TaskManager.test.ts b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts similarity index 91% rename from packages/backend-tasks/src/tasks/TaskManager.test.ts rename to packages/backend-tasks/src/tasks/TaskScheduler.test.ts index 3f2d9dbc54..ce8e797503 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.test.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts @@ -17,10 +17,10 @@ import { DatabaseManager, getVoidLogger } from '@backstage/backend-common'; import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Duration } from 'luxon'; -import { TaskManager } from './TaskManager'; +import { TaskScheduler } from './TaskScheduler'; import waitForExpect from 'wait-for-expect'; -describe('TaskManager', () => { +describe('TaskScheduler', () => { const logger = getVoidLogger(); const databases = TestDatabases.create({ ids: ['POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'], @@ -42,7 +42,7 @@ describe('TaskManager', () => { 'can return a working plugin impl, %p', async databaseId => { const database = await createDatabase(databaseId); - const manager = new TaskManager(database, logger).forPlugin('test'); + const manager = new TaskScheduler(database, logger).forPlugin('test'); const fn = jest.fn(); await manager.scheduleTask({ diff --git a/packages/backend-tasks/src/tasks/TaskManager.ts b/packages/backend-tasks/src/tasks/TaskScheduler.ts similarity index 80% rename from packages/backend-tasks/src/tasks/TaskManager.ts rename to packages/backend-tasks/src/tasks/TaskScheduler.ts index d065447fb2..81d1fca479 100644 --- a/packages/backend-tasks/src/tasks/TaskManager.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.ts @@ -20,29 +20,29 @@ import { memoize } from 'lodash'; import { Duration } from 'luxon'; import { Logger } from 'winston'; import { migrateBackendTasks } from '../database/migrateBackendTasks'; -import { PluginTaskManagerImpl } from './PluginTaskManagerImpl'; -import { PluginTaskManagerJanitor } from './PluginTaskManagerJanitor'; -import { PluginTaskManager } from './types'; +import { PluginTaskSchedulerImpl } from './PluginTaskSchedulerImpl'; +import { PluginTaskSchedulerJanitor } from './PluginTaskSchedulerJanitor'; +import { PluginTaskScheduler } from './types'; /** - * Deals with management and locking related to distributed tasks. + * Deals with the scheduling of distributed tasks. * * @public */ -export class TaskManager { +export class TaskScheduler { static fromConfig( config: Config, options?: { databaseManager?: DatabaseManager; logger?: Logger; }, - ): TaskManager { + ): TaskScheduler { const databaseManager = options?.databaseManager ?? DatabaseManager.fromConfig(config); const logger = (options?.logger || getRootLogger()).child({ type: 'taskManager', }); - return new TaskManager(databaseManager, logger); + return new TaskScheduler(databaseManager, logger); } constructor( @@ -56,13 +56,13 @@ export class TaskManager { * @param pluginId - The unique ID of the plugin, for example "catalog" * @returns A {@link PluginTaskManager} instance */ - forPlugin(pluginId: string): PluginTaskManager { + forPlugin(pluginId: string): PluginTaskScheduler { const databaseFactory = memoize(async () => { const knex = await this.databaseManager.forPlugin(pluginId).getClient(); await migrateBackendTasks(knex); - const janitor = new PluginTaskManagerJanitor({ + const janitor = new PluginTaskSchedulerJanitor({ knex, waitBetweenRuns: Duration.fromObject({ minutes: 1 }), logger: this.logger, @@ -72,7 +72,7 @@ export class TaskManager { return knex; }); - return new PluginTaskManagerImpl( + return new PluginTaskSchedulerImpl( databaseFactory, this.logger.child({ plugin: pluginId }), ); diff --git a/packages/backend-tasks/src/tasks/index.ts b/packages/backend-tasks/src/tasks/index.ts index bb59b3a3e2..9e0a06f71c 100644 --- a/packages/backend-tasks/src/tasks/index.ts +++ b/packages/backend-tasks/src/tasks/index.ts @@ -14,5 +14,9 @@ * limitations under the License. */ -export { TaskManager } from './TaskManager'; -export type { PluginTaskManager, TaskDefinition, TaskFunction } from './types'; +export { TaskScheduler } from './TaskScheduler'; +export type { + PluginTaskScheduler, + TaskDefinition, + TaskFunction, +} from './types'; diff --git a/packages/backend-tasks/src/tasks/types.ts b/packages/backend-tasks/src/tasks/types.ts index c717a17c3b..4693af7ef3 100644 --- a/packages/backend-tasks/src/tasks/types.ts +++ b/packages/backend-tasks/src/tasks/types.ts @@ -92,12 +92,11 @@ export interface TaskDefinition { } /** - * Deals with management and locking related to distributed tasks, for a given - * plugin. + * Deals with the scheduling of distributed tasks, for a given plugin. * * @public */ -export interface PluginTaskManager { +export interface PluginTaskScheduler { /** * Schedules a task function for coordinated exclusive invocation across * workers. From 41009bb3a5cd17e14c88778d90d6f7d766252d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 28 Oct 2021 21:47:29 +0200 Subject: [PATCH 21/27] fix missing link in api report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/api-report.md | 1 - packages/backend-tasks/src/tasks/TaskScheduler.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/backend-tasks/api-report.md b/packages/backend-tasks/api-report.md index 27259304c8..eeb6412840 100644 --- a/packages/backend-tasks/api-report.md +++ b/packages/backend-tasks/api-report.md @@ -32,7 +32,6 @@ export type TaskFunction = // @public export class TaskScheduler { constructor(databaseManager: DatabaseManager, logger: Logger_2); - // Warning: (ae-unresolved-link) The @link reference could not be resolved: The package "@backstage/backend-tasks" does not have an export "PluginTaskManager" forPlugin(pluginId: string): PluginTaskScheduler; // (undocumented) static fromConfig( diff --git a/packages/backend-tasks/src/tasks/TaskScheduler.ts b/packages/backend-tasks/src/tasks/TaskScheduler.ts index 81d1fca479..b24338e5c6 100644 --- a/packages/backend-tasks/src/tasks/TaskScheduler.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.ts @@ -54,7 +54,7 @@ export class TaskScheduler { * Instantiates a task manager instance for the given plugin. * * @param pluginId - The unique ID of the plugin, for example "catalog" - * @returns A {@link PluginTaskManager} instance + * @returns A {@link PluginTaskScheduler} instance */ forPlugin(pluginId: string): PluginTaskScheduler { const databaseFactory = memoize(async () => { From 06b035d120d72d6b37887fa79c87fec44e2ec1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 29 Oct 2021 14:11:15 +0200 Subject: [PATCH 22/27] bump to the latest versions of backstage packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/package.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/backend-tasks/package.json b/packages/backend-tasks/package.json index 01671638e9..10db5cfaac 100644 --- a/packages/backend-tasks/package.json +++ b/packages/backend-tasks/package.json @@ -29,9 +29,9 @@ "clean": "backstage-cli clean" }, "dependencies": { - "@backstage/backend-common": "^0.9.7", - "@backstage/config": "^0.1.10", - "@backstage/errors": "^0.1.3", + "@backstage/backend-common": "^0.9.8", + "@backstage/config": "^0.1.11", + "@backstage/errors": "^0.1.4", "@backstage/types": "^0.1.1", "@types/luxon": "^2.0.4", "knex": "^0.95.1", @@ -43,8 +43,8 @@ "zod": "^3.9.5" }, "devDependencies": { - "@backstage/backend-test-utils": "^0.1.7", - "@backstage/cli": "^0.8.0", + "@backstage/backend-test-utils": "^0.1.8", + "@backstage/cli": "^0.8.1", "jest": "^26.0.1", "wait-for-expect": "^3.0.2" }, From fdfd2f8a6285abb6692adff8f208c7a949cf5c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 29 Oct 2021 15:08:19 +0200 Subject: [PATCH 23/27] remove double config dep in cli MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/forty-ligers-protect.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/forty-ligers-protect.md diff --git a/.changeset/forty-ligers-protect.md b/.changeset/forty-ligers-protect.md new file mode 100644 index 0000000000..e3e46fb7f3 --- /dev/null +++ b/.changeset/forty-ligers-protect.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +remove double config dep From 3bf223818726c7d60da54dd6059940ca1c66e70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 29 Oct 2021 15:25:17 +0200 Subject: [PATCH 24/27] update to the right version of the errors package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/early-cobras-explode.md | 5 +++++ packages/catalog-client/package.json | 2 +- packages/e2e-test/package.json | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 .changeset/early-cobras-explode.md diff --git a/.changeset/early-cobras-explode.md b/.changeset/early-cobras-explode.md new file mode 100644 index 0000000000..34b4cb0e98 --- /dev/null +++ b/.changeset/early-cobras-explode.md @@ -0,0 +1,5 @@ +--- +'@backstage/catalog-client': patch +--- + +Update to the right version of @backstage/errors diff --git a/packages/catalog-client/package.json b/packages/catalog-client/package.json index efdaacc1e0..9caac50c90 100644 --- a/packages/catalog-client/package.json +++ b/packages/catalog-client/package.json @@ -31,7 +31,7 @@ }, "dependencies": { "@backstage/catalog-model": "^0.9.5", - "@backstage/errors": "^0.1.3", + "@backstage/errors": "^0.1.4", "cross-fetch": "^3.0.6" }, "devDependencies": { diff --git a/packages/e2e-test/package.json b/packages/e2e-test/package.json index 1b8e321edb..6c941cda58 100644 --- a/packages/e2e-test/package.json +++ b/packages/e2e-test/package.json @@ -25,7 +25,7 @@ }, "devDependencies": { "@backstage/cli-common": "^0.1.1", - "@backstage/errors": "^0.1.2", + "@backstage/errors": "^0.1.4", "@types/fs-extra": "^9.0.1", "@types/node": "^14.14.32", "@types/puppeteer": "^5.4.4", From e72d9ba9f624ef5430ebe9807f726a8f710302b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 29 Oct 2021 16:23:08 +0200 Subject: [PATCH 25/27] remove unnecessary dep on backend-test-utils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- yarn.lock | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/yarn.lock b/yarn.lock index 4c42941ad9..6eb9e272d1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -29435,6 +29435,11 @@ zip-stream@^4.1.0: compress-commons "^4.1.0" readable-stream "^3.6.0" +zod@^3.9.5: + version "3.11.6" + resolved "https://registry.npmjs.org/zod/-/zod-3.11.6.tgz#e43a5e0c213ae2e02aefe7cb2b1a6fa3d7f1f483" + integrity sha512-daZ80A81I3/9lIydI44motWe6n59kRBfNzTuS2bfzVh1nAXi667TOTWWtatxyG+fwgNUiagSj/CWZwRRbevJIg== + zwitch@^1.0.0: version "1.0.5" resolved "https://registry.npmjs.org/zwitch/-/zwitch-1.0.5.tgz#d11d7381ffed16b742f6af7b3f223d5cd9fe9920" From 3f447ec3e12dd10e3763d228597c2accb1cb2837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 6 Nov 2021 09:58:15 +0100 Subject: [PATCH 26/27] use lodash.once instead of memoize, and skip unnecessary PK index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-tasks/migrations/20210928160613_init.js | 4 ---- packages/backend-tasks/src/tasks/TaskScheduler.ts | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/backend-tasks/migrations/20210928160613_init.js b/packages/backend-tasks/migrations/20210928160613_init.js index cd812b59e8..21d80ae24c 100644 --- a/packages/backend-tasks/migrations/20210928160613_init.js +++ b/packages/backend-tasks/migrations/20210928160613_init.js @@ -50,7 +50,6 @@ exports.up = async function up(knex) { .dateTime('current_run_expires_at') .nullable() .comment('The time that the current task run will time out'); - table.index(['id'], 'backstage_backend_tasks__tasks__id_idx'); }); }; @@ -61,8 +60,5 @@ exports.down = async function down(knex) { // // tasks // - await knex.schema.alterTable('backstage_backend_tasks__tasks', table => { - table.dropIndex([], 'backstage_backend_tasks__tasks__id_idx'); - }); await knex.schema.dropTable('backstage_backend_tasks__tasks'); }; diff --git a/packages/backend-tasks/src/tasks/TaskScheduler.ts b/packages/backend-tasks/src/tasks/TaskScheduler.ts index b24338e5c6..f1668a6691 100644 --- a/packages/backend-tasks/src/tasks/TaskScheduler.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.ts @@ -16,7 +16,7 @@ import { DatabaseManager, getRootLogger } from '@backstage/backend-common'; import { Config } from '@backstage/config'; -import { memoize } from 'lodash'; +import { once } from 'lodash'; import { Duration } from 'luxon'; import { Logger } from 'winston'; import { migrateBackendTasks } from '../database/migrateBackendTasks'; @@ -57,7 +57,7 @@ export class TaskScheduler { * @returns A {@link PluginTaskScheduler} instance */ forPlugin(pluginId: string): PluginTaskScheduler { - const databaseFactory = memoize(async () => { + const databaseFactory = once(async () => { const knex = await this.databaseManager.forPlugin(pluginId).getClient(); await migrateBackendTasks(knex); From 5f606617dd73e60deeae9669c3afbeea985b2c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 11 Nov 2021 20:28:07 +0100 Subject: [PATCH 27/27] fixup yarn.lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/package.json | 2 +- yarn.lock | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 446b3b5afe..03ae1fbe62 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -79,8 +79,8 @@ } }, "devDependencies": { - "@backstage/backend-test-utils": "^0.1.8", "@backstage/cli": "^0.8.2", + "@backstage/test-utils": "^0.1.21", "@types/archiver": "^5.1.0", "@types/compression": "^1.7.0", "@types/concat-stream": "^1.6.0", diff --git a/yarn.lock b/yarn.lock index 6eb9e272d1..c558014df3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -20895,6 +20895,11 @@ node-abi@^2.21.0: dependencies: semver "^5.4.1" +node-abort-controller@^3.0.1: + version "3.0.1" + resolved "https://registry.npmjs.org/node-abort-controller/-/node-abort-controller-3.0.1.tgz#f91fa50b1dee3f909afabb7e261b1e1d6b0cb74e" + integrity sha512-/ujIVxthRs+7q6hsdjHMaj8hRG9NuWmwrz+JdRwZ14jdFoKSkm+vDsCbF9PLpnSqjaWQJuTmVtcWHNLr+vrOFw== + node-addon-api@^3.0.0: version "3.2.1" resolved "https://registry.npmjs.org/node-addon-api/-/node-addon-api-3.2.1.tgz#81325e0a2117789c0128dab65e7e38f07ceba161"