OAK-12425: opt into Caffeine SYNC without metadata changes

oak-doc/src/site/markdown/nodestore/documentmk.md

@@ -937,6 +937,14 @@ of concern.
 
 ### <a name="cache-configuration"></a> Cache Configuration
 
+CacheLIRS remains the default. With OAK-12425, enable
+`FT_CAFFEINE_CACHE_OAK-12425` or set `-Doak.documentMK.caffeineCache=true` to
+select Caffeine with SYNC maintenance. Selection is fixed per builder; recreate
+the builder/store or restart the service after changing the toggle.
+The legacy `-Doak.documentMK.guavaCache=true` also selects Caffeine SYNC,
+regardless of the new toggle; despite its name, it no longer selects Guava.
+Setting both properties to `true` selects the same Caffeine SYNC implementation.
+
 In a default setup the [DocumentNodeStoreService][osgi-config]
 takes a single config for `cache` which is internally distributed among the 
 various caches above in following way

oak-doc/src/site/markdown/osgi_config.md

@@ -260,8 +260,8 @@ prevDocCachePercentage | 4 | Percentage of `cache` allocated for `prevDocCache`.
 childrenCachePercentage | 15 (was 10 until 1.5.14) | Percentage of `cache` allocated for `childrenCache`. See [Caching][doc-cache] | 1.0.12
 diffCachePercentage | 30 (was 5 until 1.5.14) | Percentage of `cache` allocated for `diffCache`. See [Caching][doc-cache] | 1.0.12
 docChildrenCachePercentage | 0 (was 3 until 1.5.6) | Percentage of `cache` allocated for `docChildrenCache`. See [Caching][doc-cache] (Removed since 1.5.6) | 1.0.12
-cacheSegmentCount | 16 | The number of segments in the LIRS cache | 1.0.15, 1.2.3, 1.3.0
-cacheStackMoveDistance | 16 | The delay to move entries to the head of the queue in the LIRS cache | 1.0.15, 1.2.3, 1.3.0
+cacheSegmentCount | 16 | The number of segments in the LIRS cache (ignored by the opt-in Caffeine cache, see `FT_CAFFEINE_CACHE_OAK-12425`; toggling it requires a DocumentNodeStore restart) | 1.0.15, 1.2.3, 1.3.0
+cacheStackMoveDistance | 16 | The delay to move entries to the head of the queue in the LIRS cache (ignored by the opt-in Caffeine cache, see `FT_CAFFEINE_CACHE_OAK-12425`; toggling it requires a DocumentNodeStore restart) | 1.0.15, 1.2.3, 1.3.0
 sharedDSRepoId | "" | Custom SharedDataStore repositoryId. Used when custom blobstore configured. Should be unique among the repositories sharing the datastore. | 1.2.11
 blobTrackSnapshotIntervalInSecs | 0 | The blob ids cached/tracked locally are synchronized with the DataStore at this interval. Any additions and deletions will be visible to other cluster nodes or repositories connected to the shared DatStore after this. This should be less than the blobGcMaxAgeInSecs parameter above and the frequency of blob gc. A value of `0` disables blob ID tracking (default since Oak 2.4.0). See [Blob tracker][blobtracker]. | 1.5.6 
 updateLimit | 100000 | The number of updates kept in memory until changes are written to a branch in the DocumentStore | 1.7.0  

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java

@@ -35,6 +35,7 @@
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.DEFAULT_PREV_DOC_CACHE_PERCENTAGE;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.DEFAULT_PREV_NO_PROP_CACHE_PERCENTAGE;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.DEFAULT_UPDATE_LIMIT;
+import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.FT_CAFFEINE_CACHE;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_AVOID_EXCLUSIVE_MERGE_LOCK;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_FULL_GC_ENABLED;
 import static org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.DEFAULT_EMBEDDED_VERIFICATION_ENABLED;
@@ -194,15 +195,21 @@
             name = "LIRS Cache Segment Count",
             description = "The number of segments in the LIRS cache " +
                     "(default 16, a higher count means higher concurrency " +
