464165 - do l10n repackages on change for mozilla-central and 1.9.1

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Armen [:armenzg]]

This bug will track changes for:
- l10n nightly repackages on mozilla-central - bug 460791
- l10n repackages on change - bug 464163
- l10n changes for hg release automation - bug 464154
- l10n changes for dealing with approaching firefox3.1 branch - bug 464164

Comment 1

[chris hofmann]

when we change over from doing builds on push to the l10n repository to more frequent nightly builds are we going to have enough horsepower and dedicated vm's to make sure build cycle times stay low?

if we think this will be a problem should we get another bug on file to increase the machines dedicated to the task?

Comment 2

[Armen [:armenzg]]

(In reply to comment #1)
> when we change over from doing builds on push to the l10n repository to more
> frequent nightly builds are we going to have enough horsepower and dedicated
> vm's to make sure build cycle times stay low?
> 
> if we think this will be a problem should we get another bug on file to
> increase the machines dedicated to the task?
We have recently added several VMs to the pool of slaves for this and other projects that will require more work to happen in the slaves for now we are good

Comment 3

[Chris Cooper [:coop] (he/him)]

Changing the summary because that's what this bug boils down to anyway, and the old summary was *way* too open-ended.

Comment 4

[Chris Cooper [:coop] (he/him)]

Attachment: Merging Axel's changes to process/factory.py

The only real collisions I had when attempting to merge Axel's buildbotcustom with the releng version were in the mobile sections. This won't get checked in on its own, but I wanted to run this patch by both of you first to make sure I hadn't completely mucked up the merge.

Comment 5

[Aki Sasaki (not active)]

Comment on attachment 365038 [details] [diff] [review]
Merging Axel's changes to process/factory.py

Looks like this entire diff is reverting stuff I've done; feel free to revert or reverse patch.

Comment 6

[Aki Sasaki (not active)]

(In reply to comment #5)
> feel free to revert
> or reverse patch.

Er, revert locally. I'd like to keep my changes in the repository =)

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 365038 [details] [diff] [review]
Merging Axel's changes to process/factory.py

What aki said, this one looks like a bad merge.

Comment 8

[Chris Cooper [:coop] (he/him)]

Attachment: Complete merge from Axel's buildbotcustom repo

I went back and re-merged from scratch.

bhearsum: the only pain point now is changes/hgpoller.py, the rest is all file adds or moves. Feel free to concentrate your review there. I think the testing scripts are still WIP (Axel, correct me if I'm wrong).

Patch is running on staging-master right now.

Comment 9

[Chris Cooper [:coop] (he/him)]

Attachment: Complete merge from Axel's buildbotcustom repo, now with git diffs!

Same as before, but with git diffs for more clarity/completeness.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 365197 [details] [diff] [review]
Complete merge from Axel's buildbotcustom repo, now with git diffs!

>diff --git a/bin/README b/bin/README
>+++ b/bin/create-buildbot
>diff --git a/bin/create-stage b/bin/create-stage
(etc)

Can you create a subdirectory for these files in bin/ ?. If/When we have other files in there it's going to be confusing to have them side by side, esp. the README.

>+bigSlaveLock = locks.SlaveLock("bigSlaveLock")

I take it this lock is only for the test setup?



It would take me a week to understand this code. I don't understand why a lot of it exists (for example, why do we need Pluggable?), but Coop tells me Armen and him are happy with it.

Since most of this code doesn't touch non-l10n stuff I'm giving it a conditional r+. I do want to see it running in staging before we think about deploying it live though. Can you ping me next time you've got it up and running? I'm interested to see it both running, and even just to see the master side code.

Comment 11

[Axel Hecht [:Pike]]

Hrm, didn't get to that review yet, but I can comment on Ben's review comments.

Yeah, I'm fine with moving the scripts, or we could just rename the README to README-l10n-stage, so that we could have multiple readme files next to each other.

The bigSlaveLock is a bug, I didn't get that we're actually using max_builds now. I just converted my setup on the l10n server to use that, which fixes some awkward races nicely. Races that totally show up on my waterfalls on http://blog.mozilla.com/axel/2009/02/24/triggering-l10n-builds/, too. We should get those fixed in a follow up, IMHO.

Pluggable is a safe-safe guard against hanging requests. Basically, it allows me to unplug a callback when I want to, which you can't do with just a defer. So I'm adding an additional layer between the deferred and the actual callback that I can set to ignore.

Comment 12

[[e-mail] (:bhearsum)]

(In reply to comment #11)
> Hrm, didn't get to that review yet, but I can comment on Ben's review comments.
> 
> Yeah, I'm fine with moving the scripts, or we could just rename the README to
> README-l10n-stage, so that we could have multiple readme files next to each
> other.

Let's move them, please.

> 
> The bigSlaveLock is a bug, I didn't get that we're actually using max_builds
> now. I just converted my setup on the l10n server to use that, which fixes some
> awkward races nicely. Races that totally show up on my waterfalls on
> http://blog.mozilla.com/axel/2009/02/24/triggering-l10n-builds/, too. We should
> get those fixed in a follow up, IMHO.
> 

Cool.

> Pluggable is a safe-safe guard against hanging requests. Basically, it allows
> me to unplug a callback when I want to, which you can't do with just a defer.
> So I'm adding an additional layer between the deferred and the actual callback
> that I can set to ignore.

What does this get us over requests that timeout in a reasonable amount of time?

This was just an example though, really. I could be wrong, but it just seems like there's a lot of complicated code that doesn't need to be complicated. The l10n.ini config parsing is another example.

Comment 13

[Chris Cooper [:coop] (he/him)]

(In reply to comment #10) 
> Can you create a subdirectory for these files in bin/ ?. If/When we have other
> files in there it's going to be confusing to have them side by side, esp. the
> README.

Moved to bin/l10n-stage.

(In reply to comment #12)
> This was just an example though, really. I could be wrong, but it just seems
> like there's a lot of complicated code that doesn't need to be complicated. The
> l10n.ini config parsing is another example.

I'll file bugs for cleanup and/or simplification, but the quickest path to get us all working from the same codebase is to get this landed. As you said, this doesn't really impact existing releng code.

bhearsum: I'll get this version running in staging today so you can have a look.

Comment 14

[[e-mail] (:bhearsum)]

I had a look at this running yesterday and it seemed to work fine. I was particularly looking to make sure that existing HgPoller's and builds ran fine - and indeed, they seemed to. I still think it's confusing within our existing display systems to have multiple build types in the same column - but I know we're working towards a better system.

A nice side effect of this patch is getting a file list into the Change object - nice.

Consider this a full r=bhearsum at this point.

Comment 15

[Chris Cooper [:coop] (he/him)]

Attachment: Buildbot config changes for l10n repack on change for m-c and 1.9.1

I still have some concerns about disk space on slaves, but I'd like to push this to production during off-peak tonight and let it run over the weekend. If we hit problems with disk space, I can yank the plug on 1.9.1.

I'll adjust bsmedberg's machines to send builds to a different dir in the meantime.

Comment 16

[Chris Cooper [:coop] (he/him)]

Comment on attachment 365197 [details] [diff] [review]
Complete merge from Axel's buildbotcustom repo, now with git diffs!

changeset:   209:bf5c131a5d73

Comment 17

[Chris Cooper [:coop] (he/him)]

Comment on attachment 365963 [details] [diff] [review]
Buildbot config changes for l10n repack on change for m-c and 1.9.1

changeset:   987:81a3fe325029

Comment 18

[Chris Cooper [:coop] (he/him)]

Attachment: Changes missing from the first patch

I've made these local changes on production-master to get things started tonight. They amount to a few missing changes in mozilla2/release_master.py related to moving the l10n module and the config.py changes necessary to enable_l10n for 1.9.1.

I'm sending the 1.9.1 builds directly to latest-mozilla-1.9.1-l10n and have changed bsmedberg's builders to send to experimental (see bug 467833).

Comment 19

[Chris Cooper [:coop] (he/him)]

The reconfig has happened. We have pending l10n nightlies for both m-c and 1.9.1 in about 10 hours (other l10n builds may occur in the interim if check-ins happen).

I'm updating ReleaseEngineering:BuildbotMasterChanges now, and will continue to monitor the master and builds over the weekend.

Comment 20

[Chris Cooper [:coop] (he/him)]

Attachment: Start monitoring latest-mozilla-1.9.1-l10n

Since we'll finally have l10n nightlies here, we should start monitoring them.

Comment 21

[Axel Hecht [:Pike]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f162aa6d3ad9408271318621352b520789c03917 triggered the 1.9.1 builders as expected just now :-)

Pushed just before 6, mac and linux builds done by 6:20, windows builds by 6:40.

There is one expected red, which is Malay (ms), which didn't land on 1.9.1 yet. Oddly enough, the windows build is failing, but I didn't find the corresponding box on the Mozilla-l10n tinderbox page.

Comment 22

[Axel Hecht [:Pike]]

All in all, this seems to work pretty smoothly, I have found one issue in l10nbuilds.ini (on both staging and not), they're both specifying an explicit list of locales instead of 'all'. Not sure if we want to handle that in a separate bug.

Comment 23

[Chris Cooper [:coop] (he/him)]

(In reply to comment #22)
> All in all, this seems to work pretty smoothly, 

Agreed.

> I have found one issue in
> l10nbuilds.ini (on both staging and not), they're both specifying an explicit
> list of locales instead of 'all'. Not sure if we want to handle that in a
> separate bug.

No, we can switch it to "all" if that's what we need. I just carried forward the ini files Armen was using. I'll consider this an r+ for that change to l10nbuilds.ini.

Comment 24

[Axel Hecht [:Pike]]

Yeah, all that's needed is un-commenting the locales lines, and removing the static lists.

Comment 25

[Chris Cooper [:coop] (he/him)]

Comment on attachment 366066 [details] [diff] [review]
Start monitoring latest-mozilla-1.9.1-l10n

Checking in Firefox_mozilla-1.9.1.txt;
/cvsroot/mozilla/tools/tinderbox-configs/monitoring/Firefox_mozilla-1.9.1.txt,v  <--  Firefox_mozilla-1.9.1.txt
new revision: 1.3; previous revision: 1.2
done

Comment 26

[Chris Cooper [:coop] (he/him)]

Comment on attachment 366062 [details] [diff] [review]
Changes missing from the first patch

changeset:   988:cdae41de4ee

Comment 27

[Chris Cooper [:coop] (he/him)]

Seems to be working. Axel will be filing separate follow-up bugs.