should have a way of running builds after X seconds of idle

[bhearsum]

Personally, I feel that the Periodic scheduler should behave this way by default. Either way, it's trivial to add this feature to it, or subclass it to an Idle scheduler.

I've got code for this, what do people prefer?

[bhearsum]

idle scheduler

[bhearsum]

Here's a possible implementation. I went with subclassing to avoid breaking existing installations. I'm doing real-world testing of it the next day or two.

[Pike]

It's be cool if "idle" really meant "not building anything". Getting a change doesn't really imply a build, and builds may not end at a specific time.

If we really did idle builders, that'd probably mean stopService on startBuildset and stopService on finishBuildset (or what the callbacks are called)

[bhearsum]

Yeah, that might be better. Could probably just subscribe a method of the scheduler to the build. I'll give this some more thought.

[dustin]

I'm *clearly* missing something, but how is this different from a regular Scheduler with a treeStableTimer?

[Pike]

Replying to dustin:

I'm *clearly* missing something, but how is this different from a regular Scheduler with a treeStableTimer?

treeStableTimer waits for n seconds to build a change set. IdleScheduler? builds every, say, 30 minutes if there's no triggered build.

One use case would be a performance test build, where, if you don't have new builds to test, you could gather more data on existing builds.

[bhearsum]

Replying to dustin:

I'm *clearly* missing something, but how is this different from a regular Scheduler with a treeStableTimer?

The patch I attached is an obviously silly implementation and probably no different than a regular Scheduler w/ treeStableTimer. Pike's last comment is an accurate description.

[Pike]

Should the scheduler be tied to a particular builder? Set of builders?

Might be more performant and compact to set up if one could have one IdleScheduler? for a bunch of related builds, but I'm not sure what idle would mean then. Ben, do we have a case where we'd actually would want to say "build all these builders when all of them had been idle for at least n secs"?

I guess the scheduler could then directly call into control.getBuilder(foo).requestBuildSoon(req) with some "idle force" BuildRequest?.

[bhearsum]

So, the more I delve into this the harder of a problem it becomes. My first attempt was obviously silly, so I went and augmented Periodic to try and do it. As it turns out, if you have more than one Builder for any given branch you end up needing 1 Scheduler + 1 Periodic per Builder, otherwise you get an incorrect amount of Idle timer. This is a very common use case for us, I don't know about others. I've been thinking that it may be easier/better to implement it as part of Builder. How do others feel about that?

[Pike]

I think it should be fairly easy to hook up a scheduler for a set of builders. Not that I tried.

I see two different scenarios:

a) whenever a single builder in a set is idle for n secs, force a build b) whenever all builders in a set have been idle for n secs, force a build on all

The back-end logic differs a bit. Anywho, the scheduler would subscribe to the BuilderStatus? objects for all its builders, and reset a timer, pretty much like treeStableTimer does, based on the logic (a vs b), and if the timer fires, it goes to the master, getBuilder(), and requestBuild with a fake BuildRequest?.

I wouldn't put that logic into the Builder itself, though.

[dustin]

This sounds like the problem needs to be broken into sub-problems. Buildbot could probably do with a more sophisticated notion of "busy" vs. "idle", so that might be the first patch. The second patch would then add scheduling atop that new notion. I'm just throwing out ideas here :)

[Pike]

Sounds to me like we're talking about partitions inside the master here. Group of builders, group of schedulers, group of changesources. Somewhat bundled together?

[bhearsum]

Replying to Pike:

I think it should be fairly easy to hook up a scheduler for a set of builders. Not that I tried.

Well, I have a patch that works if you have 1 change source for 1 builder. For me, that means I need one change source per platform instead of one. I think that's silly. It would be bad to have multiple instances of a ChangeSource polling the exact same location when only one is needed.

[bhearsum]

Replying to dustin:

This sounds like the problem needs to be broken into sub-problems. Buildbot could probably do with a more sophisticated notion of "busy" vs. "idle", so that might be the first patch. The second patch would then add scheduling atop that new notion. I'm just throwing out ideas here :)

Yeah, you're probably right.

Thanks for helping me iterate on this, both of you.

[bhearsum]

expose builder busyness in a simple way (w/ tests)

[Pike]

I'd adjust the doc string to not talk about slaves, the rest looks good to me.

I'm not exactly sure if ~isBusy should be treated as isIdle in the edgecase when there are no slaves for that builder currently running. But it might be actually good to catch that in the scheduler code.

