1183877 - Increase total-chunks for Android 4.3 Debug crashtests, js-reftests, and reftests

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Geoff Brown [:gbrown]]

See bug 1140471 and particularly https://bugzilla.mozilla.org/show_bug.cgi?id=1140471#c14:

"We think there is value in running Debug reftests on Android and we think the run-times are not surprising in context (we expect debug to run 2x to 3x slower, Android tests are slow, the emulator is slow and limits our ability to use multi-core cpu effectively).

"Let's try running these with as many chunks as necessary."

Comment 1

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0cadc2b2dbc7&exclusion_profile=false hacks an increase in total-chunks as follows:

4 crashtest chunks
20 js-reftest chunks
48 plain-reftest chunks

Because of the nature of the hack, only the first few jobs of each category are run, but these results suggest that these total-chunk options are approximately correct.

Notice run-times from try:

C1 50 minutes
C2 55
J1 59
J2 50
J3 44
J4 53
J5 39
J6 55
R1 50
R2 47
R3 53
R4 67
R5 44
R6 35
...

Comment 2

[Kim Moir [:kmoir] ET]

Trying to think of a non-ugly hack to implement this.  It's a bit different in that we will have more chunks on debug that on opt. And I adding 48 test suites for debug plain reftests in our configs is not pretty.

Comment 3

[Kim Moir [:kmoir] ET]

Have some patches in progress for this and the the total # of builders doesn't seem to be a problem anymore :-) thanks to to jgriffin

gbrown: I assume you want these new tests to ride the trains?

Comment 4

[Geoff Brown [:gbrown]]

That's great! Yes, please ride the trains.

Comment 5

[Kim Moir [:kmoir] ET]

Attachment: bug1183877.patch

buildbot configs patch

Comment 6

[Kim Moir [:kmoir] ET]

Attachment: bug1183877builder.diff

builder diff

Comment 7

[Kim Moir [:kmoir] ET]

So I'm still working on this. It was quite a monumental task to get this working in staging but I think I have it working now and am waiting for test results.

Comment 8

[Kim Moir [:kmoir] ET]

Attachment: bug1183877m-c.patch

Comment 9

[Kim Moir [:kmoir] ET]

Attachment: bug1183877m-c.patch

mozilla-central patch for mozharness

Comment 10

[Kim Moir [:kmoir] ET]

Attachment: bug1183877.patch

buildbot configs

Comment 11

[Jordan Lund (:jlund)]

I'm sorry kim I did not get to this today. I will look at it tomorrow

Comment 12

[Jordan Lund (:jlund)]

Comment on attachment 8643199 [details] [diff] [review]
bug1183877.patch

Review of attachment 8643199 [details] [diff] [review]:
-----------------------------------------------------------------

looks great! just one question below regarding try/cedar.

reseting r? for now. please r? me again when you have responded or updated the patch.

::: mozilla-tests/mobile_config.py
@@ +3002,5 @@
> +        'blob_upload': True,
> +        'timeout': 2400,
> +        'script_maxtime': 14400,
> +    },
> +    ),   

whitespace

@@ +3413,5 @@
>              BRANCHES[name]['platforms']['android-api-11']['panda_android']['debug_unittest_suites'] = deepcopy(ANDROID_MOZHARNESS_JSREFTEST + ANDROID_MOZHARNESS_CRASHTEST + ANDROID_MOZHARNESS_PLAIN_REFTEST)
>  
> +# bug 118387 Increase total-chunks for Android 4.3 Debug crashtests, js-reftests, and reftests
> +for name, branch in items_at_least(BRANCHES, 'gecko_version', 42):
> +    if name in ('try', 'cedar' ):

It seems like try and cedar have similar builder lists to other trunk branches wrt api 11+ debug. How come you are not changing these too?

Comment 13

[Kim Moir [:kmoir] ET]

Attachment: bug1183877-2.patch

Comment 14

[Kim Moir [:kmoir] ET]

Attachment: bug1183877builder-2.diff

Comment 15

[Kim Moir [:kmoir] ET]

Attachment: bug1183877-3.patch

Comment 16

[Kim Moir [:kmoir] ET]

Attachment: bug1183877builder-3.diff

Comment 17

[Kim Moir [:kmoir] ET]

Comment on attachment 8644427 [details] [diff] [review]
bug1183877-3.patch

change it so new debug builders run on cedar and try too.  As well, leave 4.0 jobs running on try so they can still be run on try only they ride off the trains

Comment 18

[Jordan Lund (:jlund)]

Comment on attachment 8644427 [details] [diff] [review]
bug1183877-3.patch

Review of attachment 8644427 [details] [diff] [review]:
-----------------------------------------------------------------

thumbs!

fyi - I didn't put this second patch through dump master but logic wise it looks great

::: mozilla-tests/mobile_config.py
@@ +3412,5 @@
>              }
>              BRANCHES[name]['platforms']['android-api-11']['panda_android']['debug_unittest_suites'] = deepcopy(ANDROID_MOZHARNESS_JSREFTEST + ANDROID_MOZHARNESS_CRASHTEST + ANDROID_MOZHARNESS_PLAIN_REFTEST)
>  
> +# bug 1183877 Increase total-chunks for Android 4.3 Debug crashtests, js-reftests, and reftests
> +for name, branch in items_at_least(BRANCHES, 'gecko_version', 42):    

