Add ts test to talos that uses a dirty profile

RESOLVED FIXED

Status

P3
normal
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: anodelman, Assigned: anodelman)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 10 obsolete attachments)

704 bytes, patch
catlee
: review+
anodelman
: checked-in+
Details | Diff | Splinter Review
1.71 KB, patch
bhearsum
: review+
anodelman
: checked-in+
Details | Diff | Splinter Review
19.77 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
1.39 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
22.85 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
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.
(Assignee)

Updated

11 years ago
Component: Testing → Release Engineering: Future
Product: Core → mozilla.org
QA Contact: testing → release
Version: unspecified → other
OS: Mac OS X → All
Hardware: x86 → All
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.
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 :)
(Assignee)

Updated

10 years ago
Assignee: nobody → anodelman
Priority: -- → P3
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.
(Assignee)

Updated

10 years ago
Component: Release Engineering: Future → Release Engineering
(Assignee)

Comment 4

10 years ago
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).
(Assignee)

Updated

10 years ago
Depends on: 492135
(Assignee)

Comment 5

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

Comment 6

10 years ago
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
(Assignee)

Updated

10 years ago
Depends on: 498820
(Assignee)

Comment 7

10 years ago
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
(Assignee)

Comment 8

10 years ago
Created attachment 391244 [details] [diff] [review]
[Checked in]add dirty profile ts tests to graph server

No data to collect just yet, but we'll be ready.
Attachment #391244 - Flags: review?(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?
(Assignee)

Updated

10 years ago
Depends on: 507233
(Assignee)

Comment 10

10 years ago
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.
Attachment #391244 - Flags: review?(catlee) → review+
(Assignee)

Comment 11

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

changeset:   232:0c2db6202471
Attachment #391244 - Attachment description: add dirty profile ts tests to graph server → [Checked in]add dirty profile ts tests to graph server
Attachment #391244 - Flags: checked-in+
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.
(Assignee)

Comment 13

10 years ago
Created attachment 393107 [details] [diff] [review]
support dirty profile testing in talos

Mostly adding the option to use a specific profile for each individual test in a set.
Attachment #393107 - Flags: review?(catlee)
(Assignee)

Comment 14

10 years ago
Created attachment 393108 [details] [diff] [review]
add dirty profile test to talos-staging-pool buildbot configs
Attachment #393108 - Flags: review?(catlee)
(Assignee)

Comment 15

10 years ago
Created attachment 393109 [details] [diff] [review]
add support in buildbotcustom for dirty profile testing (mostly through additional file download)
Attachment #393109 - Flags: review?(catlee)
(Assignee)

Updated

10 years ago
Depends on: 480340
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.
Attachment #393109 - Flags: review?(catlee) → review+
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 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?
Attachment #393107 - Flags: review?(catlee) → review-
(Assignee)

Comment 19

10 years ago
Created attachment 393965 [details] [diff] [review]
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.
Attachment #393965 - Flags: review?(joduinn)
(Assignee)

Updated

10 years ago
Attachment #393107 - Attachment is obsolete: true
(Assignee)

Comment 20

10 years ago
Created attachment 393969 [details] [diff] [review]
add dirty profile test to talos-staging-pool buildbot configs (take 2)
Attachment #393108 - Attachment is obsolete: true
Attachment #393969 - Flags: review?(joduinn)
Attachment #393108 - Flags: review?(catlee)
(Assignee)

Comment 21

10 years ago
Created attachment 393970 [details] [diff] [review]
add support in buildbotcustom for dirty profile testing (mostly through additional file download) (take 2)

Fixed according to catlee's review comments.
Attachment #393109 - Attachment is obsolete: true
Attachment #393970 - Flags: review?(joduinn)
(Assignee)

Comment 22

10 years ago
Created attachment 393972 [details] [diff] [review]
add support in buildbotcustom for dirty profile testing (mostly through additional file download) (take 3)

