Skip to content

Commit

Permalink
Update and reenable periodic task vacuum (#3354)
Browse files Browse the repository at this point in the history
We should enable the task vaccum process before flutter/flutter#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 flutter/flutter#121989.
2) enables as a daily cronjob running 12:00 PST
  • Loading branch information
keyonghan authored Dec 21, 2023
1 parent 16cf1a0 commit 74d347b
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,32 +53,16 @@ class VacuumStaleTasks extends RequestHandler<Body> {
final DatastoreService datastore = datastoreProvider(config.db);

final List<FullTask> tasks = await datastore.queryRecentTasks(slug: slug).toList();
final Set<Task> tasksToBeReset = <Task>{};
final List<Task> tasksToBeReset = <Task>[];
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<Task> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -41,53 +33,38 @@ void main() {
});

test('skips when no tasks are stale', () async {
final List<Task> expectedTasks = <Task>[
final List<Task> originalTasks = <Task>[
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<Task> tasks = config.db.values.values.whereType<Task>().toList();
expect(tasks, expectedTasks);
expect(tasks[0].status, Task.statusInProgress);
});

test('resets stale task', () async {
final List<Task> originalTasks = <Task>[
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,
),
];
Expand All @@ -97,10 +74,10 @@ void main() {
await tester.get(handler);

final List<Task> tasks = config.db.values.values.whereType<Task>().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);
});
});
}
7 changes: 3 additions & 4 deletions cron.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 74d347b

Please sign in to comment.