1093443 - Unhide Linux32 debug mochitest-e10s-2 when it meets visibility requirements

Status: RESOLVED FIXED

(Tree Management Graveyard :: Visibility Requests, defect)

Description

[Phil Ringnalda (:philor)]

Hint: it doesn't, not by miles.

A try push which was trying to look green, and had to retrigger linux32 debug m-e10s-2 a dozen times, started me noticing, and then an accidental click on a green run, which had a timeout in it, made me notice it even more.

https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-central&fromchange=0b81c10a9074&tochange=a458860efad7&searchQuery=Ubuntu%20VM%2012.04%20mozilla-central%20debug%20test%20mochitest-e10s-2 is 10 retriggers across each of several pushes. Notice in particular the fourth one down, where five of the ten runs were orange, but all five of the green runs also had timeouts that the harness failed to report.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Correct. "Enable in production and forget about it" isn't an acceptable option. The job has been hidden. Leaving this bug open for un-hiding it when it can be made to meet visibility standards.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This seems really extreme. Please give me more warning if you're going to do this. I was never CCed on any of these bugs. Bug 886301 is responsible for the majority of the failures. Why don't we just disable that?

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy is the policy. If you want to disable your way to victory, feel free. We can reconsider at that point.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I've disabled the test in bug 886301. Ryan, please re-enable the suite or tell me what additional tests need to be disabled. Regardless of the policy, it's totally unacceptable to disable an entire suite of tests with no prior warning. We've discussed this many times on dev-platform.

Comment 5

[Ed Morley [:emorley]]

(In reply to Bill McCloskey (:billm) from comment #4)
> I've disabled the test in bug 886301. Ryan, please re-enable the suite or
> tell me what additional tests need to be disabled. Regardless of the policy,
> it's totally unacceptable to disable an entire suite of tests with no prior
> warning. We've discussed this many times on dev-platform.

Ryan has not disabled the suite. That requires releng changes & is not the point of the policy.

Instead (as explained at the top of the policy, in the policy name itself & the summary of this bug), we're discussing the visibility of the job (as in still running, but whether visible in the default view or not). As such, your job is still running & making it hidden does not require advanced warning if it is not adhering to the policy, particularly for new job types.

To view jobs that are hidden click the eye icon slightly to the left of the quick filter field in the treeherder UI.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Regardless, the danger of hiding or disabling the job is that it will regress further. The worst would be if we started leaking memory during this job, in which case it could take a week or more of debugging to fix the problem. I realize that the sheriffing can be pretty frustrating and unrewarding, but debugging shutdown leaks is truly horrible.

Comment 7

[Ed Morley [:emorley]]

The job is still running and the results for each time it runs viewable as normal (once the hidden jobs button pressed). As such, any further regressions can be as easily associated with a checkin as if the job were visible in the default view, so this makes no difference to the ease of regression range finding.

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Ed Morley [:edmorley] from comment #7)
> The job is still running and the results for each time it runs viewable as
> normal (once the hidden jobs button pressed). As such, any further
> regressions can be as easily associated with a checkin as if the job were
> visible in the default view, so this makes no difference to the ease of
> regression range finding.

That just makes it harder for other devs because their code can be backed out because of tests that they don't even see. Often they'll argue against a backout because their feature is really important and then you're stuck debugging the problem anyway.

Comment 9

[Ed Morley [:emorley]]

(In reply to Bill McCloskey (:billm) from comment #8)
> That just makes it harder for other devs because their code can be backed
> out because of tests that they don't even see. Often they'll argue against a
> backout because their feature is really important and then you're stuck
> debugging the problem anyway.

This case isn't ideal (though it should be rare, given only one chunk of mochitest e10s on one platform has been hidden), however it doesn't really differ from backing someone out over breakage caused in nightly that didn't cause any tests to fail. If you receive pushback for these, the sheriffs and/or Blassey I'm sure will be happy to back you up, given the importance of e10s :-)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

(In reply to Bill McCloskey (:billm) from comment #4)
> I've disabled the test in bug 886301. Ryan, please re-enable the suite or
> tell me what additional tests need to be disabled.

AFAICT, bug 1091322 is the primary remaining issue (hitting ~1/5 runs). I've pinged Milan in the bug, but maybe you can help find an owner too. Combined with bug 979627, the suite is still failing overall ~1/3 of the time. You can discuss with the media team about disabling the webrtc tests on e10s as well.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

We worked around bug 1091322 by disabling the TV mochitests on e10s. I'm going to be disabling test_ignoreuserfocus.html on e10s shortly. Beyond that, things looks reasonably green now. Unhidden.