570814 - Nightly builds should all use the same revision

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P4)

Description

[Chris AtLee [:catlee]]

Attachment: Use the same revision for all nightly builds

This is probably a dup, but I can't find it right now.

The nightly builds for all platforms on a branch should all use the same revision.  It would be nice if we looked back in time a bit and chose revisions that were green, or at least built successfully.

Comment 1

[[e-mail] (:bhearsum)]

Comment on attachment 449942 [details] [diff] [review]
Use the same revision for all nightly builds

I'm not entirely fit to comment on 0.8.0/schedulerdb best practices, but at a higher level, this looks great. It falls back on a old behaviour if it can't find a green nightly, too.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 449942 [details] [diff] [review]
Use the same revision for all nightly builds

Looks good apart from a few suggestions.

>diff --git a/misc.py b/misc.py
>+    nightly_scheduler=SpecificNightly(
...
>+        hour="*", minute=range(0,60,3),

Hope this is just for testing!

>diff --git a/scheduler.py b/scheduler.py
>+def lastGreen(db, branch, builderNames, max_search=100):
>+            WHERE
>+                buildsets.sourcestampid = sourcestamps.id AND
>+                buildrequests.buildsetid = buildsets.id AND
>+                buildrequests.complete = 1 AND
>+                buildrequests.results IN (0,1) AND
>+                sourcestamps.revision IS NOT NULL AND
>+                buildrequests.buildername in %s

Need to limit by branch here too, using like to cope with the way we use tracemonkey-win32-opt-unittest and friends.

The max_search parameter will be a bit vulnerable to how many builds occur per revision. We could avoid that if we looked back at most max_search revisions, possibly getting that information in a similar way to lastChangeset().

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #2)
> Need to limit by branch here too, using like to cope with the way we use
> tracemonkey-win32-opt-unittest and friends.

Beg your pardon, I was testing without the condition on buildername.

Comment 4

[Chris AtLee [:catlee]]

Attachment: Use the same revision for all nightly builds

Basically the same as before...I removed the debugging schedule and also added some logging for how long the operation is taking.  It only runs once per day per branch, so the extra logging won't be a problem.

I increased the max_search to 10,000 builds.  On the current production database, this query takes 0.05s to run.

I renamed lastGreen* to lastGood* because we'll also accept orange builds.

Comment 5

[Chris AtLee [:catlee]]

Comment on attachment 452715 [details] [diff] [review]
Use the same revision for all nightly builds

Small fix coming

Comment 6

[Chris AtLee [:catlee]]

Attachment: Use the same revision for all nightly builds

Same as before, except don't create schedulers with empty builder lists.

Comment 7

[[e-mail] (:bhearsum)]

Comment on attachment 452787 [details] [diff] [review]
Use the same revision for all nightly builds

This looks OK to me, but I saw on the newsgroups that someone didn't want to look so far back in history. Doesn't look like that quite got figured out. r+, but I bet some changes will be needed!

Comment 8

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

(In reply to comment #7)
> Comment on attachment 452787 [details] [diff] [review]
> Use the same revision for all nightly builds
> 
> This looks OK to me, but I saw on the newsgroups that someone didn't want to
> look so far back in history. Doesn't look like that quite got figured out. r+,
> but I bet some changes will be needed!

Further discussions in newsgroup, summarized. 

It doesn't feel useful to attempt a nightly that we know will fail to compile-and-link. To avoid this, we're proposing two mechanical changes. For a given night, on a given branch:

1) use the same source code revision for each OS

2) use a "good" source code revision. To start with, the definition of
"good" is that it does successfully compile-and-link on all OS. If the
latest source code revision did not compile-and-link, we'll use the
previous most recent code revision that *does* compile-and-link.

(Initially, we're defining "good" to be "compile-and-link" on all OS.
However, over time, we're planning to raise the standard to
"compile-and-link-and-pass-some-testsuites", and eventually
"compile-and-link-and-pass-all-testsuites". However, that will obviously
need unittests to be more stable first!)

If we ever find there are no "good" changesets since the previous
nightly, we are planning to not produce another nightly of the same "good"
source code as the previous night. Instead, we're going to wait for a "good"
revision.

Comment 9

[Chris AtLee [:catlee]]

Attachment: Use the same revision for all nightly builds...plus some extra fun

Comment 10

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

(In reply to comment #9)
> Created attachment 488573 [details] [diff] [review]
> Use the same revision for all nightly builds...plus some extra fun

catlee: will this approach for consistent nightly buildIDs also give us consistent release buildIDs?

Comment 11

[Chris AtLee [:catlee]]

(In reply to comment #10)
> (In reply to comment #9)
> > Created attachment 488573 [details] [diff] [review] [details]
> > Use the same revision for all nightly builds...plus some extra fun
> 
> catlee: will this approach for consistent nightly buildIDs also give us
> consistent release buildIDs?

Not immediately, but it won't be hard to apply the same ideas to the release builds as well.

Comment 12

[Dustin J. Mitchell [:dustin] (he/him)]

+    def addTriggeredBuildsSteps(self,
+                                triggeredSchedulers=None):
+        '''Trigger other schedulers.
+        We don't include these steps by default because different
+        children may want to trigger builds at different stages.
+        '''
+        if triggeredSchedulers is None:
+            if self.triggeredSchedulers is None:
+                return True
+            triggeredSchedulers = self.triggeredSchedulers
+
+        for triggeredScheduler in triggeredSchedulers:
+            self.addStep(Trigger(
+                schedulerNames=[triggeredScheduler],
+                copy_properties=['buildid', 'builduid'],
+                waitForFinish=False))
+

I think you wanted to copy (self.triggeredSchedulers[:]) there? Or not use a local variable at all?

Looks good otherwise..

Comment 13

[Chris AtLee [:catlee]]

Comment on attachment 488573 [details] [diff] [review]
Use the same revision for all nightly builds...plus some extra fun

I don't think there's any reason to copy self.triggeredSchedulers...it's just in the for loop a few lines down.

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Nit: you've got empty lines at the end of test/test_misc_scheduler_propscheduler.py and test/test_misc_scheduler_propfuncs.py.

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 488573 [details] [diff] [review]
Use the same revision for all nightly builds...plus some extra fun

see comment #12

Comment 16

[Chris AtLee [:catlee]]

Comment on attachment 488573 [details] [diff] [review]
Use the same revision for all nightly builds...plus some extra fun

A bit of a bumpy landing.  Messed up merging code into default, and then ran into a reconfig-only problem.

changeset:   1131:0b84a062f16b
changeset:   1133:94b7596a2523
changeset:   1134:73d58829d012

Comment 17

[Chris AtLee [:catlee]]

Masters were reconfigured this morning.  New builds look like they're getting ids set properly.

Will close this tomorrow assuming nightly builds happen as expected.

Comment 18

[Chris AtLee [:catlee]]

Looks like everything got triggered as expected!

Comment 20

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

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > Created attachment 488573 [details] [diff] [review] [details] [details]
> > > Use the same revision for all nightly builds...plus some extra fun
> > 
> > catlee: will this approach for consistent nightly buildIDs also give us
> > consistent release buildIDs?
> 
> Not immediately, but it won't be hard to apply the same ideas to the release
> builds as well.

Filed bug#617840 to track.