Fixed bad var name in previous version of patch.
Attachment #393970 - Attachment is obsolete: true
Attachment #393972 - Flags: review?(joduinn)
Attachment #393970 - Flags: review?(joduinn)
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.
Attachment #393965 - Flags: review?(joduinn) → review+
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?
(Assignee)

Comment 25

10 years ago
* 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
(Assignee)

Comment 26

10 years ago
Created attachment 395191 [details] [diff] [review]
add dirty profile test to talos-staging-pool buildbot configs (take 3)
Attachment #393969 - Attachment is obsolete: true
Attachment #393969 - Flags: review?(joduinn)
(Assignee)

Comment 27

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

Added support for list of possible addon downloads.
Attachment #393972 - Attachment is obsolete: true
Attachment #395193 - Flags: review?(bhearsum)
Attachment #393972 - Flags: review?(joduinn)
(Assignee)

Updated

10 years ago
Attachment #395191 - Attachment is patch: true
Attachment #395191 - Attachment mime type: application/octet-stream → text/plain
Attachment #395191 - Flags: review?(bhearsum)
(Assignee)

Comment 28

10 years ago
Created attachment 395195 [details] [diff] [review]
dirty ts testing on production pool/try pool
Attachment #395195 - Flags: review?(bhearsum)
(Assignee)

Updated

10 years ago
Depends on: 511292
(Assignee)

Comment 29

10 years ago
Created attachment 395201 [details] [diff] [review]
[Checked in]add dirty profile test names to graph server (missed some in last patch)
Attachment #395201 - Flags: review?(bhearsum)
Attachment #395201 - Flags: review?(bhearsum) → review+
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/
Attachment #395193 - Flags: review?(bhearsum) → review-
Attachment #395191 - Flags: review?(bhearsum) → review+
Attachment #395195 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 31

10 years ago
Created attachment 395419 [details] [diff] [review]
dirty testing on staging/production/try talos buildbot-configs
Attachment #395191 - Attachment is obsolete: true
Attachment #395195 - Attachment is obsolete: true
Attachment #395419 - Flags: review?(bhearsum)
(Assignee)

Comment 32

10 years ago
Created attachment 395420 [details] [diff] [review]
final version of buildbotcustom updates for dirty profile testing
Attachment #395193 - Attachment is obsolete: true
Attachment #395420 - Flags: review?(bhearsum)
(Assignee)

Comment 33

10 years ago
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.
Attachment #395201 - Attachment description: add dirty profile test names to graph server (missed some in last patch) → [Checked in]add dirty profile test names to graph server (missed some in last patch)
Attachment #395201 - Flags: checked-in+
Attachment #395419 - Flags: review?(bhearsum) → review+
Attachment #395420 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 34

10 years ago
Created attachment 395426 [details] [diff] [review]
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.
Attachment #393965 - Attachment is obsolete: true
Attachment #395426 - Flags: review?(bhearsum)
(Assignee)

Comment 35

10 years ago
Profiles directory set up on talos-master along with crontab to pull updated profile db's nightly.
(Assignee)

Comment 36

10 years ago
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 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.
Attachment #395426 - Flags: review?(bhearsum)
Attachment #395426 - Flags: review+
Attachment #395426 - Flags: checked-in+
Comment on attachment 395420 [details] [diff] [review]
final version of buildbotcustom updates for dirty profile testing

changeset:   393:25ecb6001b40
Attachment #395420 - Flags: checked-in+
Comment on attachment 395419 [details] [diff] [review]
dirty testing on staging/production/try talos buildbot-configs

changeset:   1445:b0b059bdaaba
Attachment #395419 - Flags: checked-in+
Had to land this bustage fix: changeset:   1446:2cc63cf7e684 in buildbot-configs
talos-pool and talos-try masters have been reconfig'ed for this.
We've had green runs with this on try and in production.
Had to push another bustage fix to get them reporting to Tinderbox:
changeset:   1448:950eb71dd8ae
(Assignee)

Comment 44

10 years ago
Dirty profile testing up and running.  Some cleanup in bug 511766, but we can call this particular bug fixed.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.