Could you briefly describe your case (number of projects, categories structure, frequency of job submissions, etc.)?

BTW, I confirm that the current implementation may cause huge calculation efforts in the case of big number of jobs with a same category:

• keySet() causes a useless copy of the data
• copy to a new array leads to an additional copy of items

It makes sense to extend a the cache's lock time, but to iterate the HashMap directly.

I'll reassign issue to Jesse Glick, who is an author of https://github.com/jenkinsci/throttle-concurrent-builds-plugin/pull/10
Jesse, do you have some time to fix the issue? If no, I can do it on my own

I do not actually run this plugin anywhere so I have no real way of confirming whether a given change improves or degrades performance on a large installation.

Extending the scope of the lock on propertiesByCategory may avoid overhead, but runs the risk of deadlocks, if foreign calls such as getItem are included.

BTW the actual thread dump shows an issue in WeakHashMap.HashIterator.hasNext, such as an endless loop. The keySet method does not copy any data, and there is no indication that the array copy overhead is relevant. If I am right, iterating the map directly would not make any difference.

What JRE is being used to run Jenkins? If not the most recent Java 7, try updating and see if the issue remains.

I have a very similar issue with 1.8.1 compared to 1.8, only for me it goes into 100%CPU during startup of Jenkins and never shows up the Dashboard any more, keeps saying Jenkins is still starting up.

I think this is not a performance problem but rather some sort of endless loop, or at least a very big loop if this simple put runs for over half an hour!

I'm on 1.7.0_45

Reverting to 1.8 fixed it for me for now...

CPU Sampling Content

Level Name Class CPU Time [ms] ↓ Total CPU Time [ms] Execution Time [ms] Total Time [ms] Wait time [ms]
1 setOwner(AbstractProject) ThrottleJobProperty 1.00 1.00 71700 71700 71650.00
2 setOwner(Job) ThrottleJobProperty 0.00 1.00 0 71700 0.00
3 onLoad(ItemGroup, String) Job 0.00 1.00 0 71700 0.00
4 onLoad(ItemGroup, String) AbstractProject 0.00 1.00 0 71700 0.00
5 onLoad(ItemGroup, String) Project 0.00 1.00 0 47800 0.00
6 load(ItemGroup, File) Items 0.00 1.00 0 47800 0.00
7 run(Reactor) Jenkins$18 0.00 1.00 0 47800 0.00
8 run(Reactor) TaskGraphBuilder$TaskImpl 0.00 1.00 0 47800 0.00
9 runTask(Task) Reactor 0.00 1.00 0 47800 0.00
10 runTask(Task) Jenkins$7 0.00 1.00 0 47800 0.00

Debugging some more seems to indicate that it actually loops in the java.util.WeakHashMap in the last two lines of the following:

