414660 - Add ts test to talos that uses a dirty profile

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

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

To get a better feel for real world startup performance we should add a test to talos that uses a dirty profile.  To create this test we are mostly just missing what we would consider a 'standard' profile that we would expect an end user to have.

We also need to consider that this would have to work with both 1.8 and 1.9.  That may effect the sort of profile that could be used if significant migration work would be required.

Comment 1

[Dietrich Ayala (:dietrich)]

Alice, can we make this bug more general, to include the running of all Talos tests against N profiles?

We can then make bug 489183 dependent on the infrastructure changes resulting from this.

Comment 2

[Chris AtLee [:catlee]]

I kind of like this, except that "running of all Talos tests against N profiles" means multiplying our talos run-time by N.

We could test each profile in parallel I guess...but that's an awful lot of mac minis :)

Comment 3

[Dietrich Ayala (:dietrich)]

To clarify: I'll be deploying this on some boxes that we have that are separate from existing tinderboxes. These will be doing continuous builds, and pushing data to the graphserver, but won't report to the main tinderbox tree for mozilla-central.

Comment 4

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

After talking with dietrich what we want is:

- a machine within the build network that can generate profiles for testing, these should then be uploaded to stage.m.o
- talos boxes will periodically download new profiles from stage.m.o
- talos will know how to test using multiple profiles

What we are missing then is the automation of profile generation (along with the smarts to post to stage) and the code in talos to correctly use different profiles and post under different names depending on profile in use (ie, ts_dirty, etc).

Comment 5

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

Setting up profile generator on cruncher.build.m.o.  Any suggestions as to the parameters that you want me to run this with?

Comment 6

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

After irc discussion with ddahl/dietrich discovered:

- db generator creates random links/dates/information, generates a unique profile per run
- each time talos uses a newly generated db it will be like talos is running a new test
- we need a means of *only* updating the dates in a given profile db so that is is the same profile, but the dates are refreshed so that links do not age off
- this is the only way that we'll get consistent and useful talos results

Comment 7

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

Current plan:

- use the generator tool to create three dirty profiles

python builddb/generator.py -i avg

python builddb/generator.py -i min

python builddb/generator.py -i max

- update the profiles each night using

builddb/increment_dates.py

Call the tests:
places_generated_max, places_generated_med, places_generated_min

Comment 8

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

Attachment: [Checked in]add dirty profile ts tests to graph server

No data to collect just yet, but we'll be ready.

Comment 9

[Chris AtLee [:catlee]]

Comment on attachment 391244 [details] [diff] [review]
[Checked in]add dirty profile ts tests to graph server

Is this naming scheme going to work once we start adding more tests?

Comment 10

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

These were the agreed upon names provided by the places team.  Also, since they are in the graph server we can always change the pretty display name at will.

Comment 11

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

Comment on attachment 391244 [details] [diff] [review]
[Checked in]add dirty profile ts tests to graph server

changeset:   232:0c2db6202471

Comment 12

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Per phone call with Alice yesterday, there are basically 3 different "dirty profile" suites being tracked here: small, medium, large. 

The large suite is blocked, until the script in bug#498820 can handle this.

Meanwhile, we're going ahead with small and medium suites, as they are working fine.

Comment 13

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

Attachment: support dirty profile testing in talos

Mostly adding the option to use a specific profile for each individual test in a set.

Comment 14

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

Attachment: add dirty profile test to talos-staging-pool buildbot configs

Comment 15

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

Attachment: add support in buildbotcustom for dirty profile testing (mostly through additional file download)

Comment 16

[Chris AtLee [:catlee]]

Comment on attachment 393109 [details] [diff] [review]
add support in buildbotcustom for dirty profile testing (mostly through additional file download)

