570891 - Do not coalesce try server pushes

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[(no longer active)]

Coalescing builds and unit test runs on normal branches makes a lot of sense, but it's a major inconvenience on the try server.  I've had this happen a lot of times when I pushed something to the try server and checked back in a couple of hours only to see that someone else pushed something less than than three minutes after me and they stole my builds and unit test runs.

Let's not do this on try!

Comment 1

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

Pushes within 3 minutes of each other ? Or some other time period ?

Comment 2

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

We probably need to set treeStableTimer to 0 for try,
 http://hg.mozilla.org/build/buildbotcustom/file/default/misc.py#l519

Comment 3

[(no longer active)]

(In reply to comment #1)
> Pushes within 3 minutes of each other ? Or some other time period ?

I'm not sure if it's 3 minutes.  I think you actually told me on irc once that it's set to 3 minutes.  :-)

Comment 4

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

Where these builds combined ?

[e-mail]
Tue Jun 08 17:42:57 2010 -0700	ef4c8b473e8c	Brian Birtles — Bug 492458 - SVG SMIL: Implement backwards seeking - Part 1 -interval and instance time filtering 
...
[e-mail]
Tue Jun 08 17:40:59 2010 -0700	3e5095185cea	Ehsan Akhgari — Bug 569709 - Figure out the max nr of entries we should store in the disk cache 

That's most recent push from you that I can find.

Comment 5

[(no longer active)]

Well, actually it wasn't.  Tinderbox is so slow right now that I can't go back in history to find an example...

Comment 6

[Armen [:armenzg]]

Ehsan is this an example of what you are hitting?
The two changes were pushed at the same time "Wed 09 Jun 2010 14:36:00"

This doesn't sound to me as the expected behaviour but I could be wrong.

2 builds. 1 talos job with both revisions (it downloaded the build of 27d8113d68f7).
http://production-master02.build.mozilla.org:8011/builders/WINNT%205.2%20tryserver%20build/builds/546
http://production-master02.build.mozilla.org:8011/builders/WINNT%205.2%20tryserver%20build/builds/547
http://talos-master02.build.mozilla.org:8012/builders/Rev3%20WINNT%205.1%20tryserver%20talos%20tp4/builds/437

For reference:
http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-27d8113d68f7
http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-900afc7662ee

Comment 7

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: change no-merge scheduler to have None for treeStableTimer

did a checkconfig on a local master instance to make sure this passes. Setting the treeStableTimer to None will make sure that each push creates its own change.

Comment 8

[(no longer active)]

(In reply to comment #6)
> Ehsan is this an example of what you are hitting?

Yes, I guess so...

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

My patch doesn't address that talos issue though. generateTalosBranchObjects is merging those builds, which is strange.  Seems like two different issues though.

Comment 10

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: turn off queue merging for tryserver talos

so this will handle the second issue - the merging of builds in talos.

Comment 11

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

Comment on attachment 450466 [details] [diff] [review]
change no-merge scheduler to have None for treeStableTimer

>diff --git a/misc.py b/misc.py
>+        extra_args['treeStableTimer'] = None

I think this is going to give us a build for each changeset rather than push; we set 3 seconds on the old try. If testing proves me wrong then go ahead and ask for review from bhearsum/me as timezones dictate.

Comment 12

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: change no-merge scheduler to have 3 seconds for treeStableTimer

You're right - None leads to a change per changeset, not per push.  Knocking this down to 3 seconds to match the way old try did it.

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 450485 [details] [diff] [review]
turn off queue merging for tryserver talos

http://hg.mozilla.org/build/buildbot-configs/rev/b62ced51792a

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 450763 [details] [diff] [review]
change no-merge scheduler to have 3 seconds for treeStableTimer

http://hg.mozilla.org/build/buildbotcustom/rev/ee4a6ca2e197

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Reconfig'd try master to pick up the new treeStableTimer.

Comment 16

[Joe Drew (not getting mail)]

This just happened to me on a try push (867c3741e16d) - it got coalesced with the push right after it. Maybe this didn't get propagated to buildbot 0.8.0?

Comment 17

[Chris AtLee [:catlee]]

This was missed originally, and I landed a fix...but didn't update the masters :(

I've updated the master (pm01/scheduler_master), so this shouldn't happen any more.

Please reopen if that's not the case!

Comment 18

[Joe Drew (not getting mail)]

Can we add an automated test for this? It hurt a lot :(

Comment 19

[Chris AtLee [:catlee]]

(In reply to comment #12)
> Created an attachment (id=450763) [details]
> change no-merge scheduler to have 3 seconds for treeStableTimer
> 
> You're right - None leads to a change per changeset, not per push.  Knocking
> this down to 3 seconds to match the way old try did it.

How were you getting a change per changeset?  Doesn't the hg poller only give us tips?

Just caught an instance of two tests arriving at the same time, so < 3 seconds, so merged.  Need to work harder on fixing this!

Comment 20

[Chris AtLee [:catlee]]

(In reply to comment #18)
> Can we add an automated test for this? It hurt a lot :(

I added a check that will e-mail us when it detects jobs that have != changes in them.

Comment 21

[Chris AtLee [:catlee]]

(In reply to comment #20)
> (In reply to comment #18)
> > Can we add an automated test for this? It hurt a lot :(
> 
> I added a check that will e-mail us when it detects jobs that have != changes
> in them.

I mean, != 1 change in them.

Comment 22

[(no longer active)]

It also happened to me once after the patches landed in this bug.  My changeset c00bbd837385 was coalesced into the next changeset I pushed: d1653821d023.

Comment 23

[Chris AtLee [:catlee]]

(In reply to comment #22)
> It also happened to me once after the patches landed in this bug.  My changeset
> c00bbd837385 was coalesced into the next changeset I pushed: d1653821d023.

Those happened prior to me landing the changes on the master at ~19:00.

Comment 24

[Chris AtLee [:catlee]]

Attachment: set treeStableTimer=None

Lukas' test of treeStableTimer=None was done on buildbot 0.7.10, which didn't support treeStableTimer=None.  Scheduler.addChange was raising exceptions, and so the poller kept submitting the same change over and over since it wasn't saving its last seen revision.

I've tested it with buildbot 0.8.0 and it looks like it works fine.

Comment 25

[Chris AtLee [:catlee]]

Comment on attachment 452293 [details] [diff] [review]
set treeStableTimer=None

773:d0f4f41ac108

Comment 26

[Chris AtLee [:catlee]]

pm02/try-trunk-master and pm01/scheduler_master updated and reconfigured

Comment 27

[Chris AtLee [:catlee]]

I think this is fixed now.