Closed Bug 1364228 Opened 3 years ago Closed 3 years ago

Increase Marionette startup timeout, at least for linux mochitests

Categories

(Testing :: Marionette, enhancement)

enhancement
Not set

Tracking

(firefox-esr52 fixed, firefox54 fixed, firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

Details

Attachments

(1 file, 1 obsolete file)

As discussed in bug 1261598, it appears that firefox intermittently requires more than 120 seconds to start up, at least in our aws linux32-debug environment.

Increasing the wait to 180 seconds seems to eliminate the failures in that bug, at least in try pushes.
Attachment #8866983 - Flags: review?(hskupin)
Comment on attachment 8866983 [details] [diff] [review]
increase marionette startup timeout to 180 seconds

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

I'm against changing the default value of the harness here. Given that this is only an issue for our poor performance Linux32 workers. 

Instead we should update the TaskCluster configuration to pass in this higher timeout value to the Marionette job. Preferred only for Linux 32. Otherwise I feel we could miss some potential regressions in the startup time.

Btw. the startup time was initially increased from 60, because it is also used for restarts and Firefox has a shutdown timeout of 60s before non-stopping threads are getting killed.
Attachment #8866983 - Flags: review?(hskupin) → review-
(In reply to Henrik Skupin (:whimboo) from comment #2)
> I'm against changing the default value of the harness here. Given that this
> is only an issue for our poor performance Linux32 workers. 

> Instead we should update the TaskCluster configuration to pass in this 
> higher timeout value to the Marionette job. Preferred only for Linux 32. 
> Otherwise I feel we could miss some potential regressions in the startup time.

I checked that again and found typical mochitest startup times of about 10 seconds or less on Windows and OSX and 30 seconds or less on linux64. I agree, only linux32 (debug especially, but also opt) is typically 60+ seconds.

On the other hand, I think there's no real upper limit or even a target for **browser startup time in a unit test environment**. Even if Windows opt startup time suddenly jumps to 120+ seconds for, say, a mochitest run, I predict we'll still shrug it off, blaming the test hardware, or the unnatural browser configuration used for mochitests (or reftests, etc) -- and that might be the right call. Startup performance regressions are better tracked and responded to in the context of specific performance tests like Talos.

I'll have a look at the TC configuration alternative anyway...
This adds a --marionette-startup-timeout option for mochitests and reftests and uses that for linux mochitests. 

We normally use the same test options for all linux mochitests. I don't see a convenient way to distinguish linux32/linux64 and I'm comfortable using the higher timeout for linux64 mochitests, even though it doesn't seem to be strictly necessary on linux64.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=36de5b84000aab5c6bf85d0bb7970c170cfa8c19 has this change + logging to report the startup timeout (search on "ZZZ"). It verifies that linux mochitests use 180 seconds while other platforms (like Windows) and other tests (like reftests) continue to use 120 seconds.

My first attempt at this change was not effective: I found I needed to adjust timeout handling in start_session() to incorporate the specified startup_timeout -- easy to fix, but an unexpected complication.
Attachment #8866983 - Attachment is obsolete: true
Attachment #8867782 - Flags: review?(hskupin)
Summary: Increase Marionette DEFAULT_STARTUP_TIMEOUT → Increase Marionette startup timeout, at least for linux mochitests
Comment on attachment 8867782 [details] [diff] [review]
increase marionette startup timeout to 180 seconds, for linux mochitests

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

I'm currently away for the Rust training and won't have the time to check this until Thursday. Andreas, could you have a look instead? I'm not sure if that is the right approach, given that I see that other harnesses do not call ´raise_for_port()´ after creating the Marionette driver instance. They directly try to start the session. Is that what we want?
Attachment #8867782 - Flags: review?(hskupin) → review?(ato)
Comment on attachment 8867782 [details] [diff] [review]
increase marionette startup timeout to 180 seconds, for linux mochitests

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

Looks in line with older code to modify the socket timeout.
Attachment #8867782 - Flags: review?(ato) → review+
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/75ec8cde6e20
Increase marionette startup timeout for Linux mochitests; r=ato
https://hg.mozilla.org/mozilla-central/rev/75ec8cde6e20
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Whiteboard: [checkin-needed-beta][checkin-needed-esr52]
https://hg.mozilla.org/releases/mozilla-beta/rev/63e5df872250
Whiteboard: [checkin-needed-beta][checkin-needed-esr52] → [checkin-needed-esr52]
You need to log in before you can comment on or make changes to this bug.