[ 
https://issues.apache.org/jira/browse/OAK-8051?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16773796#comment-16773796
 ] 

Thomas Mueller commented on OAK-8051:
-------------------------------------

As far as I see, your patch is basically just this:
{noformat}
Index: 
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/persistentCache/CacheMap.java
     public CacheMap(MapFactory factory, String name, Builder<K, V> builder) {
         this.factory = factory;
         this.name = name;
         this.builder = builder;
         openMap();
+        if (this.map == null) {
+            throw new IllegalStateException("Could not open map for '" + name 
+ "'");
+        }
     }


Index: 
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/persistentCache/CacheTest.java
     @Test
+    @Ignore("OAK-8051")
     public void recoverIfCorrupt() throws Exception {
         FileUtils.deleteDirectory(new File("target/cacheTest"));
         new File("target/cacheTest").mkdirs();
{noformat}

The test fails because CacheMap cannot be instantiated.

The idea behind CacheMap reopening attempts is that:
* (A) it tries 10 time to reopen if there was an exception (like: interrupted 
exception, caused by Thread.interrupt)
* (B) if reopening failed 10 times, then it goes in a "closed" mode where it's 
still usable but doesn't do anything (doesn't cache anything)

Your patch breaks both (A) and (B), in case Thread.interrupt was called 
slightly before opening CacheMap.

What's wrong with the current code is that:
* A NPE is logged in CacheMap, which can be confusing
* It would be better if 10 retries are attempted immediately in the constructor 
of CacheMap, instead of later on when trying to use the CacheMap

Here a patch that should do this (a test case is needed):

{noformat}
svn diff -x --ignore-all-space

Index: 
src/main/java/org/apache/jackrabbit/oak/plugins/document/persistentCache/CacheMap.java
===================================================================
--- 
src/main/java/org/apache/jackrabbit/oak/plugins/document/persistentCache/CacheMap.java
      (revision 1853143)
+++ 
src/main/java/org/apache/jackrabbit/oak/plugins/document/persistentCache/CacheMap.java
      (working copy)
@@ -46,7 +46,14 @@
         this.name = name;
         this.builder = builder;
         openMap();
+        // OAK-8051: if opening failed, immediately try to re-open,
+        // until either the the map is closed, or open
+        for (int i = 0; map == null && !closed; i++) {
+            if (map == null) {
+                reopen(i, null);
     }
+        }
+    }
{noformat}

So the changes are:

* if map is null, retry until either map is not null or it's closed
* no NPE is logged

