Opened 11 years ago

Last modified 7 years ago

#296 new enhancement

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

Reported by: bhearsum Owned by:
Priority: major Milestone: 0.9.+
Version: 0.7.7 Keywords:
Cc: Pike, dustin, catlee, daniel@…

Description

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?

Attachments (7)

idleScheduler-v2.diff (5.1 KB) - added by bhearsum 11 years ago.
idle scheduler
builder-busyness.diff (4.4 KB) - added by bhearsum 11 years ago.
expose builder busyness in a simple way (w/ tests)
moreSchedulerWatchers.diff (2.5 KB) - added by bhearsum 10 years ago.
add more types of watchers for Schedulers
idleScheduler-again.diff (10.4 KB) - added by bhearsum 10 years ago.
Idle Scheduler, again + docs + tests
notifySchedulers.diff (2.9 KB) - added by bhearsum 10 years ago.
[v3] scheduler notification for buildsets
schedulersUseNotify.diff (4.2 KB) - added by bhearsum 10 years ago.
[v3] enable schedulers to use buildset notification
idleScheduler.diff (8.2 KB) - added by bhearsum 10 years ago.
[v3] idle scheduler using buildset scheduler notification

Download all attachments as: .zip

Change History (39)

Changed 11 years ago by bhearsum

idle scheduler

comment:1 Changed 11 years ago by bhearsum

  • Owner set to bhearsum
  • Status changed from new to assigned

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.

comment:2 Changed 11 years ago by Pike

  • Cc Pike added

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)

comment:3 Changed 11 years ago by 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.

comment:4 follow-ups: Changed 11 years ago by dustin

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

comment:5 Changed 11 years ago by dustin

  • Cc dustin added

comment:6 in reply to: ↑ 4 Changed 11 years ago by 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.

comment:7 in reply to: ↑ 4 Changed 11 years ago by 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.

comment:8 Changed 11 years ago by 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?.

comment:9 Changed 11 years ago by 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?

comment:10 follow-up: Changed 11 years ago by 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.

comment:11 follow-up: Changed 11 years ago by 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 :)

comment:12 Changed 11 years ago by 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?

comment:13 in reply to: ↑ 10 Changed 11 years ago by 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.

comment:14 in reply to: ↑ 11 Changed 11 years ago by 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.

Changed 11 years ago by bhearsum

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

comment:15 follow-up: Changed 11 years ago by 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.

comment:16 in reply to: ↑ 15 Changed 11 years ago by 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.

comment:17 Changed 10 years ago by 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.

Changed 10 years ago by bhearsum

add more types of watchers for Schedulers

Changed 10 years ago by bhearsum

Idle Scheduler, again + docs + tests

comment:18 Changed 10 years ago by 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 =.

comment:19 Changed 10 years ago by bhearsum

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

Changed 10 years ago by bhearsum

[v3] scheduler notification for buildsets

Changed 10 years ago by bhearsum

[v3] enable schedulers to use buildset notification

Changed 10 years ago by bhearsum

[v3] idle scheduler using buildset scheduler notification

comment:20 Changed 10 years ago by 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.

comment:21 Changed 10 years ago by dustin

  • Keywords review added

comment:22 Changed 10 years ago by dustin

  • Keywords review removed
  • Milestone changed from undecided to 0.7.9

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.

comment:23 Changed 10 years ago by 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

comment:24 Changed 10 years ago by 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.

comment:25 Changed 10 years ago by dustin

  • Milestone changed from 0.8.0 to 0.7.10

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.

comment:26 Changed 10 years ago by bhearsum

  • Cc catlee added

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.

comment:27 Changed 10 years ago by 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.

comment:28 Changed 10 years ago by bhearsum

  • Owner bhearsum deleted
  • Status changed from assigned to new

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.

comment:29 Changed 10 years ago by dustin

  • Milestone changed from 0.7.10 to 0.7.+

comment:30 Changed 9 years ago by ddunbar

  • Cc daniel@… added

comment:31 Changed 7 years ago by 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.

comment:32 Changed 7 years ago by dustin

  • Milestone changed from 0.8.+ to 0.9.+

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.

Note: See TracTickets for help on using tickets.