From 74d347b24f5be3a8e83d9a7e4090f9ce82c57e17 Mon Sep 17 00:00:00 2001 From: keyonghan <54558023+keyonghan@users.noreply.github.com> Date: Thu, 21 Dec 2023 11:36:17 -0800 Subject: [PATCH] Update and reenable periodic task vacuum (#3354) We should enable the task vaccum process before https://github.com/flutter/flutter/issues/122117 is fixed, especially when there is limited gardener support. This PR: 1) updates the vacuum logic to be based on `task.status` is `in progress` and `builder` is null, instead of being based on `created time` which caused https://github.com/flutter/flutter/issues/121989. 2) enables as a daily cronjob running 12:00 PST --- .../scheduler/vacuum_stale_tasks.dart | 28 +++----------- .../scheduler/vacuum_stale_tasks_test.dart | 37 ++++--------------- cron.yaml | 7 ++-- 3 files changed, 16 insertions(+), 56 deletions(-) diff --git a/app_dart/lib/src/request_handlers/scheduler/vacuum_stale_tasks.dart b/app_dart/lib/src/request_handlers/scheduler/vacuum_stale_tasks.dart index 852c526b7..8a8e066c8 100644 --- a/app_dart/lib/src/request_handlers/scheduler/vacuum_stale_tasks.dart +++ b/app_dart/lib/src/request_handlers/scheduler/vacuum_stale_tasks.dart @@ -53,32 +53,16 @@ class VacuumStaleTasks extends RequestHandler { final DatastoreService datastore = datastoreProvider(config.db); final List tasks = await datastore.queryRecentTasks(slug: slug).toList(); - final Set tasksToBeReset = {}; + final List tasksToBeReset = []; for (FullTask fullTask in tasks) { final Task task = fullTask.task; - if (task.status != Task.statusInProgress) { - continue; - } - - if (task.createTimestamp == null) { - log.fine('Vacuuming $task due to createTimestamp being null'); + if (task.status == Task.statusInProgress && task.buildNumber == null) { + task.status = Task.statusNew; + task.createTimestamp = 0; tasksToBeReset.add(task); - continue; - } - - final DateTime now = nowValue ?? DateTime.now(); - final DateTime create = DateTime.fromMillisecondsSinceEpoch(task.createTimestamp!); - final Duration queueTime = now.difference(create); - - if (queueTime > kTimeoutLimit) { - log.fine('Vacuuming $task due to staleness'); - tasksToBeReset.add(task); - continue; } } - - final Iterable inserts = - tasksToBeReset.map((Task task) => task..status = Task.statusNew).map((Task task) => task..createTimestamp = 0); - await datastore.insert(inserts.toList()); + log.info('Vacuuming stale tasks: $tasksToBeReset'); + await datastore.insert(tasksToBeReset); } } diff --git a/app_dart/test/request_handlers/scheduler/vacuum_stale_tasks_test.dart b/app_dart/test/request_handlers/scheduler/vacuum_stale_tasks_test.dart index 3135b7961..93f40fb3b 100644 --- a/app_dart/test/request_handlers/scheduler/vacuum_stale_tasks_test.dart +++ b/app_dart/test/request_handlers/scheduler/vacuum_stale_tasks_test.dart @@ -20,14 +20,6 @@ void main() { late VacuumStaleTasks handler; final Commit commit = generateCommit(1); - final DateTime now = DateTime(2023, 2, 9, 13, 37); - - /// Helper function for returning test times relative to [now]. - DateTime relativeToNow(int minutes) { - final Duration duration = Duration(minutes: minutes); - - return now.subtract(duration); - } setUp(() { config = FakeConfig(); @@ -41,32 +33,20 @@ void main() { }); test('skips when no tasks are stale', () async { - final List expectedTasks = [ + final List originalTasks = [ generateTask( 1, status: Task.statusInProgress, - created: relativeToNow(1), - parent: commit, - ), - generateTask( - 2, - status: Task.statusSucceeded, - created: relativeToNow(VacuumStaleTasks.kTimeoutLimit.inMinutes + 5), - parent: commit, - ), - generateTask( - 3, - status: Task.statusInProgress, - created: relativeToNow(VacuumStaleTasks.kTimeoutLimit.inMinutes), parent: commit, + buildNumber: 123, ), ]; - await config.db.commit(inserts: expectedTasks); + await config.db.commit(inserts: originalTasks); await tester.get(handler); final List tasks = config.db.values.values.whereType().toList(); - expect(tasks, expectedTasks); + expect(tasks[0].status, Task.statusInProgress); }); test('resets stale task', () async { @@ -74,20 +54,17 @@ void main() { generateTask( 1, status: Task.statusInProgress, - created: relativeToNow(1), parent: commit, ), generateTask( 2, status: Task.statusSucceeded, - created: relativeToNow(VacuumStaleTasks.kTimeoutLimit.inMinutes + 5), parent: commit, ), // Task 3 should be vacuumed generateTask( 3, status: Task.statusInProgress, - created: relativeToNow(VacuumStaleTasks.kTimeoutLimit.inMinutes + 1), parent: commit, ), ]; @@ -97,10 +74,10 @@ void main() { await tester.get(handler); final List tasks = config.db.values.values.whereType().toList(); - expect(tasks[0], originalTasks[0]); - expect(tasks[1], originalTasks[1]); - expect(tasks[2].status, Task.statusNew); + expect(tasks[0].createTimestamp, 0); + expect(tasks[0].status, Task.statusNew); expect(tasks[2].createTimestamp, 0); + expect(tasks[2].status, Task.statusNew); }); }); } diff --git a/cron.yaml b/cron.yaml index 43e261814..bb497e647 100644 --- a/cron.yaml +++ b/cron.yaml @@ -7,12 +7,11 @@ cron: url: /api/vacuum-github-commits schedule: every 6 hours -# Disabled for https://github.com/flutter/flutter/issues/121989 # TODO(keyonghan): will delete if `In Progress` hanging issue is resolved: # https://github.com/flutter/flutter/issues/120395#issuecomment-1444810718 -#- description: vacuum stale tasks -# url: /api/scheduler/vacuum-stale-tasks -# schedule: every 10 minutes +- description: vacuum stale tasks + url: /api/scheduler/vacuum-stale-tasks + schedule: every 12 hours - description: backfills builds url: /api/scheduler/batch-backfiller