提交 f1aa7ba0 编写于 作者: K Kohsuke Kawaguchi

Fixed the lock contention problem on Queue.getItems()

From what I was told by Emanuele today in the office hours, this came up
during the Copenhagen hackathon.
上级 8891c86a
...@@ -70,6 +70,8 @@ Upcoming changes</a> ...@@ -70,6 +70,8 @@ Upcoming changes</a>
Default max # of concurrent HTTP request handling threads were brought down to a sane number (from 1000(!) to 20) Default max # of concurrent HTTP request handling threads were brought down to a sane number (from 1000(!) to 20)
<li class=rfe> <li class=rfe>
Display non-default update site URLs in the Advanced tab of Plugin Manager. (Currently not configurable from this UI.) Display non-default update site URLs in the Advanced tab of Plugin Manager. (Currently not configurable from this UI.)
<li class=rfe>
Fixed the lock contention problem on <tt>Queue.getItems()</tt>
<li class=rfe> <li class=rfe>
Put slave back online automatically, if there's enough disk space again Put slave back online automatically, if there's enough disk space again
(<a href="https://github.com/jenkinsci/jenkins/pull/514">pull 514</a>) (<a href="https://github.com/jenkinsci/jenkins/pull/514">pull 514</a>)
......
...@@ -24,8 +24,10 @@ ...@@ -24,8 +24,10 @@
*/ */
package hudson.model; package hudson.model;
import com.google.common.collect.ImmutableList;
import hudson.AbortException; import hudson.AbortException;
import hudson.BulkChange; import hudson.BulkChange;
import hudson.CopyOnWrite;
import hudson.ExtensionList; import hudson.ExtensionList;
import hudson.ExtensionPoint; import hudson.ExtensionPoint;
import hudson.Util; import hudson.Util;
...@@ -87,6 +89,7 @@ import java.util.TreeSet; ...@@ -87,6 +89,7 @@ import java.util.TreeSet;
import java.util.Map.Entry; import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicLong;
import java.util.logging.Level; import java.util.logging.Level;
import java.util.logging.Logger; import java.util.logging.Logger;
...@@ -161,6 +164,41 @@ public class Queue extends ResourceController implements Saveable { ...@@ -161,6 +164,41 @@ public class Queue extends ResourceController implements Saveable {
*/ */
private final ItemList<BuildableItem> pendings = new ItemList<BuildableItem>(); private final ItemList<BuildableItem> pendings = new ItemList<BuildableItem>();
private final CachedItemList itemsView = new CachedItemList();
/**
* Maintains a copy of {@link Queue#getItems()}
*
* @see Queue#getApproximateItemsQuickly()
*/
private class CachedItemList {
/**
* The current cached value.
*/
@CopyOnWrite
private volatile List<Item> itemsView = Collections.emptyList();
/**
* When does the cache info expire?
*/
private final AtomicLong expires = new AtomicLong();
List<Item> get() {
long t = System.currentTimeMillis();
long d = expires.get();
if (t>d) {// need to refresh the cache
long next = t+1000;
if (expires.compareAndSet(d,next)) {
// avoid concurrent cache update via CAS.
// if the getItems() lock is contended,
// some threads will end up serving stale data,
// but that's OK.
itemsView = ImmutableList.copyOf(getItems());
}
}
return itemsView;
}
}
/** /**
* Data structure created for each idle {@link Executor}. * Data structure created for each idle {@link Executor}.
* This is a job offer from the queue to an executor. * This is a job offer from the queue to an executor.
...@@ -609,6 +647,27 @@ public class Queue extends ResourceController implements Saveable { ...@@ -609,6 +647,27 @@ public class Queue extends ResourceController implements Saveable {
r[idx++] = p; r[idx++] = p;
return r; return r;
} }
/**
* Like {@link #getItems()}, but returns an approximation that might not be completely up-to-date.
*
* <p>
* At the expense of accuracy, this method does not lock {@link Queue} and therefore is faster
* in a highly concurrent situation.
*
* <p>
* The list obtained is an accurate snapshot of the queue at some point in the past. The snapshot
* is updated and normally no less than one second old, but this is a soft commitment that might
* get violated when the lock on {@link Queue} is highly contended.
*
* <p>
* This method is primarily added to make UI threads run faster.
*
* @since 1.483
*/
public List<Item> getApproximateItemsQuickly() {
return itemsView.get();
}
public synchronized Item getItem(int id) { public synchronized Item getItem(int id) {
for (Item item: waitingList) if (item.id == id) return item; for (Item item: waitingList) if (item.id == id) return item;
......
...@@ -426,14 +426,14 @@ public abstract class View extends AbstractModelObject implements AccessControll ...@@ -426,14 +426,14 @@ public abstract class View extends AbstractModelObject implements AccessControll
return true; return true;
} }
public List<Queue.Item> getQueueItems() { private List<Queue.Item> filterQueue(List<Queue.Item> base) {
if (!isFilterQueue()) { if (!isFilterQueue()) {
return Arrays.asList(Jenkins.getInstance().getQueue().getItems()); return base;
} }
Collection<TopLevelItem> items = getItems(); Collection<TopLevelItem> items = getItems();
List<Queue.Item> result = new ArrayList<Queue.Item>(); List<Queue.Item> result = new ArrayList<Queue.Item>();
for (Queue.Item qi : Jenkins.getInstance().getQueue().getItems()) { for (Queue.Item qi : base) {
if (items.contains(qi.task)) { if (items.contains(qi.task)) {
result.add(qi); result.add(qi);
} else } else
...@@ -447,6 +447,14 @@ public abstract class View extends AbstractModelObject implements AccessControll ...@@ -447,6 +447,14 @@ public abstract class View extends AbstractModelObject implements AccessControll
return result; return result;
} }
public List<Queue.Item> getQueueItems() {
return filterQueue(Arrays.asList(Jenkins.getInstance().getQueue().getItems()));
}
public List<Queue.Item> getApproximateQueueItemsQuickly() {
return filterQueue(Jenkins.getInstance().getQueue().getApproximateItemsQuickly());
}
/** /**
* Returns the path relative to the context root. * Returns the path relative to the context root.
* *
......
...@@ -36,7 +36,7 @@ THE SOFTWARE. ...@@ -36,7 +36,7 @@ THE SOFTWARE.
<l:task icon="images/24x24/new-computer.png" href="new" title="${%New Node}" permission="${createPermission}" /> <l:task icon="images/24x24/new-computer.png" href="new" title="${%New Node}" permission="${createPermission}" />
<l:task icon="images/24x24/setting.png" href="configure" title="${%Configure}" permission="${app.ADMINISTER}" /> <l:task icon="images/24x24/setting.png" href="configure" title="${%Configure}" permission="${app.ADMINISTER}" />
</l:tasks> </l:tasks>
<t:queue items="${app.queue.items}" /> <t:queue items="${app.queue.approximateItemsQuickly}" />
<t:executors /> <t:executors />
</l:side-panel> </l:side-panel>
</j:jelly> </j:jelly>
\ No newline at end of file
...@@ -28,6 +28,6 @@ THE SOFTWARE. ...@@ -28,6 +28,6 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?> <?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:s="/lib/form"> <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:s="/lib/form">
<l:ajax> <l:ajax>
<t:queue items="${it.queueItems}" /> <t:queue items="${it.approximateItemsQuickly}" />
</l:ajax> </l:ajax>
</j:jelly> </j:jelly>
\ No newline at end of file
...@@ -70,7 +70,7 @@ THE SOFTWARE. ...@@ -70,7 +70,7 @@ THE SOFTWARE.
<st:include page="tasks-bottom.jelly" it="${it.owner}" optional="true" /> <st:include page="tasks-bottom.jelly" it="${it.owner}" optional="true" />
<t:actions /> <t:actions />
</l:tasks> </l:tasks>
<t:queue items="${it.queueItems}" /> <t:queue items="${it.approximateQueueItemsQuickly}" />
<t:executors computers="${it.computers}" /> <t:executors computers="${it.computers}" />
<j:forEach var="w" items="${it.widgets}"> <j:forEach var="w" items="${it.widgets}">
<st:include it="${w}" page="index.jelly" /> <st:include it="${w}" page="index.jelly" />
......
...@@ -29,7 +29,6 @@ import static hudson.model.ItemGroupMixIn.loadChildren; ...@@ -29,7 +29,6 @@ import static hudson.model.ItemGroupMixIn.loadChildren;
import hudson.CopyOnWrite; import hudson.CopyOnWrite;
import hudson.EnvVars; import hudson.EnvVars;
import hudson.Extension; import hudson.Extension;
import hudson.ExtensionList;
import hudson.FilePath; import hudson.FilePath;
import hudson.Functions; import hudson.Functions;
import hudson.Indenter; import hudson.Indenter;
...@@ -60,7 +59,6 @@ import hudson.model.TopLevelItem; ...@@ -60,7 +59,6 @@ import hudson.model.TopLevelItem;
import hudson.search.CollectionSearchIndex; import hudson.search.CollectionSearchIndex;
import hudson.search.SearchIndexBuilder; import hudson.search.SearchIndexBuilder;
import hudson.tasks.BuildStep; import hudson.tasks.BuildStep;
import hudson.tasks.BuildStepDescriptor;
import hudson.tasks.BuildWrapper; import hudson.tasks.BuildWrapper;
import hudson.tasks.BuildWrappers; import hudson.tasks.BuildWrappers;
import hudson.tasks.Builder; import hudson.tasks.Builder;
...@@ -993,8 +991,19 @@ public class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,MavenMod ...@@ -993,8 +991,19 @@ public class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,MavenMod
* Returns the {@link MavenModule}s that are in the queue. * Returns the {@link MavenModule}s that are in the queue.
*/ */
public List<Queue.Item> getQueueItems() { public List<Queue.Item> getQueueItems() {
List<Queue.Item> r = new ArrayList<hudson.model.Queue.Item>(); return filter(Arrays.asList(Jenkins.getInstance().getQueue().getItems()));
for( Queue.Item item : Jenkins.getInstance().getQueue().getItems() ) { }
/**
* Returns the {@link MavenModule}s that are in the queue.
*/
public List<Queue.Item> getApproximateQueueItemsQuickly() {
return filter(Jenkins.getInstance().getQueue().getApproximateItemsQuickly());
}
private List<Queue.Item> filter(Collection<Queue.Item> base) {
List<Queue.Item> r = new ArrayList<Queue.Item>();
for( Queue.Item item : base) {
Task t = item.task; Task t = item.task;
if((t instanceof MavenModule && ((MavenModule)t).getParent()==this) || t ==this) if((t instanceof MavenModule && ((MavenModule)t).getParent()==this) || t ==this)
r.add(item); r.add(item);
......
...@@ -28,6 +28,6 @@ THE SOFTWARE. ...@@ -28,6 +28,6 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?> <?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:s="/lib/form"> <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:s="/lib/form">
<l:ajax> <l:ajax>
<t:queue items="${it.queueItems}" /> <t:queue items="${it.approximateQueueItemsQuickly}" />
</l:ajax> </l:ajax>
</j:jelly> </j:jelly>
\ No newline at end of file
...@@ -31,6 +31,6 @@ THE SOFTWARE. ...@@ -31,6 +31,6 @@ THE SOFTWARE.
sense when modules are built in parallel. sense when modules are built in parallel.
See related HUDSON-1892 for how this confuses users. See related HUDSON-1892 for how this confuses users.
--> -->
<t:queue items="${it.queueItems}"/> <t:queue items="${it.approximateQueueItemsQuickly}"/>
</j:if> </j:if>
</j:jelly> </j:jelly>
\ No newline at end of file
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册