Lazy loading build records

[Kohsuke Kawaguchi]

As any serious Jenkins users would know, a big Jenkins instance takes a
long time to start up. One of the problems here is that it loads every
build record before it starts accepting HTTP requests, and therefore a
natural solution to this problem is to try to lazy load build records
on-demand.

I've been working on this in and out, but I finally got to a milestone.
And incidentally I got contacted by Christian Wolfgang about this, as
this very topic was discussed in the Copenhagen hackathon [3].

My changes are in the lazy-load branch [1]. You can also see the commits
made in this branch [2] that aren't in master.



What's already done
===================

RunMap is conceptually a map from build number to Run, and Runs form a
bi-directional linked list between their adjacent neighbors. So the
first step was to turn RunMap into a virtual map. That is, it loads each
build records on-demand as requested. Ditto to the bi-directional
linking between Runs.

What makes this interesting is that build records are keyed by their
timestamps on disk, not by numbers (and numbers are available as
symlinks but not in all the platforms.) So RunMap uses a binary search
to locate the build that's currently needed. For this to work, I made an
assumption that for build #M and #N. M>N iff M.timestamp>N.timestamp.
This process also incorporates symlinks as a hint to speed up the search
if they are available.

Much of this logic is in AbstractLazyLoadRunMap, which has no
dependencies to the rest of Jenkins core. This was so that I can test
this class quickly and efficiently without using HudsonTestCase (and
it's because historically I developed this code outside core.) See its
search method that is the heart of this logic.

RunMap was then modified to inherit this class to provide this necessary
behaviour.


The next step was to make Job/AbstractProject take advantages of this.
The Job class expects subtypes to provide the _getRuns method that
returns SortedMap of the builds, and all the other methods on Job that
deal with obtaining build records work on this map. Some of these
methods are overriden in AbstractProject to take advantages of RunMap.


I then proceeded to update RunList, which is a class that's used to list
up builds that satisfy a certain criteria. In the master, this class
extends from ArrayList and every time a new filter is applied, all
builds that satisfy the criteria gets copied into this array. This
doesn't work very well with lazy loading, so I modified this class to
only lazily walk through the builds and pick up ones that satisfy the
criteria.

For example, a typical use case of this class is "take all the builds of
a job, narrow it down those that have failed, then list up first 10,
render it to RSS". The new lazy implementation works very well with this.

Unfortunately, to make this work, I needed to make a signature breaking
change --- the class now extends from AbstractList and not from
ArrayList. I did scan the source code of all the plugins to see their
use of RunList, and I didn't spot anywhere it's casted to ArrayList, so
I think the impact of this would be small.


And at this point, it passes all the unit tests.



What needs to be done
=====================
Clearly more testing needs to be done. Code coverage of
AbstractLazyLoadRunMap is actually pretty good, but the logic is complex.

I also need to try this with a real Jenkins instance, mainly to see if
there's some code in Jenkins or plugins that tries to eagerly load all
the build records of all the jobs.

To help us find this, we probably need some logging that tells us who's
loading build records when.

I'll try this with some real Jenkins deployments, and when that's ready,
I'd like to merge this to the master. If anyone is willing to give this
a shot before it hits the master, I'd highly appreciate that.



The next goal after that is to deal with places where someone tries to
load all the build records of one job. Unfortunately, this happens today
in numerous places. Just in the job top page, test report trend or code
coverage trend will cover the entire build history. So this will be a
slow process. More about this later.



WDYT?


[1] http://github.com/jenkinsci/jenkins/tree/lazy-load
[2] https://github.com/jenkinsci/jenkins/compare/master...lazy-load
[3] http://wiki.praqma.net/jcicodecamp12/sessions/improve-start-up-time
--
Kohsuke Kawaguchi | CloudBees, Inc. | http://cloudbees.com/
Try Nectar, our professional version of Jenkins

[Arnaud Héritier]

Thx a lot KK.

It's a great improvement to come.

As I reported this issue also a long long time ago I'll be happy to test it.

My new infrastructure is using the .deb of jenkins.

How can I build it from your branch to give it a try ?

Arnaud

--

-----

Arnaud Héritier

06-89-76-64-24

http://aheritier.net

Mail/GTalk: aher...@gmail.com

Twitter/Skype : aheritier

[Emanuele Zattin]

Great job Kohsuke! I'll try and get somebody in Nokia to test this if possible.

Emanuele Zattin
---------------------------------------------------
-I don't have to know an answer. I don't feel frightened by not knowing things; by being lost in a mysterious universe without any purpose — which is the way it really is, as far as I can tell, possibly. It doesn't frighten me.- Richard Feynman

[Jesse Glick]

On 09/20/2012 03:16 PM, Kohsuke Kawaguchi wrote:
> numbers are available as symlinks but not in all the platforms

By the way those platforms should be significantly expanded as of 646154f.

> the class now extends from AbstractList and not from ArrayList

Safer to not have publicly accessible classes extend anything at all: define a factory method returning List or some interface extending List, or do not directly
implement List but provide a List asList() adapter method. Too late for this case of course but may be a useful habit for the future.

> if there's some code in Jenkins or plugins that tries to eagerly load all the build records of all the jobs

Such as Jenkins.getPeople?

> [2] https://github.com/jenkinsci/jenkins/compare/master...lazy-load

Missing @since on various newly added members.

dir = File.createTempFile("lazyload","test") could probably use Util.createTempDir, but ideally there would be a mockable interface for the list of build directories to
begin with.

(By the way the reason I would do this sort of change in a forked repository and file a pull request—even when permissions would allow me to just use a branch in
jenkinsci/jenkins—is so it is possible to comment on individual lines in the aggregate diff: GitHub only allows line comments on single commits and pull requests, not
general branch comparisons. Also a to-do list could live in the pull request’s description rather than a note.txt in the branch.)

[Kohsuke Kawaguchi]

On 09/20/2012 01:13 PM, Jesse Glick wrote:
> On 09/20/2012 03:16 PM, Kohsuke Kawaguchi wrote:
>> numbers are available as symlinks but not in all the platforms
>
> By the way those platforms should be significantly expanded as of 646154f.

Right.

>> the class now extends from AbstractList and not from ArrayList
>
> Safer to not have publicly accessible classes extend anything at all: define a factory method returning List or some interface extending List, or do not directly
> implement List but provide a List asList() adapter method. Too late for this case of course but may be a useful habit for the future.

Yes.

>> if there's some code in Jenkins or plugins that tries to eagerly load all the build records of all the jobs
>
> Such as Jenkins.getPeople?

Yes. I haven't thought of this too much, but I'm hoping that if we make
them look at just recent builds, instead of all, that would be an OK
compromise.

>> [2] https://github.com/jenkinsci/jenkins/compare/master...lazy-load
>
> Missing @since on various newly added members.

I'll add "@since 1.LAZYLOAD"

> dir = File.createTempFile("lazyload","test") could probably use Util.createTempDir, but ideally there would be a mockable interface for the list of build directories to
> begin with.
>
> (By the way the reason I would do this sort of change in a forked repository and file a pull request—even when permissions would allow me to just use a branch in
> jenkinsci/jenkins—is so it is possible to comment on individual lines in the aggregate diff: GitHub only allows line comments on single commits and pull requests, not
> general branch comparisons. Also a to-do list could live in the pull request’s description rather than a note.txt in the branch.)

We can create a pull request from one branch to another, so if that's
the only thing, I'd encourage you to create a branch. It lets others
make changes, and other people can see what's being worked on.

Wrt line comment on aggregated diff, I don't know what will happen when
more commits are added --- I suspect they'll get lost.

Feel free to make changes and push into the lazy-load branch. I'm now
running a real instance and fixing bugs, so I won't be touching anything
cosmetic.

[Kohsuke Kawaguchi]

http://kohsuke.org/private/20120920/jenkins.war

I've deployed this exact binary on https://ci.jenkins-ci.org/

I just set up https://ci.jenkins-ci.org/job/jenkins_lazy_load/ so this
should provide up-to-date build.

On 09/20/2012 12:28 PM, Emanuele Zattin wrote:
> Great job Kohsuke! I'll try and get somebody in Nokia to test this if possible.
>
> Emanuele Zattin
> ---------------------------------------------------
> -I don't have to know an answer. I don't feel frightened by not knowing things;

> by being lost in a mysterious universe without any purpose � which is the way it

> [1] http://github.com/jenkinsci/__jenkins/tree/lazy-load
> <http://github.com/jenkinsci/jenkins/tree/lazy-load>
> [2] https://github.com/jenkinsci/__jenkins/compare/master...lazy-__load
> <https://github.com/jenkinsci/jenkins/compare/master...lazy-load>
> [3]
> http://wiki.praqma.net/__jcicodecamp12/sessions/__improve-start-up-time

> <http://wiki.praqma.net/jcicodecamp12/sessions/improve-start-up-time>
> --
> Kohsuke Kawaguchi | CloudBees, Inc. | http://cloudbees.com/
> Try Nectar, our professional version of Jenkins
>
>
>
>
> --
> -----

> Arnaud H�ritier
> 06-89-76-64-24
> http://aheritier.net
> Mail/GTalk: aher...@gmail.com <mailto:aher...@gmail.com>
> Twitter/Skype : aheritier

[Christian Wolfgang]

Awesome! Thanks, Kohsuke!

I will test it as soon as possible.

Christian

On Fri, Sep 21, 2012 at 1:38 AM, Kohsuke Kawaguchi
<kkawa...@cloudbees.com> wrote:
>
> http://kohsuke.org/private/20120920/jenkins.war
>
> I've deployed this exact binary on https://ci.jenkins-ci.org/
>
> I just set up https://ci.jenkins-ci.org/job/jenkins_lazy_load/ so this
> should provide up-to-date build.
>
>
> On 09/20/2012 12:28 PM, Emanuele Zattin wrote:
>>
>> Great job Kohsuke! I'll try and get somebody in Nokia to test this if
>> possible.
>>
>> Emanuele Zattin
>> ---------------------------------------------------
>> -I don't have to know an answer. I don't feel frightened by not knowing
>> things;

>> by being lost in a mysterious universe without any purpose — which is the

>> way it
>> really is, as far as I can tell, possibly. It doesn't frighten me.-
>> Richard Feynman
>>
>>

>> On Thu, Sep 20, 2012 at 9:22 PM, Arnaud Héritier <aher...@gmail.com

>> Arnaud Héritier

[Baptiste MATHUS]

That's an absolutely great news! Congrats again Kohsuke.

Seeing this makes me think about a side-feature. 

Lazy-loading build records is great when it comes to start the server, but might we consider its unloading somehow? 

For example, on our instance (~10 nodes, 250 jobs), we've encountered some memory issues.

We didn't investigate that very far, but couldn't we consider using some kind of WeakHashMap to handle build results? 

This way, the GC might be able to regain memory by freeing some build results accessed not so often. 

And it would eventually get naturally reloaded with that great new lazy-loading feature when needed.

What do you think?

Cheers

PS : Please excuse me if this sounds stupid according to things I would have missed about the current behaviour. Just let know if so.

2012/9/21 Kohsuke Kawaguchi <kkawa...@cloudbees.com>

http://kohsuke.org/private/20120920/jenkins.war

I've deployed this exact binary on https://ci.jenkins-ci.org/

I just set up https://ci.jenkins-ci.org/job/jenkins_lazy_load/ so this should provide up-to-date build.

On 09/20/2012 12:28 PM, Emanuele Zattin wrote:

Great job Kohsuke! I'll try and get somebody in Nokia to test this if possible.

Emanuele Zattin
---------------------------------------------------
-I don't have to know an answer. I don't feel frightened by not knowing things;

by being lost in a mysterious universe without any purpose — which is the way it

really is, as far as I can tell, possibly. It doesn't frighten me.- Richard Feynman

On Thu, Sep 20, 2012 at 9:22 PM, Arnaud Héritier <aher...@gmail.com

<mailto:aher...@gmail.com>> wrote:

     Thx a lot KK.

     It's a great improvement to come.

     As I reported this issue also a long long time ago I'll be happy to test it.
     My new infrastructure is using the .deb of jenkins.
     How can I build it from your branch to give it a try ?

     Arnaud


     On Thu, Sep 20, 2012 at 9:16 PM, Kohsuke Kawaguchi <kkawa...@cloudbees.com

     Arnaud Héritier
     06-89-76-64-24
     http://aheritier.net

     Mail/GTalk: aher...@gmail.com <mailto:aher...@gmail.com>
     Twitter/Skype : aheritier

--
Kohsuke Kawaguchi | CloudBees, Inc. | http://cloudbees.com/
Try Nectar, our professional version of Jenkins

--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !
nbsp;!

[Vincent Latombe]

Hello,

I agree with Baptiste, it would be great to also allow unloading of builds, this would reduce the overall heap usage and maybe allow to keep more builds per job.

Vincent

2012/9/21 Baptiste MATHUS <bma...@gmail.com>

[Jesse Glick]

On 09/20/2012 05:20 PM, Kohsuke Kawaguchi wrote:
> We can create a pull request from one branch to another, so if that's the only thing, I'd encourage you to create a branch.

No, my point was really that it is hard to _comment_ on changes made in a branch which is not a pull request. You have to either use the mailing list, which is awkward;
or look up the commit that introduced that line and comment on it, which is a pain, and will not work well if and when the comment is addressed.

> Wrt line comment on aggregated diff, I don't know what will happen when more commits are added --- I suspect they'll get lost.

No, they continue to be displayed in the diff tab unless and until a commit changes the lines that were commented on—at which point the discussion tab of the pull request
shows that there were some obsolete comments at a particular point. This makes it easy for the author of the pull request to see at a glance all the details that might
still need to be addressed according to reviewers.

[Kohsuke Kawaguchi]

On 09/21/2012 10:20 AM, Jesse Glick wrote:
> On 09/20/2012 05:20 PM, Kohsuke Kawaguchi wrote:
>> We can create a pull request from one branch to another, so if that's the only thing, I'd encourage you to create a branch.
>
> No, my point was really that it is hard to _comment_ on changes made in a branch which is not a pull request. You have to either use the mailing list, which is awkward;
> or look up the commit that introduced that line and comment on it, which is a pain, and will not work well if and when the comment is addressed.

I created https://github.com/jenkinsci/jenkins/pull/573 to keep track of
this. IIUC, I think we both now agree that there's no downside to doing
this as a branch as opposed to do this in a personal repository.

>> Wrt line comment on aggregated diff, I don't know what will happen when more commits are added --- I suspect they'll get lost.
>
> No, they continue to be displayed in the diff tab unless and until a commit changes the lines that were commented on—at which point the discussion tab of the pull request
> shows that there were some obsolete comments at a particular point. This makes it easy for the author of the pull request to see at a glance all the details that might
> still need to be addressed according to reviewers.
>

[Kohsuke Kawaguchi]

Done.

On 09/21/2012 02:09 AM, Baptiste MATHUS wrote:
> That's an absolutely great news! Congrats again Kohsuke.
>
> Seeing this makes me think about a side-feature.
> Lazy-loading build records is great when it comes to start the server, but might
> we consider its unloading somehow?
> For example, on our instance (~10 nodes, 250 jobs), we've encountered some
> memory issues.
>
> We didn't investigate that very far, but couldn't we consider using some kind
> of WeakHashMap to handle build results?
> This way, the GC might be able to regain memory by freeing some build results
> accessed not so often.
> And it would eventually get naturally reloaded with that great new lazy-loading
> feature when needed.
>
> What do you think?
>
> Cheers
> PS : Please excuse me if this sounds stupid according to things I would have
> missed about the current behaviour. Just let know if so.
>
> 2012/9/21 Kohsuke Kawaguchi <kkawa...@cloudbees.com

> <mailto:kkawa...@cloudbees.com>>
>
>
> http://kohsuke.org/private/__20120920/jenkins.war

> <http://kohsuke.org/private/20120920/jenkins.war>
>
> I've deployed this exact binary on https://ci.jenkins-ci.org/
>

> I just set up https://ci.jenkins-ci.org/job/__jenkins_lazy_load/

> <https://ci.jenkins-ci.org/job/jenkins_lazy_load/> so this should provide
> up-to-date build.
>
>
> On 09/20/2012 12:28 PM, Emanuele Zattin wrote:
>
> Great job Kohsuke! I'll try and get somebody in Nokia to test this if
> possible.
>
> Emanuele Zattin

> ------------------------------__---------------------

> -I don't have to know an answer. I don't feel frightened by not knowing
> things;
> by being lost in a mysterious universe without any purpose — which is
> the way it
> really is, as far as I can tell, possibly. It doesn't frighten me.-
> Richard Feynman
>
>
> On Thu, Sep 20, 2012 at 9:22 PM, Arnaud Héritier <aher...@gmail.com
> <mailto:aher...@gmail.com>

> <mailto:aher...@gmail.com <mailto:aher...@gmail.com>>> wrote:
>
> Thx a lot KK.
>
> It's a great improvement to come.
>
> As I reported this issue also a long long time ago I'll be happy
> to test it.
> My new infrastructure is using the .deb of jenkins.
> How can I build it from your branch to give it a try ?
>
> Arnaud
>
>
> On Thu, Sep 20, 2012 at 9:16 PM, Kohsuke Kawaguchi

> <kkawa...@cloudbees.com <mailto:kkawa...@cloudbees.com>
> <mailto:kkawaguchi@cloudbees.__com

> [1] http://github.com/jenkinsci/____jenkins/tree/lazy-load
> <http://github.com/jenkinsci/__jenkins/tree/lazy-load>

> <http://github.com/jenkinsci/__jenkins/tree/lazy-load
> <http://github.com/jenkinsci/jenkins/tree/lazy-load>>
> [2]

> https://github.com/jenkinsci/____jenkins/compare/master...__lazy-__load
> <https://github.com/jenkinsci/__jenkins/compare/master...lazy-__load>

>
> <https://github.com/jenkinsci/__jenkins/compare/master...lazy-__load
> <https://github.com/jenkinsci/jenkins/compare/master...lazy-load>>
> [3]

> http://wiki.praqma.net/____jcicodecamp12/sessions/____improve-start-up-time
> <http://wiki.praqma.net/__jcicodecamp12/sessions/__improve-start-up-time>

>
>
> <http://wiki.praqma.net/__jcicodecamp12/sessions/__improve-start-up-time <http://wiki.praqma.net/jcicodecamp12/sessions/improve-start-up-time>>
> --
> Kohsuke Kawaguchi | CloudBees, Inc. | http://cloudbees.com/
> Try Nectar, our professional version of Jenkins
>
>
>
>
> --
> -----
> Arnaud Héritier
> 06-89-76-64-24
> http://aheritier.net
> Mail/GTalk: aher...@gmail.com <mailto:aher...@gmail.com>

> <mailto:aher...@gmail.com <mailto:aher...@gmail.com>>

> Twitter/Skype : aheritier
>
>
>
>
> --
> Kohsuke Kawaguchi | CloudBees, Inc. | http://cloudbees.com/
> Try Nectar, our professional version of Jenkins
>
> --
> Baptiste <Batmat> MATHUS - http://batmat.net
> Sauvez un arbre,
> Mangez un castor !
> nbsp;!
>

[Jesse Glick]

On 09/21/2012 03:28 PM, Kohsuke Kawaguchi wrote:
> I think we both now agree that there's no downside to doing this as a branch as opposed to do this in a personal repository.

Right, I just thought you could not file a pull request from a branch in the same repo, but if you can then great.