>diff -r 417d1b245bad process/factory.py
>--- a/process/factory.py	Thu Aug 06 15:58:28 2009 -0400
>+++ b/process/factory.py	Thu Aug 06 19:21:20 2009 -0700
>@@ -3894,6 +3894,7 @@
>     def __init__(self, OS, toolsDir, envName, buildBranch, branchName,
>             configOptions, talosCmd, customManifest='', customTalos=None,
>             workdirBase=None, fetchSymbols=False, plugins='zips/plugins.zip', pageset='zips/pagesets.zip',
>+            talosAddOn='',

I'd prefer talosAddOn defaults to None.

>+        if talosAddOn != '':

And this can be changed to 'if talosAddOn:' or 'if talosAddOn is not None:'

r+ with those changes.

Comment 17

[Chris AtLee [:catlee]]

Comment on attachment 393108 [details] [diff] [review]
add dirty profile test to talos-staging-pool buildbot configs

>diff -r 90ae7eeb69fb talos-staging-pool/config.py
>--- a/talos-staging-pool/config.py	Tue Jul 28 12:26:50 2009 -0400
>+++ b/talos-staging-pool/config.py	Thu Aug 06 19:11:42 2009 -0700
>@@ -7,6 +7,8 @@
> 
> TALOS_JSS_CONFIG_OPTIONS = GRAPH_CONFIG + ['--activeTests', 'tjss']
> 
>+TALOS_DIRTY_CONFIG_OPTIONS = GRAPH_CONFIG + ['--activeTests', 'ts:ts_places_generated_min:ts_places_generated_med']
>+
> TALOS_CMD = ['python', 'run_tests.py', '--noisy', WithProperties('%(configFile)s')]
> 
> SLAVES = {
>@@ -50,10 +52,11 @@
> BRANCHES['mozilla-1.9.0']['build_branch'] = "1.9.0"
> BRANCHES['mozilla-1.9.0']['fetch_symbols'] = False
> # How many chrome tests per build to run, and whether to merge build requests
>-BRANCHES['mozilla-1.9.0']['chrome_tests'] = (1,True)
>+BRANCHES['mozilla-1.9.0']['chrome_tests'] = (0,True)
> # How many nochrome tests per build to run, and whether to merge build requests
>-BRANCHES['mozilla-1.9.0']['nochrome_tests'] = (1,True)
>+BRANCHES['mozilla-1.9.0']['nochrome_tests'] = (0,True)

Why are you changing chrome_tests and nochrome_tests?

Comment 18

[Chris AtLee [:catlee]]

Comment on attachment 393107 [details] [diff] [review]
support dirty profile testing in talos

Can you re-attach this as a unified diff with more context please?

Comment 19

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

Attachment: support dirty profile testing in talos (take 2)

Sorry for previous patch, didn't double check before I submitted it that it was actually readable and correct.

Comment 20

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

Attachment: add dirty profile test to talos-staging-pool buildbot configs (take 2)

Comment 21

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

Attachment: add support in buildbotcustom for dirty profile testing (mostly through additional file download) (take 2)

Fixed according to catlee's review comments.

Comment 22

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

Attachment: add support in buildbotcustom for dirty profile testing (mostly through additional file download) (take 3)

Fixed bad var name in previous version of patch.

Comment 23

[[e-mail] (:bhearsum)]

Comment on attachment 393965 [details] [diff] [review]
support dirty profile testing in talos (take 2)

>? places_generated_max
>? places_generated_med
>? places_generated_min

Do you need to 'cvs add' these profiles?

>   #process the results
>   if (results_server != '') and (results_link != ''):
>     #send results to the graph server
>     utils.stamped_msg("Sending results", "Started")
>     links = send_to_graph(results_server, results_link, title, date, browser_config, results)
>+    print links

Is this an intended change? Seems separate to this bug, so just checking.


>Index: sample.config
>===================================================================
>RCS file: /cvsroot/mozilla/testing/performance/talos/sample.config,v
>retrieving revision 1.30
>diff -u -8 -p -r1.30 sample.config
>--- sample.config	1 Jul 2009 01:04:10 -0000	1.30
>+++ sample.config	12 Aug 2009 01:18:29 -0000
>@@ -18,18 +18,16 @@ browser_log: browser_output.txt
> extra_args: ''
> #how long the browser takes to open/close
> browser_wait: 5
> 
> branch: testbranch
> 
> buildid: testbuildid
> 
>-profile_path: base_profile
>-

It might be a bit nicer to keep this as the default and allow each test to override. Up to you.

>-def createProfile(browser_config):
>-  if browser_config["profile_path"] != {}:
>-      # Create the new profile
>-      temp_dir, profile_dir = ffsetup.CreateTempProfileDir(browser_config['profile_path'],
>-                                                 browser_config['preferences'],
>-                                                 browser_config['extensions'])
>-      utils.debug("created profile") 
>-  else:
>-      # no profile path was set in the config, set the profile_dir to an empty string.
>-      profile_dir = ""
>+def createProfile(profile_path, browser_config):
>+  # Create the new profile
>+  temp_dir, profile_dir = ffsetup.CreateTempProfileDir(profile_path,
>+                                             browser_config['preferences'],
>+                                             browser_config['extensions'])

It's sortof unintuitive to have a function called createProfile that does that, and a bit more (eg, taking part of an existing profile and filling in the gaps). Any chance you can add a comment or docstring that explains it a bit while you're touching this area?

I'm not a Talos developer (anymore) so it's hard for me to say what's best with this code...so if you disagree with any of the above I won't be offended if you overrule me :). r=bhearsum if you want it just like this, I'm happy to re-review if you change it.

Comment 24

[[e-mail] (:bhearsum)]

Comment on attachment 393969 [details] [diff] [review]
add dirty profile test to talos-staging-pool buildbot configs (take 2)

>diff -r cfb529beab4a talos-staging-pool/config.py
>--- a/talos-staging-pool/config.py	Tue Aug 11 13:22:10 2009 -0400
>+++ b/talos-staging-pool/config.py	Tue Aug 11 19:04:12 2009 -0700
>@@ -7,6 +7,8 @@
> 
> TALOS_JSS_CONFIG_OPTIONS = GRAPH_CONFIG + ['--activeTests', 'tjss']
> 
>+TALOS_DIRTY_CONFIG_OPTIONS = GRAPH_CONFIG + ['--activeTests', 'ts:ts_places_generated_min:ts_places_generated_med']
>+

Do you want to run _max too?

>+            if dirty_tests:
>+                dirty_factory = TalosFactory(
>+                    OS=slave_platform,
>+                    toolsDir="tools",
>+                    envName=platform_config['env_name'],
>+                    workdirBase="../talos-data",
>+                    buildBranch=buildBranch,
>+                    branchName=branchName,
>+                    configOptions=TALOS_DIRTY_CONFIG_OPTIONS,
>+                    talosCmd=TALOS_CMD,
>+                    fetchSymbols=branch_config['fetch_symbols'],
>+                    plugins='',
>+                    pageset='',
>+                    talosAddOn='/builds/buildbot/profiles/dirtyDBs.zip',

A few things here:
* We should probably check-in dirtyDBs.zip somewhere before this goes into production
* I think config.py should gain a talosAddOn parameter (you can use talosAddOn=branch_config.get('talosAddOn', None) here to avoid setting it for every branch).
* It would be more futureproofed if you made talosAddOn a list, and looped through it in TalosFactory - that way it would support multiple addons.
It seems like it would be useful to check this in somewhere - I can see us wanting to look back in time if we update it. What do you think? Also, I wonder if it's better in the long term to set this in config.py, and make it a list. Eg, to support multiple addons.

What do you think about that?

Comment 25

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

* dirtyDBs.zip is updated and maintained on cruncher.b.m.o - it won't be checked in as it is being updated nightly and isn't static.  At the moment cruncher updates it and the talos master machines does a cron job that pulls in a new copy every night
* I agree that talosAddOn would be more useful as a list, I'll put that fix in
* I'm not sure about the config.py change, but I'll look at that as well

Comment 26

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

Attachment: add dirty profile test to talos-staging-pool buildbot configs (take 3)

Comment 27

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

Attachment: add support in buildbotcustom for dirty profile testing (mostly through additional file download) (take 4)

Added support for list of possible addon downloads.

Comment 28

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

Attachment: dirty ts testing on production pool/try pool

Comment 29

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

Attachment: [Checked in]add dirty profile test names to graph server (missed some in last patch)

Comment 30

[[e-mail] (:bhearsum)]

Comment on attachment 395193 [details] [diff] [review]
add support in buildbotcustom for dirty profile testing (mostly through additional file download) (take 4)

You need to use:
self.talosAddOn = talosAddOn[:] here to copy the list. Otherwise we'll end up hitting shared reference problems.

And a nit: s/talosAddOn/talosAddOns/

Comment 31

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

Attachment: dirty testing on staging/production/try talos buildbot-configs

Comment 32

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

Attachment: final version of buildbotcustom updates for dirty profile testing

Comment 33

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

Comment on attachment 395201 [details] [diff] [review]
[Checked in]add dirty profile test names to graph server (missed some in last patch)

changeset:   240:075668c41652

Also, had justdave push changes to production graph server.  Graph server changes complete now for dirty tests.

Comment 34

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

Attachment: talos support for dirty profile testing, now with added profile directories

Pretty much the same as before, but with the correct addition of the new base dirty profiles.

Comment 35

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

Profiles directory set up on talos-master along with crontab to pull updated profile db's nightly.

Comment 36

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

For downtime should:

- check in talos changes
- check in buildbot-config/buildbotcustom changes
- reconfig talos-pool and talos-try
- watch to ensure that columns start appearing on everything but Firefox3.0

I think that that should be it, the rest of the machine set up is already completed.

Comment 37

[[e-mail] (:bhearsum)]

Comment on attachment 395426 [details] [diff] [review]
talos support for dirty profile testing, now with added profile directories

I landed the previous version of this patch first, because of my own sillyness. The real landing is here:
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.50; previous revision: 1.49
done
Checking in sample.config;
/cvsroot/mozilla/testing/performance/talos/sample.config,v  <--  sample.config
new revision: 1.33; previous revision: 1.32
done
Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.31; previous revision: 1.30
done
RCS file: /cvsroot/mozilla/testing/performance/talos/places_generated_max/localstore.rdf,v
done
Checking in places_generated_max/localstore.rdf;
/cvsroot/mozilla/testing/performance/talos/places_generated_max/localstore.rdf,v  <--  localstore.rdf
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/places_generated_max/prefs.js,v
done
Checking in places_generated_max/prefs.js;
/cvsroot/mozilla/testing/performance/talos/places_generated_max/prefs.js,v  <--  prefs.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/places_generated_med/localstore.rdf,v
done
Checking in places_generated_med/localstore.rdf;
/cvsroot/mozilla/testing/performance/talos/places_generated_med/localstore.rdf,v  <--  localstore.rdf
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/places_generated_med/prefs.js,v
done
Checking in places_generated_med/prefs.js;
/cvsroot/mozilla/testing/performance/talos/places_generated_med/prefs.js,v  <--  prefs.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/places_generated_min/localstore.rdf,v
done
Checking in places_generated_min/localstore.rdf;
/cvsroot/mozilla/testing/performance/talos/places_generated_min/localstore.rdf,v  <--  localstore.rdf
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/places_generated_min/prefs.js,v
done
Checking in places_generated_min/prefs.js;
/cvsroot/mozilla/testing/performance/talos/places_generated_min/prefs.js,v  <--  prefs.js
initial revision: 1.1
done


I had to manually apply sample.config, probably because of the weird newlines in it.

Comment 38

[[e-mail] (:bhearsum)]

Comment on attachment 395420 [details] [diff] [review]
final version of buildbotcustom updates for dirty profile testing

changeset:   393:25ecb6001b40

Comment 39

[[e-mail] (:bhearsum)]

Comment on attachment 395419 [details] [diff] [review]
dirty testing on staging/production/try talos buildbot-configs

changeset:   1445:b0b059bdaaba

Comment 40

[[e-mail] (:bhearsum)]

Had to land this bustage fix: changeset:   1446:2cc63cf7e684 in buildbot-configs

Comment 41

[[e-mail] (:bhearsum)]

talos-pool and talos-try masters have been reconfig'ed for this.

Comment 42

[[e-mail] (:bhearsum)]

We've had green runs with this on try and in production.

Comment 43

[[e-mail] (:bhearsum)]

Had to push another bustage fix to get them reporting to Tinderbox:
changeset:   1448:950eb71dd8ae

Comment 44

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

Dirty profile testing up and running.  Some cleanup in bug 511766, but we can call this particular bug fixed.