Closed Bug 1363714 Opened 4 years ago Closed 4 years ago

Create a load balancing mechanism to slowly migrate from BB to TC

Categories

(Firefox Build System :: Task Configuration, task, P1)

All
Windows
task

Tracking

(Not tracked)

RESOLVED FIXED
mozilla55

People

(Reporter: wcosta, Assigned: wcosta)

References

Details

Attachments

(1 file)

The idea here is the decision task read an extern stored json file that tells which percentage of test task should go to taskcluster. The set of tasks must be deterministic, i.e., two runs of decision task with the same set of test tasks should generate the exact task set for Taskcluster.
Comment on attachment 8866325 [details]
Bug 1363714: Split tests tasks between TC and BB.

https://reviewboard.mozilla.org/r/137954/#review141092

This is really cool :)

::: taskcluster/taskgraph/transforms/tests.py
(Diff revision 1)
> -                test['worker-implementation'] = 'generic-worker'
> +            test['worker-implementation'] = 'generic-worker'
> -            # see if '-w' appears in try syntax
> -            elif config.config['args'].taskcluster_worker:
> -                test['worker-implementation'] = 'native-engine'
> -            else:
> -                test['worker-implementation'] = 'buildbot-bridge'

Any chance we could leave this in place?  I think it will still be handy for platforms where 0% of the load has been shifted away from BBB.

::: taskcluster/taskgraph/transforms/tests.py:676
(Diff revision 1)
>  
> +    j = load_json_file()
> +
> +    tests_set = defaultdict(list)
>      for test in tests:
> +        tests_set[test['test-platform']].append(test)

Oo, this is a really clever use of the way transforms are defined -- I like it!

::: taskcluster/taskgraph/transforms/tests.py:681
(Diff revision 1)
> +        tests_set[test['test-platform']].append(test)
> +
> +    # Make the load balancing between taskcluster and buildbot
> +    for test_platform, t in tests_set.iteritems():
> +        # We sort the list to make the order of the tasks deterministic
> +        t.sort()

I think you'll want to key this by name or label or something?  Sorting dictionaries directly isn't particularly stable, I think.

::: taskcluster/taskgraph/transforms/tests.py:692
(Diff revision 1)
> +        n = j.get(test_platform, 1.0)
> +        for i in range(int(n * len(t)), len(t)):
> +            t[i]['worker-implementation'] = 'buildbot-bridge'
> +
> +    for ts in tests_set.itervalues():
> +        for test in ts:

It would be better to just `yield test` here, and put all of the above in a new transform (`allocate_to_bbb`?).  Then the `make_job_description` transform would be unchanged by this patch.

::: taskcluster/taskgraph/transforms/tests.py:789
(Diff revision 1)
>  def get_firefox_version():
>      with open('browser/config/version.txt', 'r') as f:
>          return f.readline().strip()
> +
> +
> +def load_json_file():

Let's give this a better name - `get_tests_div`?
Attachment #8866325 - Flags: review?(dustin) → review-
Comment on attachment 8866325 [details]
Bug 1363714: Split tests tasks between TC and BB.

https://reviewboard.mozilla.org/r/137954/#review141110

::: taskcluster/taskgraph/transforms/tests.py:681
(Diff revision 1)
> +        tests_set[test['test-platform']].append(test)
> +
> +    # Make the load balancing between taskcluster and buildbot
> +    for test_platform, t in tests_set.iteritems():
> +        # We sort the list to make the order of the tasks deterministic
> +        t.sort()

I didn't get, I am sorting a list, not a dict :)
Comment on attachment 8866325 [details]
Bug 1363714: Split tests tasks between TC and BB.

https://reviewboard.mozilla.org/r/137954/#review141206

::: taskcluster/taskgraph/transforms/tests.py:682
(Diff revisions 1 - 2)
>          tests_set[test['test-platform']].append(test)
>  
>      # Make the load balancing between taskcluster and buildbot
>      for test_platform, t in tests_set.iteritems():
>          # We sort the list to make the order of the tasks deterministic
> -        t.sort()
> +        t.sort(key=lambda x: x['test-name'])

needs chunk, as discussed in irc

::: taskcluster/taskgraph/transforms/tests.py:802
(Diff revisions 1 - 2)
> -
> -def load_json_file():
> +def get_load_balacing_settings():
> +    url = "https://s3.amazonaws.com/taskcluster-graph-scheduling/tests-load.json"
> -    url = "https://s3.amazonaws.com/taskcluster-graph-scheduling/tests-div.json"
>      try:
>          return requests.get(url).json()
>      except:

`except Exception:`

(otherwise you'll catch ctrl-c)
Attachment #8866325 - Flags: review?(dustin) → review-
Blocks: 1363832
Comment on attachment 8866325 [details]
Bug 1363714: Split tests tasks between TC and BB.

https://reviewboard.mozilla.org/r/137954/#review141258
Attachment #8866325 - Flags: review?(dustin) → review+
Pushed by wcosta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/375bf84f1825
Split tests tasks between TC and BB. r=dustin
Pushed by wcosta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d20789b15aa7
Split tests tasks between TC and BB. r=dustin
Landed fixed patch
Flags: needinfo?(wcosta)
https://hg.mozilla.org/mozilla-central/rev/d20789b15aa7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.