nit: whitespace

@@ +3428,5 @@
> +                'opt_unittest_suites': deepcopy(ANDROID_4_3_C3_DICT['opt_unittest_suites']),
> +                'debug_unittest_suites': deepcopy(ANDROID_4_3_C3_TRUNK_DICT['debug_unittest_suites'] + ANDROID_4_3_MOZHARNESS_DEBUG_TRUNK),}
> +            if name in ('try'):
> +                continue
> +            BRANCHES[name]['platforms']['android-api-11']['panda_android']['debug_unittest_suites'] = []

I wonder if this will bite us later down the line. e.g. if we accidentally added a line below this and didn't see the try continue block above.

maybe we should say something like:

if name != 'try':
     BRANCHES[name]['platforms']['android-api-11']['panda_android']['debug_unittest_suites'] = []

Comment 19

[Kim Moir [:kmoir] ET]

Comment on attachment 8643180 [details] [diff] [review]
bug1183877m-c.patch

pushed to m-i

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fae9a7b76008

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fae9a7b76008

Comment 22

[Kim Moir [:kmoir] ET]

buildbot patch hasn't landed yet so reopening

Comment 23

[Kim Moir [:kmoir] ET]

Attachment: bug1183877-4.patch

patch with review comments incorporated
also changed gecko version to 43 since merge has occurred

Comment 24

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/88afda24ee11

Comment 25

[Kim Moir [:kmoir] ET]

Attachment: 1183877bustage.patch

Comment 26

[Pulsebot]

https://hg.mozilla.org/mozilla-central/rev/9a6b1cc80245

Comment 27

[Kim Moir [:kmoir] ET]

Comment on attachment 8646983 [details] [diff] [review]
1183877bustage.patch

earlier today

Comment 28

[Kim Moir [:kmoir] ET]

Attachment: bug1183877chunking.patch

I had this patch om my staging master but missed attaching it when it came to review time. Thus the cause of the (non-capacity) failures today.

Comment 29

[Jordan Lund (:jlund)]

Comment on attachment 8643180 [details] [diff] [review]
bug1183877m-c.patch

Review of attachment 8643180 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mozharness/configs/android/androidarm_4_3.py
@@ +306,5 @@
>                  "tests/layout/reftests/reftest.list"]
>          },
> +        "reftest-17": {
> +            "category": "reftest",
> +            "extra args": ["--total-chunks=48", "--this-chunk=17",

I'm kicking myself for missing this. I take half the blame.

Comment 30

[Jordan Lund (:jlund)]

Comment on attachment 8647126 [details] [diff] [review]
bug1183877chunking.patch

Review of attachment 8647126 [details] [diff] [review]:
-----------------------------------------------------------------

I can't wait to axe this complexity now that we have all of mh in tree. these in_tree bits should go away

Comment 31

[Kim Moir [:kmoir] ET]

Thanks Jordan for the reviews.  I shoulder the blame for the errors - my apologies.

So aside from the errors in my patches there are two issues from yesterday's burning trees that I need to address before enabling this again. We are adding 48 jobs on every commit.  This causes capacity issues
1) We don't have a large enough emulator64 pool to handle this
2) We don't have SETA data on these new tests to reduce the number of tests that are run on each commit.

So the next steps are that I'm going to enable these tests on a project branch to ensure the latest patches fix the issues. Then I'll talk to jmaher about how to get SETA data for these tests in a faster time frame than the usual month.

Comment 32

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5ee45f4af6b

Comment 33

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/524c6eeeeeae

Comment 34

[Kim Moir [:kmoir] ET]

Comment on attachment 8647126 [details] [diff] [review]
bug1183877chunking.patch

backed out as gbrown pointed out this will may opt tests run fewer tests

from irc
gbrown	I'm worried that 4.3 opt reftests (etc) will think total-chunks are increased but only run chunks {1..16} of 48
gbrown	no failures, but lots of tests missing
gbrown	I think we may need reftest + reftest-debug sections in the mozharness config...messy

Comment 35

[Kim Moir [:kmoir] ET]

Had a meeting wrt SETA today and decided the way forward for this was to enable the tests on try (hidden), run tests n times, then extrapolate this data to m-i etc so we can enable on other branches without crippling wait times.

https://etherpad.mozilla.org/increasing-seta

Comment 36

[Kim Moir [:kmoir] ET]

Attachment: bug1183877trunk.patch

Once we have data from SETA from bug 1205526 in place, we can enable the 4.3 debug tests on all branches and disable the corresponding 4.0 ones. Luckily, we are still under the builder limit on the linux64 masters.

Comment 37

[Kim Moir [:kmoir] ET]

Attachment: bug1183877trunkbuilder.diff

builder diff

Comment 38

[Kim Moir [:kmoir] ET]

in production

Comment 39

[Kim Moir [:kmoir] ET]

I can see jobs being skipped according to SETA data, so I'm going to close this bug.  We still have Android 4.0 debug jobs running on Pandas m-a, m-b, m-r and talos on all branches.  Talked to jmaher in #ateam and he suggested we could tackle m-a/m-b/m-r in a week or so.