From 25a5e6b5edf90f57936c330b74c09cfab8d07945 Mon Sep 17 00:00:00 2001 From: Mike Cote Date: Mon, 19 Apr 2021 13:41:30 -0400 Subject: [PATCH] Split schedule between cleanup and idle --- .../actions/server/actions_client.test.ts | 3 ++- .../actions/server/actions_config.test.ts | 3 ++- .../ensure_scheduled.test.ts | 3 ++- .../ensure_scheduled.ts | 4 ++-- .../find_and_cleanup_tasks.test.ts | 3 ++- .../find_and_cleanup_tasks.ts | 11 ++++++++-- .../register_task_definition.test.ts | 3 ++- .../task_runner.test.ts | 20 +++++++++++++++++-- .../cleanup_failed_executions/task_runner.ts | 5 ++++- x-pack/plugins/actions/server/config.test.ts | 6 ++++-- x-pack/plugins/actions/server/config.ts | 3 ++- x-pack/plugins/actions/server/plugin.test.ts | 9 ++++++--- 12 files changed, 55 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts index 1a80c39877b16..9b22e31c05e8a 100644 --- a/x-pack/plugins/actions/server/actions_client.test.ts +++ b/x-pack/plugins/actions/server/actions_client.test.ts @@ -415,7 +415,8 @@ describe('create()', () => { responseTimeout: moment.duration('60s'), cleanupFailedExecutionsTask: { enabled: true, - interval: schema.duration().validate('15s'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }, }); diff --git a/x-pack/plugins/actions/server/actions_config.test.ts b/x-pack/plugins/actions/server/actions_config.test.ts index 3b61c048f474b..70c8b0e8185d5 100644 --- a/x-pack/plugins/actions/server/actions_config.test.ts +++ b/x-pack/plugins/actions/server/actions_config.test.ts @@ -27,7 +27,8 @@ const defaultActionsConfig: ActionsConfig = { responseTimeout: moment.duration(60000), cleanupFailedExecutionsTask: { enabled: true, - interval: schema.duration().validate('15s'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }, }; diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.test.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.test.ts index 24a534da6098d..3c27a38e818ef 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.test.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.test.ts @@ -17,7 +17,8 @@ describe('ensureScheduled', () => { const config: ActionsConfig['cleanupFailedExecutionsTask'] = { enabled: true, - interval: schema.duration().validate('5m'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }; diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.ts index c8118374becf6..6dc1ce44982c1 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/ensure_scheduled.ts @@ -13,14 +13,14 @@ import { TaskManagerStartContract, asInterval } from '../../../task_manager/serv export async function ensureScheduled( taskManager: TaskManagerStartContract, logger: Logger, - { interval }: ActionsConfig['cleanupFailedExecutionsTask'] + { cleanupInterval }: ActionsConfig['cleanupFailedExecutionsTask'] ) { try { await taskManager.ensureScheduled({ id: TASK_ID, taskType: TASK_TYPE, schedule: { - interval: asInterval(interval.asMilliseconds()), + interval: asInterval(cleanupInterval.asMilliseconds()), }, state: { runs: 0, diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.test.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.test.ts index 12431389fa132..3e9d74be6a2c4 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.test.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.test.ts @@ -46,7 +46,8 @@ describe('findAndCleanupTasks', () => { const config: ActionsConfig['cleanupFailedExecutionsTask'] = { enabled: true, - interval: schema.duration().validate('5m'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }; diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.ts index 81302a58f7a50..0afb82a515b7c 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/find_and_cleanup_tasks.ts @@ -22,6 +22,10 @@ export interface FindAndCleanupTasksOpts { taskManagerIndex: string; } +export interface FindAndCleanupTasksResult extends CleanupTasksResult { + remaining: number; +} + export async function findAndCleanupTasks({ logger, actionTypeRegistry, @@ -29,7 +33,7 @@ export async function findAndCleanupTasks({ config, kibanaIndex, taskManagerIndex, -}: FindAndCleanupTasksOpts): Promise { +}: FindAndCleanupTasksOpts): Promise { logger.debug('Starting cleanup of failed executions'); const [{ savedObjects, elasticsearch }, { spaces }] = await coreStartServices; const esClient = elasticsearch.client.asInternalUser; @@ -69,5 +73,8 @@ export async function findAndCleanupTasks({ logger.debug( `Finished cleanup of failed executions. [success=${cleanupResult.successCount}, failures=${cleanupResult.failureCount}]` ); - return cleanupResult; + return { + ...cleanupResult, + remaining: result.total - cleanupResult.successCount, + }; } diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/register_task_definition.test.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/register_task_definition.test.ts index 8832a24103bde..a12ab16facdcd 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/register_task_definition.test.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/register_task_definition.test.ts @@ -27,7 +27,8 @@ describe('registerTaskDefinition', () => { const config: ActionsConfig['cleanupFailedExecutionsTask'] = { enabled: true, - interval: schema.duration().validate('5m'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }; diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.test.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.test.ts index ff9dc8f90329d..d465e532b0284 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.test.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.test.ts @@ -27,7 +27,8 @@ describe('taskRunner', () => { const config: ActionsConfig['cleanupFailedExecutionsTask'] = { enabled: true, - interval: schema.duration().validate('5m'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }; @@ -60,6 +61,7 @@ describe('taskRunner', () => { success: true, successCount: 1, failureCount: 1, + remaining: 0, }); }); @@ -81,7 +83,21 @@ describe('taskRunner', () => { }); }); - it('should return latest schedule', async () => { + it('should return idle schedule when no remaining tasks to cleanup', async () => { + const runner = taskRunner(taskRunnerOpts)({ taskInstance }); + const { schedule } = await runner.run(); + expect(schedule).toEqual({ + interval: '60m', + }); + }); + + it('should return cleanup schedule when there are some remaining tasks to cleanup', async () => { + jest.requireMock('./find_and_cleanup_tasks').findAndCleanupTasks.mockResolvedValue({ + success: true, + successCount: 1, + failureCount: 1, + remaining: 1, + }); const runner = taskRunner(taskRunnerOpts)({ taskInstance }); const { schedule } = await runner.run(); expect(schedule).toEqual({ diff --git a/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.ts b/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.ts index ef1a894c9a98f..38eb672238c7f 100644 --- a/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.ts +++ b/x-pack/plugins/actions/server/cleanup_failed_executions/task_runner.ts @@ -33,7 +33,10 @@ export function taskRunner(opts: TaskRunnerOpts) { total_cleaned_up: state.total_cleaned_up + cleanupResult.successCount, }, schedule: { - interval: asInterval(opts.config.interval.asMilliseconds()), + interval: + cleanupResult.remaining > 0 + ? asInterval(opts.config.cleanupInterval.asMilliseconds()) + : asInterval(opts.config.idleInterval.asMilliseconds()), }, }; }, diff --git a/x-pack/plugins/actions/server/config.test.ts b/x-pack/plugins/actions/server/config.test.ts index 68f57233308a4..092b5d2cce587 100644 --- a/x-pack/plugins/actions/server/config.test.ts +++ b/x-pack/plugins/actions/server/config.test.ts @@ -24,8 +24,9 @@ describe('config validation', () => { "*", ], "cleanupFailedExecutionsTask": Object { + "cleanupInterval": "PT5M", "enabled": true, - "interval": "PT5M", + "idleInterval": "PT1H", "pageSize": 100, }, "enabled": true, @@ -64,8 +65,9 @@ describe('config validation', () => { "*", ], "cleanupFailedExecutionsTask": Object { + "cleanupInterval": "PT5M", "enabled": true, - "interval": "PT5M", + "idleInterval": "PT1H", "pageSize": 100, }, "enabled": true, diff --git a/x-pack/plugins/actions/server/config.ts b/x-pack/plugins/actions/server/config.ts index 11b479217339e..7225c54d57596 100644 --- a/x-pack/plugins/actions/server/config.ts +++ b/x-pack/plugins/actions/server/config.ts @@ -52,7 +52,8 @@ export const configSchema = schema.object({ responseTimeout: schema.duration({ defaultValue: '60s' }), cleanupFailedExecutionsTask: schema.object({ enabled: schema.boolean({ defaultValue: true }), - interval: schema.duration({ defaultValue: '5m' }), + cleanupInterval: schema.duration({ defaultValue: '5m' }), + idleInterval: schema.duration({ defaultValue: '1h' }), pageSize: schema.number({ defaultValue: 100 }), }), }); diff --git a/x-pack/plugins/actions/server/plugin.test.ts b/x-pack/plugins/actions/server/plugin.test.ts index d777d9bbfa5a8..9464421d5f0fb 100644 --- a/x-pack/plugins/actions/server/plugin.test.ts +++ b/x-pack/plugins/actions/server/plugin.test.ts @@ -45,7 +45,8 @@ describe('Actions Plugin', () => { responseTimeout: moment.duration(60000), cleanupFailedExecutionsTask: { enabled: true, - interval: schema.duration().validate('15s'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }, }); @@ -214,7 +215,8 @@ describe('Actions Plugin', () => { responseTimeout: moment.duration(60000), cleanupFailedExecutionsTask: { enabled: true, - interval: schema.duration().validate('15s'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }, }); @@ -286,7 +288,8 @@ describe('Actions Plugin', () => { responseTimeout: moment.duration('60s'), cleanupFailedExecutionsTask: { enabled: true, - interval: schema.duration().validate('15s'), + cleanupInterval: schema.duration().validate('5m'), + idleInterval: schema.duration().validate('1h'), pageSize: 100, }, ...overrides,