Status

defect
P3
normal
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: sdwilsh, Assigned: lsblakk)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 10 obsolete attachments)

7.65 KB, patch
bhearsum
: checked-in+
Details | Diff | Splinter Review
8.37 KB, patch
bhearsum
: checked-in+
Details | Diff | Splinter Review
3.10 KB, patch
catlee
: review+
Details | Diff | Splinter Review
996 bytes, patch
anodelman
: review+
catlee
: checked-in+
Details | Diff | Splinter Review
4.45 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
4.50 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
Reporter

Description

11 years ago
The places folks would like a branch to do work in (kinda like TraceMonkey).  We'd also like to have some Talos boxes (at least one per platform) for some perf data.

This could really help us get some work slated for beta 2 squared away without holding mozilla-central hostage every so often.
Can you clarify:

1) what you are looking for on this project branch? Builders on which o.s.? unittests? Talos is trickier to setup - which talos tests? Also, do you want a separate tinderbox waterfall, or do you want this on the main Firefox page?

2) when you want this by? Reading comment#0 makes me think you want this well in advance of beta2?
Summary: Setup a branch for places → Setup a project branch for places
ps: sorry for delay in responding. this bug was filed right after our last triage, and then we skipped one because of the RelEng gathering in Toronto.
Reporter

Comment 3

11 years ago
I think, for the time being, we can hold off at least until after beta 2 to decide on this.
ok. Moving to future for now. Please update when you have more info.
Component: Release Engineering → Release Engineering: Future
Priority: -- → P3
In advance of starting this project branch work, could you please answer the questions below? 

These questions would have helped speed up creating the ActionMonkey and TraceMonkey project branches, so we hope these questions and answers will speed up creating the new Places project branch. 


*do you want builds?
** which o.s.? 
*** All o.s. or subset of linux, mac, win32, linux-arm
** incremental-build-on-checkin? y/n
** nightlies? y/n
** Are en-US builds enough, with no l10n? y/n
*** Note: If you specifically need l10n, this currently needs dedicated machines, and 'significantly' longer setup. 
*** If you 'must' have l10n, do you need all locales from m-c or a subset of locales?

* do you want unittests?
** which o.s.? 
*** All o.s. or subset of linux, mac, win32, linux-arm

* do you want talos?
** which o.s.?
*** All o.s. or subset of linux, mac, win32
** fast talos or full talos or both?
*** Note: talos requires dedicated h/w, so will need 'significantly' more time to buy, install, configure, calibrate machines. 

*name of branch owner, who will:
** be doing periodic refreshes from m-c
** be contact person for misc setup questions
** decide when to land back project branch onto m-c
** decide when to terminate the project branch

*timeline:
** when can we start project branch (any pre-req landings pending needed before starting project branch out from mozilla-central?)
** approx expected life span of project branch - if known?

*misc:
** need any changes to toolchain used in m-c?
** need any changes to the compile/link/repack steps used in m-c?
** preference on tinderbox waterfall name?
** preference on where to put builds on ftp.m.o?
*** used for places tinderbox-builds/my-project-branch, nightly/latest-my-project-branch/, or nightly/2008-08-08-08-my-project-branch/
** preference on name of project branch in hg?

* any other info that might be helpful to us?


