Opened 7 years ago

Last modified 6 years ago

#2309 new defect

When 'nextBuild' picks something other than the oldest build, build request merging results in incorrect revision number being used for the build

Reported by: bdash Owned by:
Priority: major Milestone: 0.9.+
Version: 0.8.6p1 Keywords:
Cc:

Description

On the WebKit Buildbot we recently altered our configuration to prioritize the newest pending build request by using a nextBuild function on the builder configuration. However, we have noticed that this causes Buildbot to build the wrong revision if merging of build requests is enabled for that builder. Such an example of the incorrect building can be seen here. In this instance, change number 430 (SVN r119458) was selected by our pickLatestBuild function due to being the most recent pending build at the time. Note however that the source stamp refers to SVN r119457, as do the commands that are later invoked. This appears to be due to the manner in which the merging of build requests occurs. It seems as though all eligible requests are merged in to the request that the nextBuild function returns. This results in the changes being listed out of order on the build page (430, 428, 429 in the example above). Code elsewhere in Buildbot appears to look only at the last change in the build request when determining the SVN revision to work with. This results in it picking the SVN revision from change number 429 (SVN r119457) rather than using the most recent SVN revision from the set of changes associated with the build.

Change History (9)

comment:1 Changed 7 years ago by tom.prince

One difficulty with this is that buildbot doesn't really have any notion of ordering of changes. For svn, it is fairly obvious how to do this, but for something like git, none of the meta-data we capture allows us to order changes.

So, I'm not sure how best we should fix this. A work around would be to disable merging of build requests.

comment:2 Changed 7 years ago by bdash

The Change instances are numbered and have timestamps associated with them. Surely one of those two pieces of information could be used to inform the ordering?

comment:3 Changed 7 years ago by tom.prince

We have no guarantee that either of those actually represent the order of commits. We could use either of those as a stop-gap measure. But that would depend on changes being added to the db in order.

comment:4 Changed 7 years ago by bdash

Perhaps the presence of a non-default nextBuild function should disable merging of build requests then, since claiming to build rN while actually building some older revision is a pretty nasty situation to hit.

comment:5 Changed 7 years ago by tom.prince

As long as the nextBuild function only differs from the default by sometimes returning requests without changes (which only get merged with requests with matching versions), then this bug wont be triggered.

To be clear: I think the current behavior is buggy. I'd like to fix it properly, rather than just with a stopgap.

comment:6 Changed 7 years ago by dustin

We already assume that changes are supplied in order when merging changes, so continuing to assume that here would not be a regression.

comment:7 Changed 7 years ago by dustin

  • Milestone changed from undecided to 0.8.+

comment:8 Changed 7 years ago by dustin

tomprince, I think #2249 was the other bug you were thinking of. Unfortunately, it's at a dead-end.

comment:9 Changed 6 years ago by dustin

  • Milestone changed from 0.8.+ to 0.9.+

Nine will switch to collapsing build queues, rather than merging. Meaning that whenever a new build request is scheduled, it may "collapse" an older build request immediately. There's still a risk of similar problems, but only two builds to look at at any one time.

Note: See TracTickets for help on using tickets.