PeepholePermalink.Cache (jenkinsci#10042)

Author: jglick

core/src/main/java/hudson/model/Job.java

@@ -853,7 +853,7 @@ public RunT getBuildForCLI(@Argument(required = true, metaVar = "BUILD#", usage
     }
 
     /**
-     * Gets the youngest build #m that satisfies {@code n<=m}.
+     * Gets the oldest build #m that satisfies {@code m ≥ n}.
      *
      * This is useful when you'd like to fetch a build but the exact build might
      * be already gone (deleted, rotated, etc.)
@@ -868,7 +868,7 @@ public RunT getNearestBuild(int n) {
     }
 
     /**
-     * Gets the latest build #m that satisfies {@code m<=n}.
+     * Gets the newest build #m that satisfies {@code m ≤ n}.
      *
      * This is useful when you'd like to fetch a build but the exact build might
      * be already gone (deleted, rotated, etc.)
@@ -977,7 +977,7 @@ public File getBuildDir() {
     protected abstract void removeRun(RunT run);
 
     /**
-     * Returns the last build.
+     * Returns the newest build.
      * @see LazyBuildMixIn#getLastBuild
      */
     @Exported

core/src/main/java/jenkins/model/PeepholePermalink.java

@@ -4,6 +4,8 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Extension;
+import hudson.ExtensionList;
+import hudson.ExtensionPoint;
 import hudson.Util;
 import hudson.model.Job;
 import hudson.model.PermalinkProjectAction.Permalink;
@@ -14,6 +16,7 @@
 import hudson.util.AtomicFileWriter;
 import java.io.File;
 import java.io.IOException;
+import java.io.Serializable;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.util.HashMap;
@@ -24,6 +27,7 @@
 import java.util.logging.Logger;
 import java.util.stream.Stream;
 import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.Beta;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
@@ -63,14 +67,6 @@
  */
 public abstract class PeepholePermalink extends Permalink implements Predicate<Run<?, ?>> {
 
-    /**
-     * JENKINS-22822: avoids rereading caches.
-     * Top map keys are {@code builds} directories.
-     * Inner maps are from permalink name to build number.
-     * Synchronization is first on the outer map, then on the inner.
-     */
-    private static final Map<File, Map<String, Integer>> caches = new HashMap<>();
-
     /**
      * Checks if the given build satisfies the peep-hole criteria.
      *
@@ -94,115 +90,216 @@ protected File getPermalinkFile(Job<?, ?> job) {
      */
     @Override
     public Run<?, ?> resolve(Job<?, ?> job) {
-        Map<String, Integer> cache = cacheFor(job.getBuildDir());
-        int n;
-        synchronized (cache) {
-            n = cache.getOrDefault(getId(), 0);
+        return ExtensionList.lookupFirst(Cache.class).get(job, getId()).resolve(this, job, getId());
+    }
+
+    /**
+     * Start from the build 'b' and locate the build that matches the criteria going back in time
+     */
+    @CheckForNull
+    private Run<?, ?> find(@CheckForNull Run<?, ?> b) {
+        while (b != null && !apply(b)) {
+            b = b.getPreviousBuild();
         }
-        if (n == RESOLVES_TO_NONE) {
-            return null;
+        return b;
+    }
+
+    /**
+     * Remembers the value 'n' in the cache for future {@link #resolve(Job)}.
+     */
+    protected void updateCache(@NonNull Job<?, ?> job, @CheckForNull Run<?, ?> b) {
+        ExtensionList.lookupFirst(Cache.class).put(job, getId(), b != null ? new Cache.Some(b.getNumber()) : Cache.NONE);
+    }
+
+    /**
+     * Persistable cache of peephole permalink targets.
+     */
+    @Restricted(Beta.class)
+    public interface Cache extends ExtensionPoint {
+
+        /** Cacheable target of a permalink. */
+        sealed interface PermalinkTarget extends Serializable {
+
+            /**
+             * Implementation of {@link #resolve(Job)}.
+             * This may update the cache if it was missing or found to be invalid.
+             */
+            @Restricted(NoExternalUse.class)
+            @CheckForNull
+            Run<?, ?> resolve(@NonNull PeepholePermalink pp, @NonNull Job<?, ?> job, @NonNull String id);
+
+            /**
+             * Partial implementation of {@link #resolve(PeepholePermalink, Job, String)} when searching.
+             * @param b if set, the newest build to even consider when searching
+             */
+            @Restricted(NoExternalUse.class)
+            @CheckForNull
+            default Run<?, ?> search(@NonNull PeepholePermalink pp, @NonNull Job<?, ?> job, @NonNull String id, @CheckForNull Run<?, ?> b) {
+                if (b == null) {
+                    // no cache
+                    b = job.getLastBuild();
+                }
+                // start from the build 'b' and locate the build that matches the criteria going back in time
+                b = pp.find(b);
+                pp.updateCache(job, b);
+                return b;
+            }
+
         }
-        Run<?, ?> b;
-        if (n > 0) {
-            b = job.getBuildByNumber(n);
-            if (b != null && apply(b)) {
-                return b; // found it (in the most efficient way possible)
+
+        /**
+         * The cache entry for this target is missing.
+         */
+        record Unknown() implements PermalinkTarget {
+            @Override
+            public Run<?, ?> resolve(PeepholePermalink pp, Job<?, ?> job, String id) {
+                return search(pp, job, id, null);
             }
-        } else {
-            b = null;
         }
 
-        // the cache is stale. start the search
-        if (b == null) {
-            b = job.getNearestOldBuild(n);
+        Unknown UNKNOWN = new Unknown();
+
+        /**
+         * The cache entry for this target is present.
+         */
+        sealed interface Known extends PermalinkTarget {}
+
+        /** There is known to be no matching build. */
+        record None() implements Known {
+            @Override
+            public Run<?, ?> resolve(PeepholePermalink pp, Job<?, ?> job, String id) {
+                return null;
+            }
         }
 
-        if (b == null) {
-            // no cache
-            b = job.getLastBuild();
+        /** Singleton of {@link None}. */
+        None NONE = new None();
+
+        /** A matching build, indicated by {@link Run#getNumber}. */
+        record Some(int number) implements Known {
+            @Override
+            public Run<?, ?> resolve(PeepholePermalink pp, Job<?, ?> job, String id) {
+                Run<?, ?> b = job.getBuildByNumber(number);
+                if (b != null && pp.apply(b)) {
+                    return b; // found it (in the most efficient way possible)
+                }
+                // the cache is stale. start the search
+                if (b == null) {
+                    b = job.getNearestOldBuild(number);
+                }
+                return search(pp, job, id, b);
+            }
         }
 
-        // start from the build 'b' and locate the build that matches the criteria going back in time
-        b = find(b);
+        /**
+         * Looks for any existing cache hit.
+         * @param id {@link #getId}
+         * @return {@link Some} or {@link #NONE} or {@link #UNKNOWN}
+         */
+        @NonNull PermalinkTarget get(@NonNull Job<?, ?> job, @NonNull String id);
 
-        updateCache(job, b);
-        return b;
+        /**
+         * Updates the cache.
+         * Note that this may be called not just when a build completes or is deleted
+         * (meaning that the logical value of the cache has changed),
+         * but also when {@link #resolve} has failed to find a cached value
+         * (or determined that a previously cached value is in fact invalid).
+         * @param id {@link #getId}
+         * @param target {@link Some} or {@link #NONE}
+         */
+        void put(@NonNull Job<?, ?> job, @NonNull String id, @NonNull Known target);
     }
 
     /**
-     * Start from the build 'b' and locate the build that matches the criteria going back in time
+     * Default cache based on a {@code permalinks} file in the build directory.
+     * There is one line per cached permalink, in the format {@code lastStableBuild 123}
+     * or (for a negative cache) {@code lastFailedBuild -1}.
      */
-    private Run<?, ?> find(Run<?, ?> b) {
-        //noinspection StatementWithEmptyBody
-        for ( ; b != null && !apply(b); b = b.getPreviousBuild())
-            ;
-        return b;
-    }
+    @Restricted(NoExternalUse.class)
+    @Extension(ordinal = -1000)
+    public static final class DefaultCache implements Cache {
+
+        /**
+         * JENKINS-22822: avoids rereading caches.
+         * Top map keys are {@code builds} directories.
+         * Inner maps are from permalink name to target.
+         * Synchronization is first on the outer map, then on the inner.
+         */
+        private final Map<File, Map<String, Known>> caches = new HashMap<>();
 
-    private static @NonNull Map<String, Integer> cacheFor(@NonNull File buildDir) {
-        synchronized (caches) {
-            Map<String, Integer> cache = caches.get(buildDir);
-            if (cache == null) {
-                cache = load(buildDir);
-                caches.put(buildDir, cache);
+        @Override
+        public PermalinkTarget get(Job<?, ?> job, String id) {
+            var cache = cacheFor(job.getBuildDir());
+            synchronized (cache) {
+                var cached = cache.get(id);
+                return cached != null ? cached : UNKNOWN;
             }
-            return cache;
         }
-    }
 
-    private static @NonNull Map<String, Integer> load(@NonNull File buildDir) {
-        Map<String, Integer> cache = new TreeMap<>();
-        File storage = storageFor(buildDir);
-        if (storage.isFile()) {
-            try (Stream<String> lines = Files.lines(storage.toPath(), StandardCharsets.UTF_8)) {
-                lines.forEach(line -> {
-                    int idx = line.indexOf(' ');
-                    if (idx == -1) {
-                        return;
-                    }
+        @Override
+        public void put(Job<?, ?> job, String id, Known target) {
+            File buildDir = job.getBuildDir();
+            var cache = cacheFor(buildDir);
+            synchronized (cache) {
+                cache.put(id, target);
+                File storage = storageFor(buildDir);
+                LOGGER.fine(() -> "saving to " + storage + ": " + cache);
+                try (AtomicFileWriter cw = new AtomicFileWriter(storage)) {
                     try {
-                        cache.put(line.substring(0, idx), Integer.parseInt(line.substring(idx + 1)));
-                    } catch (NumberFormatException x) {
-                        LOGGER.log(Level.WARNING, "failed to read " + storage, x);
+                        for (var entry : cache.entrySet()) {
+                            cw.write(entry.getKey());
+                            cw.write(' ');
+                            cw.write(Integer.toString(entry.getValue() instanceof Cache.Some some ? some.number : -1));
+                            cw.write('\n');
+                        }
+                        cw.commit();
+                    } finally {
+                        cw.abort();
                     }
-                });
-            } catch (IOException x) {
-                LOGGER.log(Level.WARNING, "failed to read " + storage, x);
+                } catch (IOException x) {
+                    LOGGER.log(Level.WARNING, "failed to update " + storage, x);
+                }
             }
-            LOGGER.fine(() -> "loading from " + storage + ": " + cache);
         }
-        return cache;
-    }
 
-    static @NonNull File storageFor(@NonNull File buildDir) {
-        return new File(buildDir, "permalinks");
-    }
+        private @NonNull Map<String, Known> cacheFor(@NonNull File buildDir) {
+            synchronized (caches) {
+                var cache = caches.get(buildDir);
+                if (cache == null) {
+                    cache = load(buildDir);
+                    caches.put(buildDir, cache);
+                }
+                return cache;
+            }
+        }
 
-    /**
-     * Remembers the value 'n' in the cache for future {@link #resolve(Job)}.
-     */
-    protected void updateCache(@NonNull Job<?, ?> job, @CheckForNull Run<?, ?> b) {
-        File buildDir = job.getBuildDir();
-        Map<String, Integer> cache = cacheFor(buildDir);
-        synchronized (cache) {
-            cache.put(getId(), b == null ? RESOLVES_TO_NONE : b.getNumber());
+        private static @NonNull Map<String, Known> load(@NonNull File buildDir) {
+            Map<String, Known> cache = new TreeMap<>();
             File storage = storageFor(buildDir);
-            LOGGER.fine(() -> "saving to " + storage + ": " + cache);
-            try (AtomicFileWriter cw = new AtomicFileWriter(storage)) {
-                try {
-                    for (Map.Entry<String, Integer> entry : cache.entrySet()) {
-                        cw.write(entry.getKey());
-                        cw.write(' ');
-                        cw.write(Integer.toString(entry.getValue()));
-                        cw.write('\n');
-                    }
-                    cw.commit();
-                } finally {
-                    cw.abort();
+            if (storage.isFile()) {
+                try (Stream<String> lines = Files.lines(storage.toPath(), StandardCharsets.UTF_8)) {
+                    lines.forEach(line -> {
+                        int idx = line.indexOf(' ');
+                        if (idx == -1) {
+                            return;
+                        }
+                        try {
+                            int number = Integer.parseInt(line.substring(idx + 1));
+                            cache.put(line.substring(0, idx), number == -1 ? Cache.NONE : new Cache.Some(number));
+                        } catch (NumberFormatException x) {
+                            LOGGER.log(Level.WARNING, "failed to read " + storage, x);
+                        }
+                    });
+                } catch (IOException x) {
+                    LOGGER.log(Level.WARNING, "failed to read " + storage, x);
                 }
-            } catch (IOException x) {
-                LOGGER.log(Level.WARNING, "failed to update " + storage, x);
+                LOGGER.fine(() -> "loading from " + storage + ": " + cache);
             }
+            return cache;
+        }
+
+        static @NonNull File storageFor(@NonNull File buildDir) {
+            return new File(buildDir, "permalinks");
         }
     }
 
@@ -380,7 +477,5 @@ public boolean apply(Run<?, ?> run) {
     @Restricted(NoExternalUse.class)
     public static void initialized() {}
 
-    private static final int RESOLVES_TO_NONE = -1;
-
     private static final Logger LOGGER = Logger.getLogger(PeepholePermalink.class.getName());
 }

test/src/test/java/jenkins/model/PeepholePermalinkTest.java

@@ -82,7 +82,7 @@ public void basics() throws Exception {
     }
 
     private void assertStorage(String id, Job<?, ?> job, Run<?, ?> build) throws Exception {
-        assertThat(Files.readAllLines(PeepholePermalink.storageFor(job.getBuildDir()).toPath(), StandardCharsets.UTF_8),
+        assertThat(Files.readAllLines(PeepholePermalink.DefaultCache.storageFor(job.getBuildDir()).toPath(), StandardCharsets.UTF_8),
             hasItem(id + " " + (build == null ? -1 : build.getNumber())));
     }