Note: RelEng will use this one tracking bug for all of the above
setups, and RelEng will create dependent bugs as needed.
Summary: Setup a project branch for places → Setup a "Places" project branch
I talked with sdwilsh, and we no longer need this project branch, so this bug can be closed. If we need one in the future, we'll queue it up in advance. Thanks!
hooray!
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
(In reply to comment #6)
> I talked with sdwilsh, and we no longer need this project branch, so this bug
> can be closed. If we need one in the future, we'll queue it up in advance.
> Thanks!

Thanks for the confirm, Dietrich!
Resolution: WONTFIX → FIXED
How is this FIXED if no branch was created? Seems like WONTFIX or INVALID would be better choices.
Reporter

Comment 10

10 years ago
I think (assuming dietrich signs off on this), that we'll want this now for future big stuff (like stuff we have planned for 1.9.2).  I guess we'll want it called something like places-dev, so we can leave open the possibility of an experimental branch (for experimental UI, etc).

We'd need a windows, linux, and mac builder (probably just fine for vista and 10.5), as well as unit tests on those platforms.  Talos would be really nice as well (and a requirement before we could really begin to use the branch really).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter

Comment 11

10 years ago
* do you want builds?
      o which o.s.? linux, mac, win32.
      o incremental-build-on-checkin? yes please
      o nightlies? no
      o Are en-US builds enough, with no l10n? en-US is sufficient

* do you want unittests? yes
      o which o.s.? linux, mac, win32

* do you want talos? yes
      o which o.s.? linux, mac, win32
      o fast talos or full talos or both? just full talos

* name of branch owner, who will:
      o be doing periodic refreshes from m-c? sdwilsh
      o be contact person for misc setup questions? dietrich
      o decide when to land back project branch onto m-c? places team (dietrich, mak, adw, ddahl, sdwilsh)
      o decide when to terminate the project branch? dietrich

* timeline:
      o when can we start project branch?  Whenever you are ready.
      o approx expected life span of project branch - if known?  indefinite

* misc:
      o need any changes to toolchain used in m-c? no
      o need any changes to the compile/link/repack steps used in m-c? no
      o preference on tinderbox waterfall name? places-dev
      o preference on where to put builds on ftp.m.o? no preference
      o preference on name of project branch in hg? places-dev
      o do you want nagios monitoring on this project branch? it is unclear what this would get us.  If it's useful, yes; if not, no.
      o what unofficial projectname do you want on this project branch? Kilimanjaro

* any other info that might be helpful to us?
Possible to have tinderboxpushlog instead of tinderbox?
Talked with john IRL, and indicted that we'd be OK being test-dummys for pool-of-talos since we wouldn't get talos until it was ready anyway.
We'd like to be able to let people who do not have access to mozilla-central to be able to land on this branch if possible (I think tracemonkey does this)
Assignee: nobody → joduinn
Component: Release Engineering: Future → Release Engineering
(In reply to comment #11)
>       o preference on tinderbox waterfall name? places-dev
talked with sdwilsh about slight tweak to name and have created: 
http://tinderbox.mozilla.org/showbuilds.cgi?tree=Places
Reporter

Comment 13

10 years ago
What are the odds we can get a tinderboxpushlog setup instead of tinderbox?
(In reply to comment #13)
> What are the odds we can get a tinderboxpushlog setup instead of tinderbox?

Am expecting to have tinderboxpushlog UI for places project branch. It currently pulls data from tinderbox, so getting tinderbox going is first step.

Comment 15

10 years ago
I would like commit access to this repo.
Reporter

Comment 16

10 years ago
Yeah - we'd like this setup like TraceMonkey where folks who do not have access to mozilla-central can in fact commit things.
(In reply to comment #16)
> Yeah - we'd like this setup like TraceMonkey where folks who do not have access
> to mozilla-central can in fact commit things.

You'll need to file an IT bug to set up a new Hg repo for this. Each person who wants access who doesn't already have Hg access will need to go through the process of getting an Hg account (SSH key, form, two vouchers). No SR is required, and the vouchers can both be MoCo if needed, but they are still required.
Attachment #384902 - Flags: review?(bhearsum)
This also changes the tracemonkey major_version to 1.9.2 to match production settings.

Ran this on staging and the builds, nightlies and unittest runs all went through fine (setup wise).  The packaged build unittests had some warnings but that seems to match the current state of these builds in m-c.
Attachment #384903 - Flags: review?(bhearsum)
Comment on attachment 384903 [details] [diff] [review]
Enable places in staging-master

>diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py

>+######## places
>+BRANCHES['places']['repo_path'] = 'projects/places'
>+BRANCHES['places']['major_version'] = '1.9.2'
>+BRANCHES['places']['product_name'] = 'firefox'
>+BRANCHES['places']['app_name'] = 'browser'
>+BRANCHES['places']['brand_name'] = 'Kilimanjaro'

I don't think we should use this here. While brand_name isn't used at the moment, it needs to match the branding of the product in the repository because it can be used for helping find the correct filename (esp. on Mac). Please change it to 'Minefield' to match the branding in the repo.

>+BRANCHES['places']['platforms'] = {
>+    'linux': {},
>+    'win32': {},
>+    'macosx': {}
>+}
>+BRANCHES['places']['platforms']['linux']['mozconfig'] = 'linux/places/nightly'
>+BRANCHES['places']['platforms']['macosx']['mozconfig'] = 'macosx/places/nightly'
>+BRANCHES['places']['platforms']['win32']['mozconfig'] = 'win32/places/nightly'
>+BRANCHES['places']['platforms']['linux']['base_name'] = 'Linux places-dev'
>+BRANCHES['places']['platforms']['win32']['base_name'] = 'WINNT 5.2 places-dev'
>+BRANCHES['places']['platforms']['macosx']['base_name'] = 'OS X 10.5.2 places-dev'

It looks like dietrich wanted everything to be 'places-dev' (the repository, tinderbox, etc). Since the repository was already cloned as 'places' I asked it would be OK to use that everywhere - he said it was fine. Please make that change.
Attachment #384903 - Flags: review?(bhearsum) → review-
Comment on attachment 384903 [details] [diff] [review]
Enable places in staging-master


>+BRANCHES['places']['tinderbox_tree'] = 'Places'

one more thing here: This should be 'MozillaTest' for staging.
Comment on attachment 384902 [details] [diff] [review]
Enable places in production-master

Same comments here, minus the tinderbox_tree change.
Attachment #384902 - Flags: review?(bhearsum) → review-
One overall comment here, too: We need more slaves on all platforms before we can enable this without overloading the master. Quite a few times lately I've seen pending builds on multiple platforms - even without release builds running.
(In reply to comment #20)
> It looks like dietrich wanted everything to be 'places-dev' (the repository,
> tinderbox, etc). Since the repository was already cloned as 'places' I asked it
> would be OK to use that everywhere - he said it was fine. Please make that
> change.
Yeah, I did check that "places not places-dev" was ok before getting repo etc setup. Sorry for not updating this bug with that info. 


(In reply to comment #23)
> One overall comment here, too: We need more slaves on all platforms before we
> can enable this without overloading the master. Quite a few times lately I've
> seen pending builds on multiple platforms - even without release builds
> running.
Getting this project branch setup, debugging setup issues and enabled is not gated on new slaves. After all, the only builds being triggered on that line will be occasional ones from us, verifying that setup is ok.

After this branch is fully setup and handed over to Dev, then yes, we'll have increased load as developers get used to the new branch and ramp up checkins on it. If you feel we need extra slaves in place to handle that load, lets work that out offline, and set them up before we handover the working branch to Dev. fwiw, I've been watching the build/unittest wait times, and we didnt seem backlogged there, so lets figure out the mismatch in what we're watching.
with changes as per bhearsum's comments.
Attachment #384902 - Attachment is obsolete: true
Attachment #385471 - Flags: review?(bhearsum)
also with changes as per bhearsum's comments.
Attachment #384903 - Attachment is obsolete: true
Attachment #385472 - Flags: review?(bhearsum)
(In reply to comment #24)
> (In reply to comment #20)
> > It looks like dietrich wanted everything to be 'places-dev' (the repository,
> > tinderbox, etc). Since the repository was already cloned as 'places' I asked it
> > would be OK to use that everywhere - he said it was fine. Please make that
> > change.
> Yeah, I did check that "places not places-dev" was ok before getting repo etc
> setup. Sorry for not updating this bug with that info. 

Ah, too funny. I actually did update the bug with that info back in comment#12!
Reassigning to Lukas, as she is actively working on it now.
Assignee: joduinn → lsblakk
Attachment #385471 - Flags: review?(bhearsum) → review+
Attachment #385472 - Flags: review?(bhearsum) → review+
Attachment #385471 - Flags: checked‑in?
Attachment #385472 - Flags: checked‑in?
Just enabling shark builds.
Attachment #386090 - Flags: checked‑in?
Attachment #385471 - Attachment is obsolete: true
Attachment #385471 - Flags: checked‑in?
Same thing for staging, enabled shark builds.
Attachment #386092 - Flags: checked‑in?
Attachment #385472 - Attachment is obsolete: true
Attachment #385472 - Flags: checked‑in?
Depends on: 501710
Attachment #386090 - Flags: checked‑in? → checked‑in+
Attachment #386092 - Flags: checked‑in? → checked‑in+
Attachment #386090 - Flags: checked‑in+ → checked‑in?
Attachment #386092 - Flags: checked‑in+ → checked‑in?
Comment on attachment 386090 [details] [diff] [review]
Enable places in production-master - take 3

changeset:   1282:1413a647b0ea
Attachment #386090 - Flags: checked‑in? → checked‑in+
Attachment #386092 - Flags: checked‑in? → checked‑in+
Comment on attachment 386092 [details] [diff] [review]
Enable places in staging-master - take 3

changeset:   1282:1413a647b0ea
It looks like these patches are going to stick this time. Please refrain from checking into this branch while things settle down, though.
We're currently working through an issue that's preventing us from picking up changes to the places repository. Things are OK with these new builders otherwise.
Depends on: 503081
We found the issue with the polling, we'll have a fix in tomorrow during downtime. bug 493740
Depends on: 493740
No longer depends on: 503081
Posted patch Add Places to Talos config (obsolete) — Splinter Review
Polling issue was fixed and now the places branch is ready to go once talos patch is landed and more buildslaves are added to the pool to handle the new workload.
Attachment #387720 - Flags: review?(anodelman)
Attachment #387720 - Attachment is obsolete: true
Attachment #387733 - Flags: review?(catlee)
Attachment #387720 - Flags: review?(anodelman)
Attachment #387734 - Flags: review?(catlee)
Comment on attachment 387733 [details] [diff] [review]
Add Places to Talos Pool config.py

Looks like this is against an older version of config.py, the jss and tp4 config variables are missing
Attachment #387733 - Flags: review?(catlee) → review-
Comment on attachment 387734 [details] [diff] [review]
Add Places to Talos Staging Pool config.py

Same thing here
Attachment #387734 - Flags: review?(catlee) → review-
All in one patch this time, sorry for not being on the most up to date, this time it's based on tip.
Attachment #387733 - Attachment is obsolete: true
Attachment #387734 - Attachment is obsolete: true
Attachment #387892 - Flags: review?(catlee)
Attachment #387892 - Flags: review?(catlee) → review+
Comment on attachment 387892 [details] [diff] [review]
Add Places to Talos Pool config.py for production and staging

changeset:   1300:07d3e8d4635d
Attachment #387935 - Flags: review?(anodelman) → review+
Comment on attachment 387935 [details] [diff] [review]
Add the Places branch to the graph server

changeset:   226:d0948ac41bfd
Attachment #387935 - Flags: checked‑in+
Reporter

Comment 46

10 years ago
An interesting thing about the pooled talos is that the points between runs aren't connected unless you actually get the same machine more than once.  I'm not sure if this should be considered a bug or not...
http://➡.ws/읗
Reporter

Comment 47

10 years ago
Also, we seem to be missing leak test boxes that provide the Rlk, Lk, MH, and A numbers.  That wasn't on the initial form either.
(In reply to comment #46)
> An interesting thing about the pooled talos is that the points between runs
> aren't connected unless you actually get the same machine more than once.  I'm
> not sure if this should be considered a bug or not...
> http://➡.ws/읗

This has always been the case, even with the non-pooled talos.  I guess the only difference is that there are more slaves, so you're less likely to get the same slave.

Maybe the graph server or dashboard should have a mode to merge all the datasets from the various machines together?
(In reply to comment #48)
> Maybe the graph server or dashboard should have a mode to merge all the
> datasets from the various machines together?

Sounds good to me. Even better if it can be done so that we can still spot machines that are consistently out of step with their peers.
Reporter

Comment 50

10 years ago
(In reply to comment #48)
> This has always been the case, even with the non-pooled talos.  I guess the
> only difference is that there are more slaves, so you're less likely to get the
> same slave.
Yup, that's what I figured, and that's why I didn't just file a bug.  It's not clear if this is a bug or not.
(In reply to comment #49)
> (In reply to comment #48)
> > Maybe the graph server or dashboard should have a mode to merge all the
> > datasets from the various machines together?
> 
> Sounds good to me. Even better if it can be done so that we can still spot
> machines that are consistently out of step with their peers.

sdwilsh,nthomas: You can already do both of those.
* To see the results of all the machines on the same graph, select the testsuite, branch, o.s. you are interested in, and then click on "add all" button on the left-hand-side of graphs.m.o. 
* The resulting graph shows different colors for different machines, and you can add/remove machines by clicking on their name in the "Displayed Graphs" panel on the left-hand-side of grapsh.m.o. This means we can spot (and isolate!) a misbehaving slave if needed. 

We worked this out as part of setup for pooling talos slaves, and it works the same across all project branches. If this doesnt give you the functionality you want, please file a separate bug for this. There's nothing places-branch specific here, and it shouldnt distract from the rest of the places branch setup work here.

Make sense?
John, if you look at something like http://tinyurl.com/ll99sp, the suggestion is to draw the line using all the points for a test, rather than all the points from a particular machine.
(In reply to comment #47)
> Also, we seem to be missing leak test boxes that provide the Rlk, Lk, MH, and A
> numbers.  That wasn't on the initial form either.

Hm, looks like we based this config off of Tracemonkey, which doesn't have them either. Do you want them?
Reporter

Comment 54

10 years ago
(In reply to comment #53)
> Hm, looks like we based this config off of Tracemonkey, which doesn't have them
> either. Do you want them?
I think we've landed things in the past that have caused them to go orange, so it'd be nice to have.  I'd be OK with that being done in a follow-up bug though.
(In reply to comment #54)
> (In reply to comment #53)
> > Hm, looks like we based this config off of Tracemonkey, which doesn't have them
> > either. Do you want them?
> I think we've landed things in the past that have caused them to go orange, so
> it'd be nice to have.  I'd be OK with that being done in a follow-up bug
> though.

It's not a big deal to turn on. Lukas - can you write the patch to do that?
Attachment #388245 - Flags: review?(bhearsum)
Attachment #388246 - Flags: review?(bhearsum)
Fixed some copy/paste errors
Attachment #388245 - Attachment is obsolete: true
Attachment #388253 - Flags: review?(bhearsum)
Attachment #388245 - Flags: review?(bhearsum)
Fixed copy/paste errors
Attachment #388246 - Attachment is obsolete: true
Attachment #388254 - Flags: review?(bhearsum)
Attachment #388246 - Flags: review?(bhearsum)
Running in staging - triggered one build of each platform which report to tinderbox MozillaTest and should have results in the next few hours.
Comment on attachment 388254 [details] [diff] [review]
Turn on leak testing for Places staging


>-BRANCHES['places']['platforms']['linux']['builds_before_reboot'] = 5
>-BRANCHES['places']['platforms']['win32']['builds_before_reboot'] = 5
>-BRANCHES['places']['platforms']['macosx']['builds_before_reboot'] = 5
>+BRANCHES['places']['platforms']['linux-debug']['build_space'] = 3
>+BRANCHES['places']['platforms']['win32-debug']['build_space'] = 4
>+BRANCHES['places']['platforms']['macosx-debug']['build_space'] = 3
>+BRANCHES['places']['platforms']['linux']['builds_before_reboot'] = None
>+BRANCHES['places']['platforms']['win32']['builds_before_reboot'] = None
>+BRANCHES['places']['platforms']['macosx']['builds_before_reboot'] = None
>+BRANCHES['places']['platforms']['linux-debug']['builds_before_reboot'] = None
>+BRANCHES['places']['platforms']['win32-debug']['builds_before_reboot'] = None
>+BRANCHES['places']['platforms']['macosx-debug']['builds_before_reboot'] = None

Please leave builds_before_reboot on in staging - set all of these to 5.
Attachment #388254 - Flags: review?(bhearsum) → review-
Attachment #388253 - Flags: review?(bhearsum) → review+
Comment on attachment 388253 [details] [diff] [review]
Turn on leak testing for Places production

This looks fine
fixed reboot setting.
Attachment #388254 - Attachment is obsolete: true
Attachment #388280 - Flags: review?(bhearsum)
Attachment #388280 - Flags: review?(bhearsum) → review+
Attachment #388253 - Flags: checked‑in+
Attachment #388280 - Flags: checked‑in+
Landed the leak test patches:
changeset:   1308:44dab27d7884
changeset:   1309:dedcaa51b018
changeset:   1310:f3aee06e1e39

Updating the master now.
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
The first leak runs generated corrupted logs and so we lost a couple of builds, then copied over the m-c logs to compare against so the next round will have wacky results but after that it should be fine.
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.