-                    "but slightly lower cache hit rate)")
+                    "but slightly lower cache hit rate). " +
+                    "Only used when the LIRS cache is enabled (feature toggle " +
+                    FT_CAFFEINE_CACHE + " disabled); ignored when Caffeine is enabled. " +
+                    "Toggling the feature takes effect only after the DocumentNodeStore is restarted.")
     int cacheSegmentCount() default DEFAULT_CACHE_SEGMENT_COUNT;
 
     @AttributeDefinition(
             name = "LIRS Cache Stack Move Distance",
             description = "The delay to move entries to the head of the queue " +
                     "in the LIRS cache " +
                     "(default 16, a higher value means higher concurrency " +
-                    "but slightly lower cache hit rate)")
+                    "but slightly lower cache hit rate). " +
+                    "Only used when the LIRS cache is enabled (feature toggle " +
+                    FT_CAFFEINE_CACHE + " disabled); ignored when Caffeine is enabled. " +
+                    "Toggling the feature takes effect only after the DocumentNodeStore is restarted.")
     int cacheStackMoveDistance() default DEFAULT_CACHE_STACK_MOVE_DISTANCE;
 
     @AttributeDefinition(

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java

@@ -32,6 +32,7 @@
 import java.util.concurrent.CopyOnWriteArraySet;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 
@@ -104,10 +105,17 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
             "oak.documentMK.manyChildren", 50);
 
     /**
-     * Whether to use the CacheLIRS (default) or the Guava cache implementation.
+     * CacheLIRS is the default. The legacy guavaCache property selects the Caffeine
+     * SYNC fallback independently of the Caffeine feature toggle.
      */
     private static final boolean LIRS_CACHE = !Boolean.getBoolean("oak.documentMK.guavaCache");
 