> PersistentCache: error during open can lead to incomplete initialization and 
> subsequent NPEs
> --------------------------------------------------------------------------------------------
>
>                 Key: OAK-8051
>                 URL: https://issues.apache.org/jira/browse/OAK-8051
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: documentmk
>    Affects Versions: 1.6.6
>            Reporter: Julian Reschke
>            Priority: Major
>              Labels: candidate_oak_1_10, candidate_oak_1_6, 
> candidate_oak_1_8, patch-available
>             Fix For: 1.12
>
>         Attachments: OAK-8051.diff
>
>
> Seen in the wild (in 1.6.6):
> {noformat}
> 22.01.2019 08:45:13.153 *WARN* [http-/0.0.0.0:80-3] 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.MapFactory Could 
> not open the store _path_/cache-4.data
> java.lang.IllegalStateException: The file is locked: nio:_path_/cache-4.data 
> [1.4.193/7]
>       at org.h2.mvstore.DataUtils.newIllegalStateException(DataUtils.java:765)
>       at org.h2.mvstore.FileStore.open(FileStore.java:168)
>       at org.h2.mvstore.MVStore.<init>(MVStore.java:348)
>       at org.h2.mvstore.MVStore$Builder.open(MVStore.java:2923)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache$1.openStore(PersistentCache.java:288)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache.createMapFactory(PersistentCache.java:361)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache.<init>(PersistentCache.java:210)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.getPersistentCache(DocumentMK.java:1232)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.buildCache(DocumentMK.java:1211)
> {noformat}
> Later on:
> {noformat}
> 22.01.2019 08:45:13.155 *WARN* [http-/0.0.0.0:80-3] 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.MapFactory Could 
> not open the map
> java.lang.NullPointerException: null
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache$1.openMap(PersistentCache.java:335)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.CacheMap.openMap(CacheMap.java:135)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.CacheMap.<init>(CacheMap.java:48)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache.openMap(PersistentCache.java:468)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.NodeCache.addGeneration(NodeCache.java:115)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache.initGenerationCache(PersistentCache.java:452)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCache.wrap(PersistentCache.java:443)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.buildCache(DocumentMK.java:1214)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.buildPrevDocumentsCache(DocumentMK.java:1182)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.buildNodeDocumentCache(DocumentMK.java:1189)
>       at 
> org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.initialize(RDBDocumentStore.java:798)
>       at 
> org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.<init>(RDBDocumentStore.java:212)
>       at 
> org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.<init>(RDBDocumentStore.java:224)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.setRDBConnection(DocumentMK.java:757)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.registerNodeStore(DocumentNodeStoreService.java:508)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.registerNodeStoreIfPossible(DocumentNodeStoreService.java:430)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.activate(DocumentNodeStoreService.java:414)
> {noformat}
> and then
> {noformat}
> 22.01.2019 08:45:16.808 *WARN* [http-/0.0.0.0:80-3] 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.CacheMap 
> Re-opening map PREV_DOCUMENT
> java.lang.NullPointerException: null
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.CacheMap.get(CacheMap.java:87)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.MultiGenerationMap.readValue(MultiGenerationMap.java:71)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.NodeCache.asyncReadIfPresent(NodeCache.java:147)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.NodeCache.readIfPresent(NodeCache.java:130)
>       at 
> org.apache.jackrabbit.oak.plugins.document.persistentCache.NodeCache.getIfPresent(NodeCache.java:213)
>       at 
> org.apache.jackrabbit.oak.plugins.document.cache.NodeDocumentCache.getIfPresent(NodeDocumentCache.java:155)
>       at 
> org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.readDocumentCached(RDBDocumentStore.java:1130)
>       at 
> org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.find(RDBDocumentStore.java:234)
>       at 
> org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.find(RDBDocumentStore.java:229)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocument.getPreviousDocument(NodeDocument.java:1338)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocument.getPreviousDocs(NodeDocument.java:1307)
>       at 
> org.apache.jackrabbit.oak.plugins.document.ValueMap$2.containsKey(ValueMap.java:170)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocument.getPreviousDocs(NodeDocument.java:1309)
>       at 
> org.apache.jackrabbit.oak.plugins.document.ValueMap$2.containsKey(ValueMap.java:170)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocument.getPreviousDocs(NodeDocument.java:1309)
>       at 
> org.apache.jackrabbit.oak.plugins.document.NodeDocument.isCommitted(NodeDocument.java:547)
>       at 
> org.apache.jackrabbit.oak.plugins.document.LastRevRecoveryAgent.determineLastModification(LastRevRecoveryAgent.java:402)
>       at 
> org.apache.jackrabbit.oak.plugins.document.LastRevRecoveryAgent.recover(LastRevRecoveryAgent.java:191)
>       at 
> org.apache.jackrabbit.oak.plugins.document.LastRevRecoveryAgent.recover(LastRevRecoveryAgent.java:156)
>       at 
> org.apache.jackrabbit.oak.plugins.document.LastRevRecoveryAgent.recoverCandidates(LastRevRecoveryAgent.java:369)
>       at 
> org.apache.jackrabbit.oak.plugins.document.LastRevRecoveryAgent.recover(LastRevRecoveryAgent.java:128)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore.checkLastRevRecovery(DocumentNodeStore.java:646)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore.<init>(DocumentNodeStore.java:564)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.getNodeStore(DocumentMK.java:856)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.registerNodeStore(DocumentNodeStoreService.java:551)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.registerNodeStoreIfPossible(DocumentNodeStoreService.java:430)
>       at 
> org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreService.activate(DocumentNodeStoreService.java:414)
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to