Builder page for luci CI bots is missing changes count |
|||||
Issue descriptionThe "Changes" column in the build page is empty. See: https://luci-milo.appspot.com/p/chromium/builders/luci.chromium.ci/Android%20arm64%20Builder%20(dbg) Its buildbot version has it: https://luci-milo.appspot.com/buildbot/chromium.android/Android%20arm64%20Builder%20%28dbg%29/
,
Feb 1 2018
+iannucci, the blamelist lord
,
Feb 3 2018
,
Feb 9 2018
So I thought about this a bit more. This is tricky because buildbot and LUCI deal with blamelists differently. Buildbot: * poll repo * gather changes * associate changes into a changelist * fire a build with a changelist * use associated changelist to show console/blamelist/'changes' column LUCI * poll repo * fire a build with a commit * when displaying commit-oriented views (like console), query git for a chunk of CLs, then find all builds matching those CLs. The key difference is that LUCI allows bisection/backfilling due to this design; adding a new build with an old commit will simply start displaying 'in between' the two commits that it bisects. In buildbot this is not possible (since the changes have been 'used up' by the previously fired builds). In addition force/rebuilds on buildbot will have NO associated changes and NO associated blamelist. On LUCI, they will all have the same blamelist (since lists of commits are dynamically associated with the build instead of statically). That leads us to the builder view. The builder view shows builds ordered by TIME (not by commit). While it would potentially be possible to add changes into this view, it would be tricky to do. IMO, we should remove the changes column from the LUCI builder view, since this view is time oriented and not commit oriented. If this feature is really, truly needed (i.e. it helps people do some workflow), then I can definitely implement it.. if it's just for visual parity with buildbot, then I'm inclined to remove the column. What do folks think?
,
Feb 10 2018
Speaking as someone that has no business knowing/caring about the implementation, the changes column is nice to have. At the time of filing, I think I was trying to answer the question "what kind of effect does length-of-blamelist have on build duration?" Having that column filled out made it trivial. Additionally, it can give you a good impression on roughly how many changes a CI bot picks up every cycle. If I were sheriffing/monitoring a waterfall, that would be nice info to have. That said, if the solution is too difficult, feel free to wontfix it. (Just know that buildbot will have won over luci in at least one aspect :P )
,
Feb 10 2018
ah ha! that is the great lie :D That sort of determination only works if the pool has /exactly/ one build, AND there are never any force builds/retries.
,
Feb 10 2018
That happens to be how our luci builder are configured right at the moment. The cost will come when we want to move beyond that.
,
Feb 10 2018
More precisely, the cost will be realized when we want to move beyond that*
,
Feb 10 2018
So I think a different (potentially better) way to do this sort of analysis is to have the bot report (as a source manifest? :)) the state of its checkout BEFORE it does the bot_update step. Then you can actually get a datapoint which is `<commitA>..<commitB> && compile takes XX minutes`. I'm going to remove the column for now.
,
Feb 10 2018
+1 to c#4 and removing column Ben, doesn’t console view solved your problem? It is the view ordered by commit. We could potentially add a radio button for order (time or commit) to the builder view. If commit, then do what console does, essentially.
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/luci/luci-go.git/+/ba2998b2e20f8634ee551aaac497de51eec1d6ff commit ba2998b2e20f8634ee551aaac497de51eec1d6ff Author: Robert Iannucci <iannucci@chromium.org> Date: Mon Feb 12 19:29:49 2018 [milo] Remove blank Changes column from buildbucket builder pages. We can add it back if we determine that there's a need for this (and a meaningful implementation). In Buildbot, changes are directly associated with Build objects. Retries and forced builds have no 'changes' associated with them. In Buildbucket, builds record the fact of "I built revision X", and then milo dynamically computes the blamelist between a build and the previous build for that builder. This is done in order to support backfilling/bisection on main waterfall builders at a later time. However, due to this design, meaningful display of a changelist on this view is more difficult than it is for buildbot. Coupled with the fact that the primary use case for this information is also less relevant than it was for Buildbot (namely; the implication that the changelist here correlates with the incremental build that the bot did. On Swarming, there can be multiple bots in the pool, the cached build can be removed between tasks, force/retry builds won't be in commit order, etc.) Diagnosing this sort of thing on the bot (speed of incremental builds) will be better served by reporting the 'currently checked out version' from the bot before the build. Then we'll know the actual starting revision, the checked out revision, and the duration of the build. R=hinoka@chromium.org, jchinlee@chromium.org Bug: 807846 Change-Id: I04727eb8487653a01deadd3e1b863082a2b863ef Reviewed-on: https://chromium-review.googlesource.com/912564 Reviewed-by: Jao-ke Chin-Lee <jchinlee@chromium.org> Commit-Queue: Robbie Iannucci <iannucci@chromium.org> [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/buildsource/buildbucket/expectations/master.tryserver.infra/InfraPresubmit.Swarming.json [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/buildsource/builder.go [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/frontend/appengine/templates/pages/builder.html [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/frontend/expectations/buildbot.builder-Basic_Test_no_builds.html [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/frontend/expectations/buildbot.builder-Basic_Test_with_builds.html [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/frontend/routes_test.go [modify] https://crrev.com/ba2998b2e20f8634ee551aaac497de51eec1d6ff/milo/frontend/ui/builder.go
,
Feb 12 2018
Fixed for now. When we're fully migrated (or at least more-migrated), it would be great to sit down and make a proper requirements list for our UI so we can design it to match user workflows. e.g. if we had monitoring for #commits v. incremental build time, I think that would be a better way to measure this effect than relying on the UI.
,
Feb 28 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by hinoka@chromium.org
, Feb 1 2018