Shouldn't produce nightly builds when there are no changes

RESOLVED FIXED

Status

Release Engineering
General
P3
normal
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: ted, Assigned: catlee)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [automation][l10n])

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
Currently catlee says we'll build and offer an update every day on Aurora even if there are no changes, the same way we do for mozilla-central. For Nightly, this seems fine, since people are used to it, but getting an update-per-day without any actual changes seems like something we should avoid for Aurora. I propose that we don't offer an update if there were no code changes since the previous build.

Comment 1

7 years ago
I believe that this is the current plan, not yet implemented.

Comment 2

7 years ago
The "no changes" needs to include the l10n repos, though, so that l10n communities can test the updates there.

Comment 3

7 years ago
That is going to be interesting!
Are you saying that if a locale makes one or more changes on a day it should trigger the L10n nightly repack for that locale (regardless if there is any en-US changes for that day)?

To be captain explicit:
* if >1 mozilla-aurora changes on the last day (regardless of L10n changes)
** create en-US nightly plus all locales
* if 0 mozilla-aurora changes on the last day & no L10n changes
** nothing gets generated
*if 0 mozilla-aurora changes on the last day & locales aa, bb & gg make a change or more
** we generate nightly L10n repacks for aa, bb, && gg against whatever latest aurora build there is.

Sounds good?
(Assignee)

Comment 4