+    static final String FT_CAFFEINE_CACHE = "FT_CAFFEINE_CACHE_OAK-12425";
+
+    // Changing the opt-in takes effect when a new builder constructs its caches.
+    static final AtomicBoolean FT_CAFFEINE_CACHE_ENABLED = new AtomicBoolean(
+            Boolean.getBoolean("oak.documentMK.caffeineCache"));
+
     /**
      * Number of content updates that need to happen before the updates
      * are automatically purged to the private branch.
@@ -131,6 +139,7 @@ public class DocumentNodeStoreBuilder<T extends DocumentNodeStoreBuilder<T>> {
     private Feature cancelInvalidationFeature;
     private Feature docStoreAvoidMergeLockFeature;
     private Feature prevNoPropCacheFeature;
+    private final Supplier<Boolean> caffeineCacheEnabled = Suppliers.memoize(FT_CAFFEINE_CACHE_ENABLED::get);
     private Weigher<CacheValue, CacheValue> weigher = new EmpiricalWeigher();
     private long memoryCacheSize = DEFAULT_MEMORY_CACHE_SIZE;
     private int nodeCachePercentage = DEFAULT_NODE_CACHE_PERCENTAGE;
@@ -1087,7 +1096,8 @@ private <K extends CacheValue, V extends CacheValue> Cache<K, V> buildCache(
             long maxWeight,
             final Set<EvictionListener<K, V>> listeners) {
         // do not use LIRS cache when maxWeight is zero (OAK-6953)
-        if (LIRS_CACHE && maxWeight > 0) {
+        boolean caffeine = caffeineCacheEnabled.get();
+        if (maxWeight > 0 && !caffeine && LIRS_CACHE) {
             return CacheLIRS.<K, V>newBuilder()
                     .module(module)
                     .weigher((key, value) -> weigher.weigh(key, value))
@@ -1105,15 +1115,15 @@ private <K extends CacheValue, V extends CacheValue> Cache<K, V> buildCache(
         }
         CacheBuilder<K, V> builder = CacheBuilder.<K, V>newBuilder()
                 .maximumWeight(maxWeight)
+                .maintenanceMode(CacheBuilder.MaintenanceMode.SYNC)
                 .weigher(weigher::weigh)
                 .recordStats();
-        if (!listeners.isEmpty()) {
-            builder = builder.evictionListener((k, v, cause) -> {
-                for (EvictionListener<K, V> l : listeners) {
-                    l.evicted(k, v, cause);
-                }
-            });
-        }
+        // PersistentCache registers its listener after the memory cache is built.
+        builder.evictionListener((k, v, cause) -> {
+            for (EvictionListener<K, V> l : listeners) {
+                l.evicted(k, v, cause);
+            }
+        });
         return builder.build();
     }
 

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java

@@ -95,6 +95,7 @@
 import org.apache.jackrabbit.oak.spi.state.RevisionGC;
 import org.apache.jackrabbit.oak.spi.state.RevisionGCMBean;
 import org.apache.jackrabbit.oak.spi.toggle.Feature;
+import org.apache.jackrabbit.oak.spi.toggle.FeatureToggle;
 import org.apache.jackrabbit.oak.spi.whiteboard.AbstractServiceTracker;
 import org.apache.jackrabbit.oak.spi.whiteboard.Registration;
 import org.apache.jackrabbit.oak.spi.whiteboard.Tracker;
@@ -306,6 +307,11 @@ protected void activate(ComponentContext context, Configuration config) throws E
         cancelInvalidationFeature = Feature.newFeature(FT_NAME_CANCEL_INVALIDATION, whiteboard);
         docStoreAvoidMergeLockFeature = Feature.newFeature(FT_NAME_AVOID_MERGE_LOCK, whiteboard);
         prevNoPropCacheFeature = Feature.newFeature(FT_NAME_PREV_NO_PROP_CACHE, whiteboard);
+        // Preserve the selected opt-in across the restart needed to change cache implementations.
+        addRegistration(whiteboard.register(FeatureToggle.class,
+                new FeatureToggle(DocumentNodeStoreBuilder.FT_CAFFEINE_CACHE,
+                        DocumentNodeStoreBuilder.FT_CAFFEINE_CACHE_ENABLED),
+                Collections.emptyMap()));
 
         registerNodeStoreIfPossible();
     }

oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentCacheConcurrencyTest.java

@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import java.io.File;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
+import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+/** Tests cross-cache loading in {@link DocumentNodeStore}. */
+public class DocumentCacheConcurrencyTest {
+
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target"));
+
+    /** Verifies a node read does not wait for an in-flight children load. */
+    @Test
+    public void coldNodeReadCompletesDuringChildrenLoad() throws Exception {
+        boolean previous = DocumentNodeStoreBuilder.FT_CAFFEINE_CACHE_ENABLED.get();
+        try {
+            for (boolean caffeine : new boolean[] {false, true}) {
+                DocumentNodeStoreBuilder.FT_CAFFEINE_CACHE_ENABLED.set(caffeine);
+                verifyColdNodeRead(caffeine);
+            }
+        } finally {
+            DocumentNodeStoreBuilder.FT_CAFFEINE_CACHE_ENABLED.set(previous);
+        }
+    }
+
+    private void verifyColdNodeRead(boolean caffeine) throws Exception {
+        PausingDocumentStore documents = new PausingDocumentStore();
+        DocumentNodeStoreBuilder<?> builder = new DocumentNodeStoreBuilder<>()
+                .setDocumentStore(documents)
+                .setAsyncDelay(0)
+                .setPersistentCache(temporaryFolder.newFolder().getAbsolutePath());
+        DocumentNodeStore store = builder.build();
+        ExecutorService executor = Executors.newFixedThreadPool(2);
+        try {
+            NodeBuilder root = store.getRoot().builder();
+            root.child("a");
+            store.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+            DocumentNodeState parent = store.getRoot();
+            store.getNodeCache().invalidateAll();
+            store.getNodeChildrenCache().invalidateAll();
+            documents.pause = true;
+            Future<DocumentNodeState.Children> children = executor.submit(() -> store.getChildren(parent, "", 10));
+            Assert.assertTrue("Children loader did not start: " + caffeine,
+                    documents.entered.await(10, TimeUnit.SECONDS));
+            Future<DocumentNodeState> node = executor.submit(() ->
+                    store.getNode(Path.fromString("/a"), parent.getLastRevision()));
+            Assert.assertNotNull("Node read must complete while children load is paused: " + caffeine,
+                    node.get(5, TimeUnit.SECONDS));
+            documents.release.countDown();
+            Assert.assertTrue(children.get(10, TimeUnit.SECONDS).children.contains("a"));
+        } finally {
+            // Stop the paused loader without entering the reverse dependency if the assertion fails.
+            documents.abort = true;
+            documents.release.countDown();
+            executor.shutdownNow();
+            Assert.assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
+            store.dispose();
+        }
+    }
+
+    private static final class PausingDocumentStore extends MemoryDocumentStore {
+        private final CountDownLatch entered = new CountDownLatch(1);
+        private final CountDownLatch release = new CountDownLatch(1);
+        private volatile boolean pause;
+        private volatile boolean abort;
+
+        @Override
+        public <T extends Document> List<T> query(Collection<T> collection, String fromKey, String toKey, int limit) {
+            if (pause && collection == Collection.NODES) {
+                entered.countDown();
+                try {
+                    if (!release.await(10, TimeUnit.SECONDS)) {
+                        throw new IllegalStateException("Timed out waiting to release children loader");
+                    }
+                } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                    throw new IllegalStateException("Children loader interrupted", e);
+                }
+                if (abort) {
+                    return Collections.emptyList();
+                }
+            }
+            return super.query(collection, fromKey, toKey, limit);
+        }
+    }
+}