[JENKINS-32517] Deprecate Queue#getApproximateItemsQuickly

Now that the item list is obtained lock-free the cache is no longer needed. Besides, the cache could be returning invalid results for requests with different authorization to the one that cached the result (in any case this invalid result is transient).
jenkinsci · Jan 29, 2016 · 638ceb2 · 638ceb2
1 parent 65d92ea
commit 638ceb2
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 49 deletions.
diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java
@@ -171,13 +171,6 @@
 @ExportedBean
 public class Queue extends ResourceController implements Saveable {
 
-    /**
-     * Defines the refresh period for of the internal cache ({@link #itemsView}).
-     * Data should be defined in milliseconds, default value - 1000;
-     * @since 1.577
-     */
-    private static int CACHE_REFRESH_PERIOD = Integer.getInteger(Queue.class.getName() + ".cacheRefreshPeriod", 1000);
-
     /**
      * Items that are waiting for its quiet period to pass.
      *
@@ -218,41 +211,6 @@ public class Queue extends ResourceController implements Saveable {
      */
     private final Cache<Long,LeftItem> leftItems = CacheBuilder.newBuilder().expireAfterWrite(5*60, TimeUnit.SECONDS).build();
 
-    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+CACHE_REFRESH_PERIOD;
-                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}.
      * This is a job offer from the queue to an executor.
@@ -864,9 +822,11 @@ private List<StubItem> filterDiscoverableItemListBasedOnPermissions(List<StubIte
      * This method is primarily added to make UI threads run faster.
      *
      * @since 1.483
+     * @deprecated Use {@link #getItems()} directly. As of 1.607 the approximation is no longer needed.
      */
+    @Deprecated
     public List<Item> getApproximateItemsQuickly() {
-        return itemsView.get();
+        return Arrays.asList(getItems());
     }
 
     public Item getItem(long id) {

diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java
@@ -480,6 +480,11 @@ public List<Queue.Item> getQueueItems() {
         return filterQueue(Arrays.asList(Jenkins.getInstance().getQueue().getItems()));
     }
 
+    /**
+     * @deprecated Use {@link #getQueueItems()}. As of 1.607 the approximation is no longer needed.
+     * @return The items in the queue.
+     */
+    @Deprecated
     public List<Queue.Item> getApproximateQueueItemsQuickly() {
         return filterQueue(Jenkins.getInstance().getQueue().getApproximateItemsQuickly());
     }

diff --git a/core/src/main/java/hudson/widgets/BuildHistoryWidget.java b/core/src/main/java/hudson/widgets/BuildHistoryWidget.java
@@ -63,7 +63,7 @@ public Item getQueuedItem() {
      */
     public List<Item> getQueuedItems() {
         LinkedList<Item> list = new LinkedList<Item>();
-        for (Item item : Jenkins.getInstance().getQueue().getApproximateItemsQuickly()) {
+        for (Item item : Jenkins.getInstance().getQueue().getItems()) {
             if (item.task == owner) {
                 list.addFirst(item);
             }

diff --git a/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly b/core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly
@@ -36,7 +36,7 @@ THE SOFTWARE.
       <l:task href="new" icon="icon-new-computer icon-md" permission="${createPermission}" title="${%New Node}"/>
       <l:task href="configure" icon="icon-setting icon-md" permission="${app.ADMINISTER}" title="${%Configure}"/>
     </l:tasks>
-    <t:queue items="${app.queue.approximateItemsQuickly}" />
+    <t:queue items="${app.queue.items}" />
     <t:executors />
   </l:side-panel>
 </j:jelly>
diff --git a/core/src/main/resources/hudson/model/View/ajaxBuildQueue.jelly b/core/src/main/resources/hudson/model/View/ajaxBuildQueue.jelly
@@ -28,6 +28,6 @@ THE SOFTWARE.
 <?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">
   <l:ajax>
-    <t:queue items="${it.approximateQueueItemsQuickly}" filtered="${it.isFilterQueue()}" />
+    <t:queue items="${it.queueItems}" filtered="${it.isFilterQueue()}" />
   </l:ajax>
 </j:jelly>
diff --git a/core/src/main/resources/jenkins/widgets/BuildQueueWidget/index.groovy b/core/src/main/resources/jenkins/widgets/BuildQueueWidget/index.groovy
@@ -2,4 +2,4 @@ package jenkins.widgets.BuildQueueWidget;
 
 def t = namespace(lib.JenkinsTagLib.class)
 
-t.queue(items:view.approximateQueueItemsQuickly, it:view, filtered:view.filterQueue)
+t.queue(items:view.queueItems, it:view, filtered:view.filterQueue)
diff --git a/core/src/main/resources/lib/hudson/queue.jelly b/core/src/main/resources/lib/hudson/queue.jelly
@@ -28,7 +28,7 @@ THE SOFTWARE.
     Displays the build queue as &lt;l:pane>
 
     <st:attribute name="items" use="required">
-      Queue items to be displayed. Normally you should specify ${app.queue.approximateItemsQuickly},
+      Queue items to be displayed. Normally you should specify ${app.queue.items},
       but for example you can specify a sublist after some filtering to narrow down
       the list.
     </st:attribute>

diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java
@@ -404,7 +404,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
         try {
             build = m.scheduleBuild2(0).get(60, TimeUnit.SECONDS);
         } catch (TimeoutException x) {
-            throw (AssertionError) new AssertionError(r.jenkins.getQueue().getApproximateItemsQuickly().toString()).initCause(x);
+            throw (AssertionError) new AssertionError(r.jenkins.getQueue().getItems().toString()).initCause(x);
         }
         r.assertBuildStatusSuccess(build);
         assertEquals("", build.getBuiltOnStr());