416911 - per-test timeout in talos

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[alice nodelman [:alice] [:anode]]

Currently talos uses the same 8 hour time limit for any test.  This can cause a pretty major delay in the browser freezing up and us correctly reporting the failure.  We actually pretty much have this code in place, we just need to start using it.

Before this change could be made all the currently used buildbot talos config files would have to be updated to have an appropriate timeout for each test.

Comment 1

[alice nodelman [:alice] [:anode]]

Attachment: per test timeout for talos

Comment 2

[alice nodelman [:alice] [:anode]]

Attachment: proposed timeouts for buildbot talos config files

Here are my proposed timeouts:

tjss : 30 minutes (1800 s)
twinopen : 5 minutes (300 s)
tsspider : 5 minutes (300 s)
tp : 2 1/2 hours (9000 s)
tp (historic page set) : 5 minutes (300 s)
tsvg : 15 minutes (900 s)
tdhtml : 15 minutes (900 s)
tgfx : 15 minutes (900 s)

The main worry would be if they are set too low so as to stop a successful test run part way through.  We'll also have to maintain these as more test pages get added to the various suites.

Comment 3

[alice nodelman [:alice] [:anode]]

Attachment: reworking per-test timeout

This is to replace the previous version of this patch which required the specification of timeouts for each test.  Now I'm making it optional since that will make it easier to check in without having to alter all the currently used config files.

Comment 4

[alice nodelman [:alice] [:anode]]

Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.12; previous revision: 1.11
done
Checking in sample.config;
/cvsroot/mozilla/testing/performance/talos/sample.config,v  <--  sample.config
new revision: 1.13; previous revision: 1.12
done

Comment 5

[alice nodelman [:alice] [:anode]]

Attachment: push per-test timeouts to stage machines

Comment 6

[alice nodelman [:alice] [:anode]]

Pushed to stage.

Checking in sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/sample.config,v  <--  sample.config
new revision: 1.11; previous revision: 1.10
done
Checking in sample.config.nochrome;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/sample.config.nochrome,v  <--  sample.config.nochrome
new revision: 1.5; previous revision: 1.4
done
Checking in sample.config.nogfx;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/sample.config.nogfx,v  <--  sample.config.nogfx
new revision: 1.11; previous revision: 1.10
done
Checking in fast.sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/fast.sample.config,v  <--  fast.sample.config
new revision: 1.3; previous revision: 1.2
done

Comment 7

[alice nodelman [:alice] [:anode]]

Attachment: better process killing on timeout (or other bad situations)

Adding timeouts showed up some flaws in how we kill lingering browser processes in windows.  While the browser is terminated it takes more time than on linux/mac because of the windows OS always giving a process the chance to close cleanly before killing.  This time difference between the request to kill and the actual killing leads to states where we are attempting to clean up profiles/temporary data that is still in use - which then throw exceptions.

This patch adds some smarts to terminating processes by doing checks to make sure that they are really dead before moving on in the code.  I've tested on win/linux and it works fine.  We may want to do some mac tests before we fully push the code in case it breaks things there.

Comment 8

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 310659 [details] [diff] [review]
better process killing on timeout (or other bad situations)

one more round with this, eh?

Looks reasonable.

Comment 9

[alice nodelman [:alice] [:anode]]

Better process killing on timeout:

Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.13; previous revision: 1.12
done
Checking in ffprocess_linux.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_linux.py,v  <--  ffprocess_linux.py
new revision: 1.7; previous revision: 1.6
done
Checking in ffprocess_mac.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_mac.py,v  <--  ffprocess_mac.py
new revision: 1.8; previous revision: 1.7
done
Checking in ffprocess_win32.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_win32.py,v  <--  ffprocess_win32.py
new revision: 1.5; previous revision: 1.4
done

Comment 10

[alice nodelman [:alice] [:anode]]

Attachment: timeout based upon browser activity [Checked in]

Giving this another try.  The issue I ran into with the other patches was that a timeout per test could too easily go wrong - it would kill tests that were running but running slowly.  This new approach has the browser send a ping back to talos after loading each page.  If talos doesn't hear from the browser in a certain amount of time it can presume that the browser is frozen and kill it.  This will also give us extra data as to what page the browser is stuck on.

I have this up and running on talos stage and you can see output files on the MozillaTest waterfall.

Comment 11

[alice nodelman [:alice] [:anode]]

