You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2016/02/22 12:28:48 UTC

svn commit: r1731627 - in /jackrabbit/oak/trunk/oak-lucene/src: main/java/org/apache/jackrabbit/oak/plugins/index/lucene/ test/java/org/apache/jackrabbit/oak/plugins/index/lucene/

Author: chetanm
Date: Mon Feb 22 11:28:48 2016
New Revision: 1731627

URL: http://svn.apache.org/viewvc?rev=1731627&view=rev
Log:
OAK-4024 - CoW uses incorrect directory on re-indexing when indexPath property is used

Applying patch from Vikas with slight modification in the approach. In reindex case we refresh the IndexDefinition based on update NodeBuilder state. Going forward we should change the IndexDefinition to work with NodeBuilder

Modified:
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopier.java
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorContext.java
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopier.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopier.java?rev=1731627&r1=1731626&r2=1731627&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopier.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopier.java Mon Feb 22 11:28:48 2016
@@ -297,7 +297,7 @@ public class IndexCopier implements Copy
      * Directory implementation which lazily copies the index files from a
      * remote directory in background.
      */
-    private class CopyOnReadDirectory extends FilterDirectory {
+    class CopyOnReadDirectory extends FilterDirectory {
         private final Directory remote;
         private final Directory local;
         private final String indexPath;
@@ -384,6 +384,10 @@ public class IndexCopier implements Copy
             return remote.openInput(name, context);
         }
 
+        Directory getLocal() {
+            return local;
+        }
+
         private void copy(final CORFileReference reference) {
             updateMaxScheduled(scheduledForCopyCount.incrementAndGet());
             executor.execute(new Runnable() {

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorContext.java?rev=1731627&r1=1731626&r2=1731627&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorContext.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorContext.java Mon Feb 22 11:28:48 2016
@@ -125,7 +125,7 @@ public class LuceneIndexEditorContext {
 
     private static final Parser defaultParser = createDefaultParser();
 
-    private final IndexDefinition definition;
+    private IndexDefinition definition;
 
     private final NodeBuilder definitionBuilder;
 
@@ -149,6 +149,8 @@ public class LuceneIndexEditorContext {
     private final ExtractedTextCache extractedTextCache;
 
     private final IndexAugmentorFactory augmentorFactory;
+
+    private final NodeState root;
     /**
      * The media types supported by the parser used.
      */
@@ -157,6 +159,7 @@ public class LuceneIndexEditorContext {
     LuceneIndexEditorContext(NodeState root, NodeBuilder definition, IndexUpdateCallback updateCallback,
                              @Nullable IndexCopier indexCopier, ExtractedTextCache extractedTextCache,
                              IndexAugmentorFactory augmentorFactory) {
+        this.root = root;
         this.definitionBuilder = definition;
         this.indexCopier = indexCopier;
         this.definition = new IndexDefinition(root, definition);
@@ -309,6 +312,9 @@ public class LuceneIndexEditorContext {
         reindex = true;
         IndexFormatVersion version = IndexDefinition.determineVersionForFreshIndex(definitionBuilder);
         definitionBuilder.setProperty(IndexDefinition.INDEX_VERSION, version.getVersion());
+
+        //Refresh the index definition based on update builder state
+        definition = new IndexDefinition(root, definitionBuilder);
     }
 
     public long incIndexedNodes() {

Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java?rev=1731627&r1=1731626&r2=1731627&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java Mon Feb 22 11:28:48 2016
@@ -68,6 +68,9 @@ import org.apache.jackrabbit.oak.spi.com
 import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
 import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
 import org.apache.jackrabbit.util.ISO8601;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import org.apache.lucene.store.FilterDirectory;
 import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;
@@ -106,6 +109,8 @@ import static org.apache.jackrabbit.oak.
 import static org.hamcrest.CoreMatchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.hamcrest.CoreMatchers.containsString;
@@ -121,6 +126,9 @@ public class LucenePropertyIndexTest ext
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
+    private String corDir = null;
+    private String cowDir = null;
+
     private LuceneIndexEditorProvider editorProvider;
 
     @Override
@@ -130,8 +138,9 @@ public class LucenePropertyIndexTest ext
 
     @Override
     protected ContentRepository createRepository() {
-        editorProvider = new LuceneIndexEditorProvider(createIndexCopier(), new ExtractedTextCache(10* FileUtils.ONE_MB, 100));
-        LuceneIndexProvider provider = new LuceneIndexProvider();
+        IndexCopier copier = createIndexCopier();
+        editorProvider = new LuceneIndexEditorProvider(copier, new ExtractedTextCache(10* FileUtils.ONE_MB, 100));
+        LuceneIndexProvider provider = new LuceneIndexProvider(copier);
         return new Oak()
                 .with(new InitialContent())
                 .with(new OpenSecurityProvider())
@@ -145,7 +154,44 @@ public class LucenePropertyIndexTest ext
 
     private IndexCopier createIndexCopier() {
         try {
-            return new IndexCopier(executorService, temporaryFolder.getRoot());
+            return new IndexCopier(executorService, temporaryFolder.getRoot()) {
+                @Override
+                public Directory wrapForRead(String indexPath, IndexDefinition definition,
+                                             Directory remote) throws IOException {
+                    Directory ret = super.wrapForRead(indexPath, definition, remote);
+                    corDir = getFSDirPath(ret);
+                    return ret;
+                }
+
+                @Override
+                public Directory wrapForWrite(IndexDefinition definition,
+                                              Directory remote, boolean reindexMode) throws IOException {
+                    Directory ret = super.wrapForWrite(definition, remote, reindexMode);
+                    cowDir = getFSDirPath(ret);
+                    return ret;
+                }
+
+                private String getFSDirPath(Directory dir){
+                    if (dir instanceof IndexCopier.CopyOnReadDirectory){
+                        dir = ((CopyOnReadDirectory) dir).getLocal();
+                    }
+
+                    dir = unwrap(dir);
+
+                    if (dir instanceof FSDirectory){
+                        return ((FSDirectory) dir).getDirectory().getAbsolutePath();
+                    }
+                    return null;
+                }
+
+                private Directory unwrap(Directory dir){
+                    if (dir instanceof FilterDirectory){
+                        return unwrap(((FilterDirectory) dir).getDelegate());
+                    }
+                    return dir;
+                }
+
+            };
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
@@ -1917,6 +1963,58 @@ public class LucenePropertyIndexTest ext
         assertQuery(propabQuery, asList("/test/a"));
     }
 
+    //OAK-4024
+    @Test
+    public void reindexWithCOWWithIndexPath() throws Exception {
+        Tree idx = createIndex("test1", of("propa", "propb"));
+        idx.setProperty(LuceneIndexConstants.INDEX_PATH, "/oak:index/test1");
+        Tree props = TestUtil.newRulePropTree(idx, "mix:title");
+        Tree prop1 = props.addChild(TestUtil.unique("prop"));
+        prop1.setProperty(LuceneIndexConstants.PROP_NAME, "jcr:title");
+        prop1.setProperty(LuceneIndexConstants.PROP_PROPERTY_INDEX, true);
+        root.commit();
+
+        //force CoR
+        executeQuery("SELECT * FROM [mix:title]", SQL2);
+
+        assertNotNull(corDir);
+        String localPathBeforeReindex = corDir;
+
+        //CoW with re-indexing
+        idx.setProperty("reindex", true);
+        root.commit();
+
+        assertNotNull(cowDir);
+        String localPathAfterReindex = cowDir;
+
+        assertNotEquals("CoW should write to different dir on reindexing", localPathBeforeReindex, localPathAfterReindex);
+    }
+
+    @Test
+    public void reindexWithCOWWithoutIndexPath() throws Exception {
+        Tree idx = createIndex("test1", of("propa", "propb"));
+        Tree props = TestUtil.newRulePropTree(idx, "mix:title");
+        Tree prop1 = props.addChild(TestUtil.unique("prop"));
+        prop1.setProperty(LuceneIndexConstants.PROP_NAME, "jcr:title");
+        prop1.setProperty(LuceneIndexConstants.PROP_PROPERTY_INDEX, true);
+        root.commit();
+
+        //force CoR
+        executeQuery("SELECT * FROM [mix:title]", SQL2);
+
+        assertNotNull(corDir);
+        String localPathBeforeReindex = corDir;
+
+        //CoW with re-indexing
+        idx.setProperty("reindex", true);
+        root.commit();
+
+        assertNotNull(cowDir);
+        String localPathAfterReindex = cowDir;
+
+        assertNotEquals("CoW should write to different dir on reindexing", localPathBeforeReindex, localPathAfterReindex);
+    }
+
     @Test
     public void fulltextQueryWithSpecialChars() throws Exception{
         Tree idx = createIndex("test1", of("propa", "propb"));