public V put(K key, V value) {
Object k = maskNull(key);
int h = hash(k);
Entry<K,V>[] tab = getTable();
int i = indexFor(h, tab.length);

for (Entry<K,V> e = tab[i]; e != null; e = e.next) {
if (h == e.hash && eq(k, e.get())) {

somehow the tab contains an null-element which points to itself via e.next, thus the code is looping endlessly.

Googling shows that this can happen if the WeakHashMap is used in multiple threads without proper synchronization, e.g. https://java.net/jira/browse/JAVASERVERFACES-2544 and https://issues.apache.org/bugzilla/show_bug.cgi?id=50078, http://www.adam-bien.com/roller/abien/entry/endless_loops_in_unsychronized_weakhashmap

However after a quick glance I do not see where the code in the plugin uses the Map outside the synchronized block, how can it have multi-threading issues then?!

I ran into the same problems now:

I use Jenkins 1.547 on Debian 5.0.10 with Throttle Concurrent Builds 1.8.1

At midnight several jobs are triggered which are in the same group causing Jenkins webinterface to freeze (process at ~100% cpu). The only solution is to kill the process. After removing the plugin everything runs fine again. In my opinion the plugin is not usable as long as the error is not fixed.

Switching back to 1.8 solved the issue for me.

@centic, for the issue's analysis

The issue has been introduce by https://github.com/jenkinsci/throttle-concurrent-builds-plugin/commit/b1f3b836ecd1fbd7a141c89469635c1ca5838dcf committed by @glick

I home to find some time to fix the issue on the next week (probably, Jesse will do it before me)
I'll put the info to the plugin's page as well.

No idea for a fix from me, sorry. The tip about unsynchronized access sounds plausible, but as you note, all accesses to the map should be synchronized from the same monitor. If you can reproduce the bug, then there are various ways of proceeding with debugging, such as switching to a (strong) HashMap to see if the endless loop goes away; this would of course introduce a memory leak, but at least you would have narrowed down the problem.

Some more info for anybody trying to work on this, I did some more investigation and found out the following:

• Replacing the WeakHashMap with HashMap is tricky as the configuration XML of the plugin also contains the type of Map, so you need to replace it there as well otherwise you'll still end up with WeakHashMaps (cost me a couple hours to find that out!)
• I think this is also the reason why the problem happens in the first place, the config-serialization is reflection-based and constructs the WeakHashMap outside of the synchronization, thus multiple threads still can access the data concurrently and because of this no synchronization whatsoever will fix it!
• That also explains why you do not see it always. It only happens if you have a number of throttles defined and use them in multiple jobs. Also starting up will sometimes work, but it may still fail later when the properties are accessed when jobs are started.

propertiesByCategory is not intended to be serialized at all, so that is probably the issue. May need to be made transient (and restored in readResolve); or making it static might suffice.

Jesse, Oleg, are you guys working on this? If not, I think we should probably do a rollback and re-release the old 1.8.0 again as 1.8.2 to trump the broken 1.8.1...

@abayer
I've started working on the issue.
BTW, I cannot re-test the fix now due to the vacation.

If somebody has a relevant dev. system, I can provide a test build.

I agree with abayer.

We have lost many person hours by assuming no harm would come from installing this update.

This is the first time in my use of jenkins that updating to the latest version has bitten me.

There should be a mechanism to prevent a plugin with such an issue being presented to end users.

> We have lost many person hours by assuming no harm would come from installing this update.
There is a notification on the plugin's Wiki page
> This is the first time in my use of jenkins that updating to the latest version has bitten me.
Heh, you are really lucky. BTW, you are right that such approach hardly affects users.
I'm responsible for this f*ckup, because I've released a version without long-run testing on development servers.
BTW, we have been waiting for @abayer's review of PR #6 and PR #10 for more than 1 month...

> There should be a mechanism to prevent a plugin with such an issue being presented to end users.
AFAIK in the open-source version of update center there is no such features.

We had the same issue after updating the plugins. After many hours we could identify the issue in this plugin (and found the warning on the wiki page).

I have a relevant system. You can send me the test build to test the fix.

Oleg - sorry about that. I get so many PR emails from all of the Jenkins repos that I don't notice all of the relevant ones. IF there's anything you need a review on now, I can take a look, but in general, I trust Jesse to be better than me at knowing what code's likely to cause issues anyway. =)

I could prototype a fix if my help is needed, but I have no realistic test environment for it.

Added the local build with a probable fix.
https://github.com/jenkinsci/throttle-concurrent-builds-plugin/pull/13

I've tried about 10 restarts with big queues, but the issue has not been reproduced. BTW, it would be great to have a light-weight unit test.

While that PR may work, I think it would be better to just make the cache be transient. I never had any intention of its being persisted to disk, so if that is what is happening, that was purely an accident (XStream automagically finding stuff and saving it). The cache would just need to be recreated if and when a job category is changed or the cache is requested, but that is pretty simple synchronization.

@Jesse
I've made the cache transient.
All other changes just provide the safe migration procedure if there is any cache data on the disk.
The load procedure is synchronized to avoid concurrency. After that, the code just re-saves the configuration in order to purge wrong data from configs.

We are testing the attached version in our production environment since about 4 hours now. Works like a charm until now.

We are consolidating two jenkins masters into one machine with about 15 Slaves now, 3000 jobs altogether and quite some continuously running jobs with concurrent builds that are heavily loading the about 100 cores. We even had severe problems with blocked threads when we rolled back to 1.8.0! Funny enough I was using the version 1.8.1 on "my" old master without problems before (even more jobs, same amount of nodes) and using the throttle concurrent builds plugin was "my" idea.

The new version works for me as well.

@abayer, do you confirm the merge?

Code changed in jenkins
User: Oleg Nenashev
Path:
src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java
http://jenkins-ci.org/commit/throttle-concurrent-builds-plugin/9b7562d4b08e0a4202130d43307082553142df82
Log:
[FIXED JENKINS-21044] - Throttling blocks the Jenkins queue

Seems the issue was in improper usage of WeakHashMap (see analysis from @centic).
I've managed to reproduce the behavior in the following case:

• There is a big number of jobs/configurations with throttling
• The builds queue is not empty
  // Seems that ThrottleQueueTaskDispatcher tries to access the data before the complete loading of the plugin's configuration.

This fix provides an explicit locking of any load operations + manual cleanup of erroneous cache data, which goes to persistence in 1.8.1
Resolves https://issues.jenkins-ci.org/browse/JENKINS-21044

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>

Code changed in jenkins
User: Oleg Nenashev
Path:
src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java
http://jenkins-ci.org/commit/throttle-concurrent-builds-plugin/c453516716079248d74ce588efc0293669e6e1a7
Log:
JENKINS-21044 - Don't create a new HashMap after the load operation

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>

Code changed in jenkins
User: Andrew Bayer
Path:
src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java
http://jenkins-ci.org/commit/throttle-concurrent-builds-plugin/70107b4222502935a9e46beffa31daae2e99e50b
Log:
Merge pull request #13 from synopsys-arc-oss/JENKINS_21044_fix

[FIXED JENKINS-21044] - Throttling blocks the Jenkins queue

Compare: https://github.com/jenkinsci/throttle-concurrent-builds-plugin/compare/dc16282a90b7...70107b422250

Thanks, Oleg!