Closed Bug 1329783 Opened 3 years ago Closed 3 years ago

Gecko decision task should be able to determine if to schedule all jobs without needing to talk to Treeherder

Categories

(Taskcluster :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla53

People

(Reporter: armenzg, Assigned: rwood)

References

Details

Attachments

(1 file)

Background note: Currently we talk to the SETA Heroku app instead of Treeherder's SETA APIs.

The current design for Treeherder's SETA is that the Gecko decision task would talk to the SETA Treeherder APIs. Treeherder would then decide if to return a complete set of tasks or a subset depending on how many times the Gecko decision task has called the API.

We can implement this logic in-tree if the Gecko decision task took the push id and calculated the modulo with the repository's SETA push count. This would allow it to know if to schedule all jobs or if to hit TH's API for the set of low value jobs.

This will require having an in-tree value to determine for each repository if SETA applies to them and after how many pushes to schedule all jobs.

This approach also has the added value that we can change the setting in tree for each repository.
This also helps for the day that we decided to put the calculations of SETA (analyze_failures run) into an S3 bucket. This would make the gecko decision task to be free to determine if to schedule all tasks or a subset. Right now, it cannot know that unless TH says so.

Remember the business logic is:
* "has it been N pushes since the last time?" OR "has it past N minutes since the last time?"
** The expiration is mainly for when there is low amount of pushes like weekends.

There's also some discussion on how to deal with the timer's expiration.

dustin said:
> I know I sound like a broken record, but .. could we do this probabalistically? 
> If random.seed(pushid).random() < 1.0/7.0, run all tasks?

jmaher said:
> While I think that would get us similar results, it doesn't give us a pattern to the tests- possibly
> we don't need a pattern to the tests.  In the case of using SETA for PGO, it would add some confusion
> as we are running tests more frequently, etc.

This proposal would also remove the need for the updating of the TaskRequest counter for each repository:
https://github.com/mozilla/treeherder/blob/master/treeherder/seta/job_priorities.py#L93-L112
https://github.com/mozilla/treeherder/blob/master/treeherder/seta/models.py#L9-L31

We can file a follow up bug to remove the old code once we get this working.
Blocks: 1326102
Should we address the following as part of this bug? Since we're modifying code around the gecko decision task? Or file a follow up?

catlee says:
I think one drawback of this approach is that not all pushes trigger builds
that will need tests. In buildbot anyway, if a push only changes files
under mobile/, then no desktop builds are produced, and therefore no
desktop tests. If this push happened to have (pushid % N) == 0, then we
would not get tests run on this push, which is expected, but not for the
right reasons. Subsequent pushes would not get the tests run either, and we
would have to wait until builds from (pushid + N) happened.

Would this problem apply also to Taskcluster? Are we implementing, or
planning to implement, the same build exclusion logic?
See the whole thread (including gps' comment) in:
https://groups.google.com/forum/#!topic/mozilla.tools/NXmJHA9TzJ0

I believe the whole topic should be dealt with separatedely from this bug.
Let's fix our current set up and have further conversations on how to get to an ideal.
Comment on attachment 8828488 [details]
Bug 1329783 - Have gecko decision task determine when to call seta;

https://reviewboard.mozilla.org/r/105870/#review106814

::: taskcluster/taskgraph/util/seta.py:81
(Diff revision 1)
>          return low_value_tasks
>  
> -    def is_low_value_task(self, label, project):
> +    def is_low_value_task(self, label, project, pushlog_id):
> +        schedule_all_every = PROJECT_SCHEDULE_ALL_EVERY.get(project, 5)
> +        # on every Nth push, want to run all tasks
> +        if int(pushlog_id) % schedule_all_every != 0:

could we instead do:

# no low value tasks if we need to schedule ALL
if int(pushlog_id) % schedule_all_every == 0:
    return False

# previous code exists
...
Attachment #8828488 - Flags: review?(jmaher) → review-
Comment on attachment 8828488 [details]
Bug 1329783 - Have gecko decision task determine when to call seta;

https://reviewboard.mozilla.org/r/105870/#review106824

one little nit

::: taskcluster/taskgraph/util/seta.py:88
(Diff revision 2)
>          # cache the low value tasks per project to avoid repeated SETA server queries
>          if project not in self.low_value_tasks:
>              self.low_value_tasks[project] = self.query_low_value_tasks(project)
>          return label in self.low_value_tasks[project]
>  
> +

nit: you are adding a blank line here, is this to pass pylint or something?
Attachment #8828488 - Flags: review?(jmaher) → review+
Blocks: 1332448
Pushed by rwood@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e8c9758e6322
Have gecko decision task determine when to call seta; r=jmaher
https://hg.mozilla.org/mozilla-central/rev/e8c9758e6322
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.