7 years ago
(In reply to comment #2)
> The "no changes" needs to include the l10n repos, though, so that l10n
> communities can test the updates there.

Yeah, that's the tricky part.

Comment 5

7 years ago
I think there are several layers of trickyness vs load:

Most load, least tricky: do all builds every day

Less load, a tad tricky: do all builds if any repo changed

Little load, a good deal tricky: do en-US plus selected l10n builds if en-US unchanged and l10n did

"least" load, way tricky: do just selected new l10n repacks if en-US unchanged and l10n did

The last solution implies (AFAICT) having an app-update version number per locale, thus "way tricky".
In reply to comment #5
Isn't that what "make" is for? I mean, by specifying make targets with appropriate dependencies, wouldn't the make program take care of rebuilding only the targets whose dependencies have changed? Then the l10n builds would be "dependent" on OT1H the en-US and OTOH the specific translations for their locale. Or am I missing something?

Comment 7

7 years ago
nightlies are clobber builds, AFAICT. Also, as it's passing the build id for the day, there are going to be some changes to the build, and thus a non-empty update.

And of course, l10n repacks have nothing like dependencies. Like anything that runs through jar.mn, I'd say. Anything preprocessed is actually hard coded to be redone at all times, IIRC.

Updated

7 years ago
Whiteboard: [automation][l10n]
(In reply to comment #0)
> Currently catlee says we'll build and offer an update every day on Aurora even
> if there are no changes, the same way we do for mozilla-central. For Nightly,
> this seems fine, since people are used to it, but getting an update-per-day
> without any actual changes seems like something we should avoid for Aurora. I
> propose that we don't offer an update if there were no code changes since the
> previous build.

(In reply to comment #1)
> I believe that this is the current plan, not yet implemented.

Correct, we're still working on this. 

The intention here is to check if "mozilla-aurora" or any repo under http://hg.mozilla.org/releases/l10n/mozilla-aurora have changed, and if *any* of these have changed, then generate a new Aurora nightly build, and all of the l10n repacks.

Worst case this means we would be "wastefully" generating new build+l10n repacks for all locales when only one locale has made any change. We're still investigating, but I believe this cost is outweighed by the benefits:
1) each latest nightly on aurora will be of the same functionality and code revision
2) this avoids complexity in our nightly updater code (scenario of "if we only repack the specific locale that changes", then questions about partial-vs-complete updates being different for different locales on a given night quickly becomes complicated.)

Until we get this figured out and in production, we're generating builds nightly, in all locales, so we do not block localizers from starting work on Aurora.

Updated

7 years ago
Priority: -- → P3
(Assignee)

Comment 9

7 years ago
I believe this is fairly easy to do. We need an alternate version of lastGoodFunc (in buildbotcustom/misc_scheduler.py) that takes an l10n branch in addition to the regular branch. For aurora this would be "releases/l10n/mozilla-aurora". In the case where there are no source changes to mozilla-aurora in the past 24 hours, the new lastGoodFunc would check for any l10n changes on the l10n branch in the past 24 hours. Any new l10n changes would then trigger a new build on the tip of aurora, which would then result in a full set of repacks being done as well.
(In reply to comment #9)
> I believe this is fairly easy to do. We need an alternate version of
> lastGoodFunc (in buildbotcustom/misc_scheduler.py) that takes an l10n branch
> in addition to the regular branch. For aurora this would be
> "releases/l10n/mozilla-aurora". In the case where there are no source
> changes to mozilla-aurora in the past 24 hours, the new lastGoodFunc would
> check for any l10n changes on the l10n branch in the past 24 hours. Any new
> l10n changes would then trigger a new build on the tip of aurora, which
> would then result in a full set of repacks being done as well.

While a great improvement, wouldn't it be best if it would check for code changes on m-aurora and then, if none, only trigger a nightly repack?

And I wonder if it would make sense to not check based on "24 hours" but rather on the cset we have recorded for the existing build in ftp, that way, if for some reason our timing gets moved around for the nightly we don't miss a cset inbetween the cycles.

The general principal remains the same however.
(Assignee)

Comment 11

7 years ago
(In reply to comment #10)
> (In reply to comment #9)
> > I believe this is fairly easy to do. We need an alternate version of
> > lastGoodFunc (in buildbotcustom/misc_scheduler.py) that takes an l10n branch
> > in addition to the regular branch. For aurora this would be
> > "releases/l10n/mozilla-aurora". In the case where there are no source
> > changes to mozilla-aurora in the past 24 hours, the new lastGoodFunc would
> > check for any l10n changes on the l10n branch in the past 24 hours. Any new
> > l10n changes would then trigger a new build on the tip of aurora, which
> > would then result in a full set of repacks being done as well.
> 
> While a great improvement, wouldn't it be best if it would check for code
> changes on m-aurora and then, if none, only trigger a nightly repack?

The major objection to this is that we'd end up with two different repacks of the same locale with the same buildid, since the buildid of the repack is inherited from the en-US build.
And in a somewhat out-there scenario where aurora doesn't change for two days, but a locale does, then you would get partial update failures for that locale. Due to the ambiguity of the buildID again, and I don't think we want to go tweaking it to avoid this.
(Assignee)

Updated

7 years ago
Assignee: nobody → catlee
There's also a simple scenario for project branches that don't do l10n (but do do nightlies), like accessibility. Skip building en-US if the changeset isn't advancing.
(Assignee)

Comment 14

7 years ago
(In reply to comment #13)
> There's also a simple scenario for project branches that don't do l10n (but
> do do nightlies), like accessibility. Skip building en-US if the changeset
> isn't advancing.

those branches don't have l10n_repo_path set, right?
(Assignee)

Comment 15

7 years ago
Created attachment 543823 [details] [diff] [review]
don't trigger nightlies if nothing (including l10n) has changed
Attachment #543823 - Flags: review?(nrthomas)
Attachment #543823 - Flags: review?(bhearsum)
Right. I mentioned it because I noticed that a11y hasn't changed since June 16th, yet we keep building nightlies on the same revision and testing them, so it would be great if the code written here included support for branches without l10n.

Comment 17

7 years ago
Looking at the patch, I see that it's not building the next nightly if there was a en-US changeset that failed to be "good"? Is that intentional?
Comment on attachment 543823 [details] [diff] [review]
don't trigger nightlies if nothing (including l10n) has changed

Review of attachment 543823 [details] [diff] [review]:
-----------------------------------------------------------------

::: test/test_misc_scheduler_nightly.py
@@ +161,5 @@
> +            ssFunc = lastGoodFunc('b1', ['builder1', 'builder2'], triggerBuildIfNoChanges=False)
> +            ss = self.dbc.runInteractionNow(lambda t: ssFunc(self.s, t))
> +            self.assertEquals(ss, None)
> +
> +            # Check that ssFunc returns sometihng if triggerBuildIfNoChanges=False

Typo here: "sometihng"

@@ +169,5 @@
> +            ssFunc = lastGoodFunc('b2', ['builder1', 'builder2'], triggerBuildIfNoChanges=False, l10nBranch='b2-l10n')
> +            ss = self.dbc.runInteractionNow(lambda t: ssFunc(self.s, t))
> +            self.assertEquals(ss.revision, 'r234567890')
> +
> +            # Check that ssFunc returns sometihng if triggerBuildIfNoChanges=False

I think this comment should be "...returns None"
Attachment #543823 - Flags: review?(bhearsum) → review+
Comment on attachment 543823 [details] [diff] [review]
don't trigger nightlies if nothing (including l10n) has changed

>diff --git a/test/test_misc_scheduler_nightly.py b/test/test_misc_scheduler_nightly.py
> def createTestData(db):
...
>     db.runQueryNow("""INSERT INTO changes
>                     (`changeid`, `author`, `is_dir`, `comments`, `when_timestamp`, `branch`, `revision`, `revlink`)
>                     VALUES
>                     (2, 'me', 0, 'here', 1, 'b2', 'r234567890', 'from poller')""")
>+    db.runQueryNow("""INSERT INTO changes
>+                    (`changeid`, `author`, `is_dir`, `comments`, `when_timestamp`, `branch`, `revision`, `revlink`)
>+                    VALUES
>+                    (3, 'me', 0, 'here', 88200, 'b2-l10n', 'r234567890', 'from poller')""")

I think you want a unique revision for the l10n here, based on what I'm seeing in the scheduler db (changes.revision is the revision in the l10n repo, not an en-US one). Then check if any of the tests fail. r+ assuming it does/or it's an easy fix.

>+            # We acheive this by faking the clock

nit, spell-o

>+            # Check that ssFunc returns sometihng if triggerBuildIfNoChanges=False
>+            # and there are no good builds or l10n changes in the past 24 hours.

Not sure I grok why we'd try a nightly when we're trying to suppress them, and know the code doesn't compile and haven't been any changes. But perhaps I've gotten my wires crossed on how this works.
Attachment #543823 - Flags: review?(nrthomas) → review+
(Assignee)

Comment 20

7 years ago
(In reply to comment #19)
> >+            # Check that ssFunc returns sometihng if triggerBuildIfNoChanges=False
> >+            # and there are no good builds or l10n changes in the past 24 hours.
> 
> Not sure I grok why we'd try a nightly when we're trying to suppress them,
> and know the code doesn't compile and haven't been any changes. But perhaps
> I've gotten my wires crossed on how this works.

I think you're referring to the same block that Ben identified above. If so, I've corrected the comment to refer say "returns None", since we don't want to do builds in this case, as you say.
(Assignee)

Updated

7 years ago
Summary: Shouldn't produce nightly updates on Aurora when there are no changes → Shouldn't produce nightly builds when there are no changes
(Assignee)

Updated

7 years ago
Attachment #543823 - Flags: checked-in+

Comment 21

7 years ago
Landed on production and reconfigured.
(Assignee)

Comment 22

7 years ago
Looks like this worked. The scheduler skipped doing nightlies on a bunch of branches last night.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Chris, I just thought about this...

We (currently) have Nagios alerts that checks that a nightly (incl. l10n) is not more than 28 hours old, and alerts us if it is that old.

Do we have any plans/thoughts on what we can/should (or did) do about this alert.

* Disable/remove it
* Change the logic to follow this logic
* Keep it as-is and have action items to manually check if it flaps each day
* Other?

I'm ok with whatever path forward is chosen on that, I just would like to know to plan on emulating it for SeaMonkey's implementation of this bug.
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.