The default bug view has changed. See this FAQ.

Nightly builds should all use the same revision

RESOLVED FIXED

Status

Release Engineering
Other
P4
normal
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: catlee, Assigned: catlee)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [buildbot])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

7 years ago
Created attachment 449942 [details] [diff] [review]
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.
Attachment #449942 - Flags: feedback?(nrthomas)
Attachment #449942 - Flags: feedback?(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.
Attachment #449942 - Flags: feedback?(bhearsum) → feedback+
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().
Attachment #449942 - Flags: feedback?(nrthomas) → feedback+
(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.
(Assignee)

Comment 4

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

Comment 5

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

Small fix coming
Attachment #452715 - Attachment is obsolete: true
Attachment #452715 - Flags: review?(nrthomas)
Attachment #452715 - Flags: review?(bhearsum)
(Assignee)

Comment 6

7 years ago
Created attachment 452787 [details] [diff] [review]
Use the same revision for all nightly builds

Same as before, except don't create schedulers with empty builder lists.
Attachment #449942 - Attachment is obsolete: true
Attachment #452787 - Flags: review?(nrthomas)
Attachment #452787 - Flags: review?(bhearsum)
Blocks: 487036

Updated

7 years ago
Attachment #452787 - Flags: review?(nrthomas)
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!
Attachment #452787 - Flags: review?(bhearsum) → review+
(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.
(Assignee)

Updated

7 years ago
OS: Linux → All
Priority: P3 → P4
Hardware: x86 → All
Whiteboard: [buidlbot]
Whiteboard: [buidlbot] → [buildbot]
(Assignee)

Updated

7 years ago
Depends on: 607852

Updated

7 years ago
Blocks: 607812
(Assignee)

Comment 9

7 years ago
Created attachment 488573 [details] [diff] [review]
Use the same revision for all nightly builds...plus some extra fun
Attachment #452787 - Attachment is obsolete: true
Attachment #488573 - Flags: feedback?(dustin)
(Assignee)

Updated

7 years ago
Attachment #488573 - Flags: feedback?(bhearsum)
Attachment #488573 - Flags: feedback?(bhearsum) → feedback+
(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?
(Assignee)

Comment 11

7 years ago
(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.
+    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..
Attachment #488573 - Flags: feedback?(dustin) → feedback+
(Assignee)

Comment 13

6 years ago
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.
Attachment #488573 - Flags: review?(dustin)
Attachment #488573 - Flags: review?(bhearsum)
Attachment #488573 - Flags: review?(bhearsum) → review+
Nit: you've got empty lines at the end of test/test_misc_scheduler_propscheduler.py and test/test_misc_scheduler_propfuncs.py.
Comment on attachment 488573 [details] [diff] [review]
Use the same revision for all nightly builds...plus some extra fun

see comment #12
Attachment #488573 - Flags: review?(dustin) → review+
(Assignee)

Updated

6 years ago
Flags: needs-reconfig?
(Assignee)

Comment 16

6 years ago
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
Attachment #488573 - Flags: checked-in+
(Assignee)

Updated

6 years ago
Flags: needs-reconfig? → needs-reconfig+
(Assignee)

Comment 17

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

Comment 18

6 years ago
Looks like everything got triggered as expected!
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 594496
Depends on: 616708
(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.
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.