Attachment: new configs for talos production/stage/try for new timeout structure [Checked in]

Comment 12

[alice nodelman [:alice] [:anode]]

Attachment: more information from the pageloader [Checked in]

The pageloader's noisy output currently only includes the page that it just loaded and not the name of the page that it is about to load.  If we get frozen on a page we are more interested in what the browser is trying to do as opposed to what it just did.

Comment 13

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 327178 [details] [diff] [review]
timeout based upon browser activity [Checked in]

ah, this old chestnut.

Comment 14

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 327180 [details] [diff] [review]
more information from the pageloader [Checked in]

>+    dumpLine("Cycle " + (cycle+1) + ": loaded " + pageName + ' (next: ' + nextName + ')');

mixing quote styles is somewhat confusing here, but acceptable.

Comment 15

[alice nodelman [:alice] [:anode]]

Comment on attachment 327178 [details] [diff] [review]
timeout based upon browser activity [Checked in]

Checking in ffprocess_win32.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_win32.py,v  <--  ffprocess_win32.py
new revision: 1.7; previous revision: 1.6
done
Checking in sample.config;
/cvsroot/mozilla/testing/performance/talos/sample.config,v  <--  sample.config
new revision: 1.16; previous revision: 1.15
done
Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.15; previous revision: 1.14
done
Checking in utils.py;
/cvsroot/mozilla/testing/performance/talos/utils.py,v  <--  utils.py
new revision: 1.6; previous revision: 1.5
done

Comment 16

[alice nodelman [:alice] [:anode]]

Comment on attachment 327179 [details] [diff] [review]
new configs for talos production/stage/try for new timeout structure [Checked in]

Checking in perf-staging/configs/sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perf-staging/configs/sample.config,v  <--  sample.config
new revision: 1.4; previous revision: 1.3
done
Checking in perfmaster2/configs/fast.production.sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/fast.production.sample.config,v  <--  fast.production.sample.config
new revision: 1.5; previous revision: 1.4
done
Checking in perfmaster2/configs/jss.production.sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/jss.production.sample.config,v  <--  jss.production.sample.config
new revision: 1.4; previous revision: 1.3
done
Checking in perfmaster2/configs/production.sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/production.sample.config,v  <--  production.sample.config
new revision: 1.7; previous revision: 1.6
done
Checking in perfmaster2/configs/production.sample.config.nochrome;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/production.sample.config.nochrome,v  <--  production.sample.config.nochrome
new revision: 1.4; previous revision: 1.3
done
Checking in perfmaster2/configs/production.sample.config.nogfx;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/perfmaster/configs/production.sample.config.nogfx,v  <--  production.sample.config.nogfx
new revision: 1.7; previous revision: 1.6
done
Checking in tryperfmaster/configs/sample.config;
/cvsroot/mozilla/tools/buildbot-configs/testing/talos/tryperfmaster/configs/sample.config,v  <--  sample.config
new revision: 1.4; previous revision: 1.3
done

Comment 17

[alice nodelman [:alice] [:anode]]

Comment on attachment 327180 [details] [diff] [review]
more information from the pageloader [Checked in]

Checking in pageloader.js;
/cvsroot/mozilla/layout/tools/pageloader/pageloader.js,v  <--  pageloader.js
new revision: 1.14; previous revision: 1.13
done

Comment 18

[alice nodelman [:alice] [:anode]]

I've checked in bustage fixes for all the config files - I had the pageloader options malformed

"-tpformat -tpnoisy tinderbox" should be "-tpnoisy -tpformat tinderbox"

Comment 19

[alice nodelman [:alice] [:anode]]

Attachment: minor cleanup of debug/noisy statements [Checked in]

Comment 20

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 328080 [details] [diff] [review]
minor cleanup of debug/noisy statements [Checked in]

are you getting '\r' because of Windows or is something putting them in there explicitly?

Comment 21

[alice nodelman [:alice] [:anode]]

Comment on attachment 328080 [details] [diff] [review]
minor cleanup of debug/noisy statements [Checked in]

Checking in utils.py;
/cvsroot/mozilla/testing/performance/talos/utils.py,v  <--  utils.py
new revision: 1.7; previous revision: 1.6
done

Comment 22

[alice nodelman [:alice] [:anode]]

To Comment #20 - there aren't any explicit \r's anyway, but they are appearing in the windows output.