[scheduler] Migrate rerun logic to Cocoon scheduler#1610
[scheduler] Migrate rerun logic to Cocoon scheduler#1610fluttergithubbot merged 12 commits intoflutter:mainfrom
Conversation
CaseyHillers
requested review from
godofredoc
and removed request for
godofredoc and
keyonghan
|
Note to reviewers: This is still WIP. I'm coming back to it now and pushed some WIP changes to my remote branch |
| final Task update = datastoreTask.task; | ||
| update.status = latestLuciTask.status; | ||
|
|
||
| final CiYaml ciYaml = await scheduler.getCiYaml(datastoreTask.commit); |
There was a problem hiding this comment.
It's expensive to load the CiYaml for every single task. Can we load it on commit level?
There was a problem hiding this comment.
scheduler.getCiYaml is cached making it trivial to pull up. Since this logic is only a for loop against existing tasks, is it ok to leave as is? It's difficult to refactor this code to be a double for loop of commit to tasks
| final Map<String, dynamic> defaultProperties = | ||
| repo == 'engine' ? Config.engineDefaultProperties : const <String, dynamic>{}; |
There was a problem hiding this comment.
Why this is removed? We need to inject this to make sure the engine rerun succeeds.
There was a problem hiding this comment.
It's in luci build service now. See
| final Target target = | ||
| ciYaml.postsubmitTargets.singleWhere((Target target) => target.value.name == datastoreTask.task.name); | ||
|
|
||
| /// Use `update.attempts - 1` as the `retries` to skip the initial run. |
There was a problem hiding this comment.
Thanks! Removed the extra logic here
| commit: datastoreTask.commit, | ||
| target: target, | ||
| task: update, | ||
| datastore: datastore, |
There was a problem hiding this comment.
With ignoreChecks default false, this rerun will never be triggered.
There was a problem hiding this comment.
ignoreChecks is reserved for the reset-prod-test use case as humans likely have a better idea if we should retry (such as needing a 4th retry). In this case, refresh chromebot will trigger tests as long as it meets the conditions in checkRerunBuilder (< 3 attempts, latest datastore does not indicate it passed).
| commit: commit, | ||
| target: target, | ||
| task: task, | ||
| priority: 29, |
There was a problem hiding this comment.
I moved this into a const variable (along with the other priorities)
|
|
||
| static const int kDefaultPriority = 30; | ||
| static const int kRerunPriority = 29; | ||
| static const int kReleasePriority = 25; |
There was a problem hiding this comment.
We agreed to use same priority for release builds?
There was a problem hiding this comment.
This is for their presubmits. When we start rolling out the cocoon scheduler on release branches, we'll need to revisit this. I filed flutter/flutter#99876 to follow up
There was a problem hiding this comment.
kReleasePriority is only referenced in post submit function _createPostsubmitScheduleBuild below, but is not injected to ScheduleBuildRequest. I am a bit confused about how this is related to the pre-submit schedule and which logic in this PR is expected to use it.
There was a problem hiding this comment.
Ah, you're right. I removed this logic (I'm not sure why we had it there in the first place)
CaseyHillers
added
the
waiting for tree to go green
4 participants
This is to fix reruns in cocoon, plugins, packages dashboards, and should help fix the engine issues.
flutter/flutter#92300
flutter/flutter#98674
Fixes flutter/flutter#99370