[bhearsum]

Replying to Pike:

I'd adjust the doc string to not talk about slaves

Yeah, that makes sense.

I'm not exactly sure if ~isBusy should be treated as isIdle in the edgecase when there are no slaves for that builder currently running. But it might be actually good to catch that in the scheduler code.

I think that !isBusy in the Builder can be treated as isIdle. As you mention though, that case should be caught by whatever is triggering builds periodically.

[bhearsum]

(I really wish there was a way to mark a patch as obsolete.)

In a minute I'm going to attach a couple patches that solve this bug. They basically implement the TODO i found in the Periodic scheduler:

# TODO: consider having this watch another (changed-based) scheduler and # merely enforce a minimum time between builds.

I avoided modifying the Periodic scheduler because I didn't want to cause unexpected behaviour change for people. There's not any harm I can see in having them both.

[bhearsum]

add more types of watchers for Schedulers

[bhearsum]

Idle Scheduler, again + docs + tests

[bhearsum]

So, I realized that my patch to add more Scheduler watchers doesn't work so well in a couple ways. It considers "build start" to be the same as "build set submitted", which is silly. It also won't work as intended for Schedulers that trigger multiple builders. Back to square one =.

[bhearsum]

Okay, here we go again...incoming series of patches that approach this in a different, better way.

[bhearsum]

[v3] scheduler notification for buildsets

[bhearsum]

[v3] enable schedulers to use buildset notification

[bhearsum]

[v3] idle scheduler using buildset scheduler notification

[bhearsum]

I know this probably can't make 0.7.8 as it's a bit invasive, but if someone has few min to give me feedback that'd be great. I'm hoping to pre-land this in our Buildbot tree after 0.7.8.

[dustin]

Tracking this in git at

 http://repo.or.cz/w/buildbot.git?a=shortlog;h=refs/remotes/dustin/bug296

My comments:

• the notifyScheduler parameter should be described in some docstrings for the functions where it's available -- particularly in the interface. The one-oh work will hopefully have a more generic subscription system, so this may be a bit easier :)
• in the second patch, you mention an unknown reason that the unit tests fail. Can you figure that out, or post the error here for others to puzzle over? The unit tests are fairly invasive, and make a lot of hidden assumptions about the source, so it may be that we need to tweak them somehow.
• need some documentation

So not quite ready for review, but this looks pretty solid.

[dustin]

Also, I get the following failure:

===============================================================================
[ERROR]: buildbot.test.test_run.Idle2.testTwoBranches

Traceback (most recent call last):
  File "/home/dustin/devel/projects/buildbot/t/buildbot/buildbot/test/test_run.py", line 376, in _testTwoBranches_branch1_2
    self.failUnlessEqual(self.isBuilding(), 1)
twisted.trial.unittest.FailTest: not equal:
a = 0
b = 1

[bhearsum]

I'm really busy right now - I'm not going to have a chance to do anything about this before the release. I suspect that failure is due to the tests relying on timers, though.

[dustin]

Am I dreaming, or are we only moments away from a solution on this? Who's working on it? Where's it at? Have I missed a patch? Sorry to be a space cadet.

[bhearsum]

catlee has done recent work on this and has a solution that works in most cases IIRC

 http://hg.mozilla.org/build/buildbotcustom/file/tip/scheduler.py

Doesn't pay attention to Force Build/Rebuild? or changes builds from another Scheduler. Works great in a 1-Scheduler environment, though.

[dustin]

Ah, sorry, I was thinking of Claude Vittoria's onlyIfChanged addition to the Nightly scheduler.

I think the patches as they stand here are still a bit too single-purpose to merge, but I'm open to being convinced otherwise.

[bhearsum]

Yeah, you're probably right about that. We've solved this problem for our purposes so I won't be hacking on this anytime soon. Going to toss this back into the pool.

[dank]

I could use an idleScheduler to scrub trunk for flaky tests while I wait for try jobs to be submitted... running a scrub job every N minutes isn't optimal, either it doesn't build as often as it could, or it gets in the way of try jobs.

[dustin]

It's hard to detect "idle", which users will expect to be a slave-related concept, not builder-related. It's particularly hard to detect in a distributed sense - and I want to get away from implementing single-master features that break down in a multi-master context.

With a message-bus in place, it would be possible for the scheduler to schedule a build for a builder if it hadn't heard about any new build requests for that scheduler in N seconds, and there were no outstanding build requests for the builder at that time.