Add warning in trigger step if collapseBuild=True for the target builder

[tardyp]

This ticket is a migrated Trac ticket 3529

People contributed to the original ticket: @rutsky, @tardyp
Ticket created on: Apr 19 2016
Ticket last modified on: Jun 07 2016

---

as per discussion

@djmitche> #topic collapse[[BuildRequest]]=False by default
18:41 <@djmitche> now, what WAS the topic
18:42 <@tardyp> collapse[[BuildRequest]]=True right now
18:42 <@djmitche> I shouldn't be allowed on irc
18:42 <@tardyp> and this does not play well with triggered build
18:42 <@tardyp> So people which use trigger step will all have the problem (I did 4 years ago)
18:42 <@tardyp> and ed did this week
18:43 @djmitche set the topic: A Software Freedom Conservancy Project | Buildbot-0.8.12 | docs: http://docs.buildbot.net/current/ | tutorial: http://docs.buildbot.net/current/tutorial | http://irclogs.jackgrigg.com/i[]node.net/buildbot | meetings: Tuesdays 1630 UTC / https://titanpad.com/buildbot-agenda
18:43 <@tardyp> I would recommend to put it False by default, and to let people set it to true when they need to optimize
18:43  @jaredgrubb quit (~Adium@76.74.153.36) Quit: Leaving.
18:43 <@djmitche> changing a default is pretty ugly though
18:44 @djmitche set the topic: A Software Freedom Conservancy Project | Buildbot-0.8.12 | docs: http://docs.buildbot.net/current/ | tutorial: http://docs.buildbot.net/current/tutorial | https://irclogs.jackgrigg.com/irc.freenode.net/buildbot | meetings: Tuesdays 1630 UTC / https://titanpad.com/buildbot-agenda
18:44  bb-github joined (~bb-github@192.30.252.42)
18:44 <bb-github> [buildbot] mention-bot commented on issue #2141: By analyzing the blame information on this pull request, we identified @tomprince, @djmitche and @rutsky to be potential reviewers https://git.io/vwsl3
18:44  bb-github left (~bb-github@192.30.252.42)
18:44 <@djmitche> is it possible to just disable it with triggered builds?
18:45 <@djmitche> my worry is, changing the default is pretty subtle, and will change behavior even for people not using triggered builds
18:45 <@tardyp> I tried to fix it that way, but there is no real way to be sure that a build is comming from a trigger step (from within the colapse br)
18:45 <cmouse> tardyp: i'm using the buildbot reporter myself, and it mostly works
18:45 <@tardyp> it will change the optimisation
18:45 <cmouse> tardyp: the only thing that is not working is that it's not reporting start of build
18:45 <@tomprince> The trigger scheduler could annotate the BR.
18:45 <cmouse> tardyp: i have no idea why it's not doing it :(
18:45 <@tardyp> while for people using trigger step, it just skip builds
18:46 <@djmitche> ^^ even as a temporary hack, I think that makes sense
18:46 <myheadhurts> So I'm running into the following issue: http://trac.buildbot.net/ticket/3528 and I'm thinking of approaching the fix in the following way: https://github.com/buildbot/buildbot/pull/2141
18:46 <myheadhurts> Would love any feedback on the approach
18:47 <@tardyp> ok. So we change the True behaviour to : compatible sourcestamps *and* not coming from a Trigger step
18:47 <@djmitche> #info build request collapsing and triggered builds do not get along well -- triggered builds end up skipped
18:47 <@tardyp> I'm fine with that too.
18:47 <@djmitche> tardyp: I think so -- that's even a good medium-term fix (since it sounds like this is architectural, not just a bug)
18:47  poz2k4444 joined (uid22701@gateway/web/irccloud.com/x-rugnxioslzukcfmd)
18:47 <bb-trac> [trac] #3528/undecided (new) updated by rutsky (Related PR: https://github.com/buildbot/buildbot/pull/2141) http://trac.buildbot.net/ticket/3528
18:48 <@djmitche> #agreed will configure collapsing to skip triggered builds by annotating the triggered builds
18:48 <@tomprince> If people want collapsing builds, then it isn't clear that skipping triggered builds isn't desired behavior.
18:48 <@djmitche> myheadhurts: without looking deeply at the PR, I'm really happy to see you working on the latent worker support :)
18:48 <@bdbaddog> so if you have multiple triggered builds waiting, they would no longer be collapsed?
18:48 <@tardyp> tomprince: I dont see a usecase where people would like to skip triggerred build
18:49 <@djmitche> a skipped triggered build has weird semantics if wait[[ForFinish]]=True
18:49 <@djmitche> or whatever that parameter is
18:49 <@bdbaddog> yes.. deadlock..
18:49 <@tardyp> well no deadlock
18:49 <@tomprince> If you have a multi-stage build pipeline, but only care about results on the latest commit, if several triggers happen before the triggered build runs, there isn't a reason to run the earlier triggers.
18:49 <myheadhurts> djmitche: Hopefully soon (once company approves it) I'll have a PR for upgrading boto2 -> boto3 and handling cross-account support for AWS. :)
18:50 <@djmitche> oo, very nice!
18:50 <@djmitche> so regarding collapsing and triggering -- it sounds like this is something we should defer until 0.9.x
18:50 <@bdbaddog> parent build would hang forever if triggered build gets skipped..
18:50 <@tomprince> Probably doesn't make sense in the wait[[ForFinish]] case (at least when that is being used for synchronization not just reporting).
18:50 <@djmitche> right
18:50 <@tomprince> Doesn
18:51 <@tomprince> t it just wait for the buildrequest to be completed, which does happen for collapsed requests? 
18:51  @jaredgrubb (opped) joined  
18:51 <@tardyp> yes
18:51  @tomprince can't remember the code, but seems to recall that was how things worked
18:51 <@tardyp> you will just see 4 builds when you awaited 8
18:52 <@tardyp> because the colapsing dont create build, it directly skips the br
18:52 <@tardyp> and that is fine with the waiting logic in trigger step, as it only waits for a complete event of the br
18:53  bb-github joined (~bb-github@192.30.252.41)
18:53 <bb-github> [buildbot] tomprince commented on issue #2141: I don't think this a correct fix. I think the intent of `build_wait_timeout == 0` is that the worker will only run a single build. I don't think this change preserves that behavior. https://git.io/vws81
18:53  bb-github left (~bb-github@192.30.252.41)
18:54 <@tardyp> it actually waits for the buildset
18:54 <@djmitche> so how badly broken is it right now?
18:54 <@djmitche> It sounds like it works, it's just not the semantics you're expecting
18:54 <@djmitche> as in, you're expecting every triggered build to actually occur, and instead they get collapsed
18:55 <@tardyp> well you start 8 builds with trigger, you have no idea what collapsing is, and eventually you get 4 builds, and buildbot seems happy
18:55 <@tardyp> that is for me very weird for a new commer
18:55 <@tardyp> build request collapsing should for me be an advanced feature, that new commers should not be aware of
18:56 <@tomprince> I think that not collapsing is probably a slightly better default, but it isn't clear wether it is enough better to warrant *changing* the default.
18:56 <@djmitche> right
18:56 <@djmitche> we're already changing so much other stuff
18:56 <@djmitche> I think we should leave this be for now
18:56 <@tomprince> Given the experience of python 3, that seems reasonable.
18:57 <@tomprince> If you want to change the default, a slightly better option might be to deprecate having a default at all, and then remove the default.
18:57 <rutsky> can we somehow warn user that he might get not what he expected?
18:57 <rutsky> or add appropriate warning in docs?
18:57 <@tardyp> that is a good idea
18:57 <@tomprince> That seems sensible.
18:58  bb-github joined (~bb-github@192.30.252.41)
18:58 <bb-github> [buildbot] aelsabbahy commented on issue #2141: Hmm, that's definitely not what I want then. Since it's very important for us that we only have single use workers.... https://git.io/vws4o
18:58  bb-github left (~bb-github@192.30.252.41)
18:58 <@tardyp> in the trigger step, if the triggered builder has True in collapse, we warn in the stdio
18:58 <@djmitche> that sounds good
18:58 <cmouse> should write a trac issue about [[OpenStackLatentWorker]] not always killing the worker.
18:58 <@tardyp> with link to the doc

---