Author: chetanm
Date: Thu Sep 15 07:21:48 2016
New Revision: 1760874

URL: http://svn.apache.org/viewvc?rev=1760874&view=rev
Log:
OAK-4412 - Lucene hybrid index

Refactor to move the logic around docs added to holder within holder. Going 
forward this would simplify logic if we use different approach for handling 
added docs say directly adding it to queue

Modified:
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorProvider.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java
    
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneDocumentHolder.java
    
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactoryTest.java

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorProvider.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorProvider.java?rev=1760874&r1=1760873&r2=1760874&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorProvider.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorProvider.java
 Thu Sep 15 07:21:48 2016
@@ -26,6 +26,7 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback;
 import org.apache.jackrabbit.oak.plugins.index.IndexingContext;
 import 
org.apache.jackrabbit.oak.plugins.index.lucene.hybrid.LocalIndexWriterFactory;
+import 
org.apache.jackrabbit.oak.plugins.index.lucene.hybrid.LuceneDocumentHolder;
 import 
org.apache.jackrabbit.oak.plugins.index.lucene.writer.DefaultIndexWriterFactory;
 import 
org.apache.jackrabbit.oak.plugins.index.lucene.writer.LuceneIndexWriterFactory;
 import org.apache.jackrabbit.oak.spi.commit.CommitContext;
@@ -117,7 +118,8 @@ public class LuceneIndexEditorProvider i
                     return null;
                 }
 
-                if 
(!indexingContext.getCommitInfo().getInfo().containsKey(CommitContext.NAME)){
+                CommitContext commitContext = 
getCommitContext(indexingContext);
+                if (commitContext == null){
                     //Logically there should not be any commit without commit 
context. But
                     //some initializer code does the commit with out it. So 
ignore such calls with
                     //warning now
@@ -128,7 +130,8 @@ public class LuceneIndexEditorProvider i
 
                 //TODO Also check if index has been done once
 
-                writerFactory = new LocalIndexWriterFactory(indexingContext, 
inMemoryDocsLimit);
+                writerFactory = new 
LocalIndexWriterFactory(getDocumentHolder(commitContext),
+                        indexingContext.getIndexPath());
 
                 //IndexDefinition from tracker might differ from one passed 
here for reindexing
                 //case which should be fine. However reusing existing 
definition would avoid
@@ -165,4 +168,18 @@ public class LuceneIndexEditorProvider i
     public void setInMemoryDocsLimit(int inMemoryDocsLimit) {
         this.inMemoryDocsLimit = inMemoryDocsLimit;
     }
+
+    private LuceneDocumentHolder getDocumentHolder(CommitContext 
commitContext){
+        LuceneDocumentHolder holder = (LuceneDocumentHolder) 
commitContext.get(LuceneDocumentHolder.NAME);
+        if (holder == null) {
+            holder = new LuceneDocumentHolder(inMemoryDocsLimit);
+            commitContext.set(LuceneDocumentHolder.NAME, holder);
+        }
+        return holder;
+    }
+
+    private static CommitContext getCommitContext(IndexingContext 
indexingContext) {
+        return (CommitContext) 
indexingContext.getCommitInfo().getInfo().get(CommitContext.NAME);
+    }
+
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java?rev=1760874&r1=1760873&r2=1760874&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactory.java
 Thu Sep 15 07:21:48 2016
@@ -20,42 +20,20 @@
 package org.apache.jackrabbit.oak.plugins.index.lucene.hybrid;
 
 import java.io.IOException;
-import java.util.List;
 
-import com.google.common.base.Preconditions;
-import org.apache.jackrabbit.oak.plugins.index.IndexingContext;
 import org.apache.jackrabbit.oak.plugins.index.lucene.IndexDefinition;
 import org.apache.jackrabbit.oak.plugins.index.lucene.writer.LuceneIndexWriter;
 import 
org.apache.jackrabbit.oak.plugins.index.lucene.writer.LuceneIndexWriterFactory;
-import org.apache.jackrabbit.oak.spi.commit.CommitContext;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.lucene.index.IndexableField;
 
 public class LocalIndexWriterFactory implements LuceneIndexWriterFactory {
-    public static final String COMMIT_PROCESSED_BY_LOCAL_LUCENE_EDITOR = 
"commitProcessedByLocalLuceneEditor";
-    private final IndexingContext indexingContext;
-    private final CommitContext commitContext;
-    private final int inMemoryDocsLimit;
-
-    public LocalIndexWriterFactory(IndexingContext indexingContext, int 
inMemoryDocsLimit) {
-        this.indexingContext = indexingContext;
-        this.commitContext = getCommitContext(indexingContext);
-        this.inMemoryDocsLimit = inMemoryDocsLimit;
-    }
+    private final LuceneDocumentHolder documentHolder;
+    private final String indexPath;
 
-    private LuceneDocumentHolder getDocumentHolder(){
-        LuceneDocumentHolder holder = (LuceneDocumentHolder) 
commitContext.get(LuceneDocumentHolder.NAME);
-        if (holder == null) {
-            //lazily initialize the holder
-            holder = new LuceneDocumentHolder();
-            commitContext.set(LuceneDocumentHolder.NAME, holder);
-        }
-        return holder;
-    }
-
-    private static CommitContext getCommitContext(IndexingContext 
indexingContext) {
-        CommitContext commitContext = (CommitContext) 
indexingContext.getCommitInfo().getInfo().get(CommitContext.NAME);
-        return Preconditions.checkNotNull(commitContext, "No commit context 
found in commit info");
+    public LocalIndexWriterFactory(LuceneDocumentHolder documentHolder, String 
indexPath) {
+        this.documentHolder = documentHolder;
+        this.indexPath = indexPath;
     }
 
     @Override
@@ -65,7 +43,6 @@ public class LocalIndexWriterFactory imp
 
     private class LocalIndexWriter implements LuceneIndexWriter {
         private final IndexDefinition definition;
-        private List<LuceneDoc> docList;
 
         public LocalIndexWriter(IndexDefinition definition) {
             this.definition = definition;
@@ -85,30 +62,13 @@ public class LocalIndexWriterFactory imp
 
         @Override
         public boolean close(long timestamp) throws IOException {
-            //This is used by testcase
-            commitContext.set(COMMIT_PROCESSED_BY_LOCAL_LUCENE_EDITOR, 
Boolean.TRUE);
+            documentHolder.done(indexPath);
             //always return false as nothing gets written to the index
             return false;
         }
 
         private void addLuceneDoc(LuceneDoc luceneDoc) {
-            if (docList == null){
-                if (definition.isSyncIndexingEnabled()){
-                    docList = 
getDocumentHolder().getSyncIndexedDocList(indexingContext.getIndexPath());
-                } else if (definition.isNRTIndexingEnabled()){
-                    docList = 
getDocumentHolder().getNRTIndexedDocList(indexingContext.getIndexPath());
-                } else {
-                    throw new IllegalStateException("Should not be invoked for 
any other indexing " +
-                            "mode apart from 'sync' and 'nrt'");
-                }
-            }
-
-            if (definition.isNRTIndexingEnabled()
-                    && 
getDocumentHolder().checkLimitAndLogWarning(inMemoryDocsLimit)){
-               return;
-            }
-
-            docList.add(luceneDoc);
+            documentHolder.add(definition.isSyncIndexingEnabled(), luceneDoc);
         }
     }
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneDocumentHolder.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneDocumentHolder.java?rev=1760874&r1=1760873&r2=1760874&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneDocumentHolder.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneDocumentHolder.java
 Thu Sep 15 07:21:48 2016
@@ -28,39 +28,62 @@ import com.google.common.collect.ListMul
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-class LuceneDocumentHolder {
+public class LuceneDocumentHolder {
     private static final Logger log = 
LoggerFactory.getLogger(LuceneDocumentHolder.class);
     public static final String NAME = "oak.lucene.documentHolder";
 
     private final ListMultimap<String, LuceneDoc> nrtIndexedList = 
ArrayListMultimap.create();
     private final ListMultimap<String, LuceneDoc> syncIndexedList = 
ArrayListMultimap.create();
+    private final int inMemoryDocsLimit;
     private boolean limitWarningLogged;
 
-    public List<LuceneDoc> getNRTIndexedDocList(String indexPath) {
-        return nrtIndexedList.get(indexPath);
+    public LuceneDocumentHolder(){
+        this(500);
     }
 
-    public Iterable<LuceneDoc> getNRTIndexedDocs(){
-        return nrtIndexedList.values();
+    public LuceneDocumentHolder(int inMemoryDocsLimit) {
+        this.inMemoryDocsLimit = inMemoryDocsLimit;
     }
 
-    public List<LuceneDoc> getSyncIndexedDocList(String indexPath) {
-        return syncIndexedList.get(indexPath);
+    public Iterable<LuceneDoc> getNRTIndexedDocs(){
+        return nrtIndexedList.values();
     }
 
     public Map<String, Collection<LuceneDoc>> getSyncIndexedDocs(){
         return syncIndexedList.asMap();
     }
 
-    public boolean checkLimitAndLogWarning(int maxSize){
-        if (nrtIndexedList.size() >= maxSize){
+    public void add(boolean sync, LuceneDoc doc) {
+        if (sync){
+            getSyncIndexedDocList(doc.indexPath).add(doc);
+        } else {
+            if (queueSizeWithinLimits()) {
+                getNRTIndexedDocList(doc.indexPath).add(doc);
+            }
+        }
+    }
+
+    public void done(String indexPath) {
+
+    }
+
+    List<LuceneDoc> getNRTIndexedDocList(String indexPath) {
+        return nrtIndexedList.get(indexPath);
+    }
+
+    List<LuceneDoc> getSyncIndexedDocList(String indexPath) {
+        return syncIndexedList.get(indexPath);
+    }
+
+    private boolean queueSizeWithinLimits(){
+        if (nrtIndexedList.size() >= inMemoryDocsLimit){
             if (!limitWarningLogged){
                 log.warn("Number of in memory documents meant for hybrid 
indexing has " +
-                        "exceeded limit [{}]. Some documents would be 
dropped", maxSize);
+                        "exceeded limit [{}]. Some documents would be 
dropped", inMemoryDocsLimit);
                 limitWarningLogged = true;
             }
-            return true;
+            return false;
         }
-        return false;
+        return true;
     }
 }

Modified: 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactoryTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactoryTest.java?rev=1760874&r1=1760873&r2=1760874&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactoryTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LocalIndexWriterFactoryTest.java
 Thu Sep 15 07:21:48 2016
@@ -25,7 +25,6 @@ import com.google.common.collect.Immutab
 import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.core.SimpleCommitContext;
-import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider;
 import org.apache.jackrabbit.oak.plugins.index.IndexUpdateProvider;
 import 
org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.IndexingMode;
 import 
org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexEditorProvider;
@@ -82,21 +81,7 @@ public class LocalIndexWriterFactoryTest
         //This is reindex case so nothing would be indexed
         //So now holder should be present in context
         assertNull(getHolder());
-        
assertNull(getCommitAttribute(LocalIndexWriterFactory.COMMIT_PROCESSED_BY_LOCAL_LUCENE_EDITOR));
-    }
-
-    @Test
-    public void holderNotInitializedUnlessIndexed() throws Exception{
-        NodeState indexed = createAndPopulateAsyncIndex(IndexingMode.NRT);
-        builder = indexed.builder();
-        builder.child("b");
-        NodeState after = builder.getNodeState();
-        syncHook.processCommit(indexed, after, newCommitInfo());
-
-        //This is incremental index case but no entry for fooIndex
-        //so holder should be null
-        assertNull(getHolder());
-        
assertNotNull(getCommitAttribute(LocalIndexWriterFactory.COMMIT_PROCESSED_BY_LOCAL_LUCENE_EDITOR));
+        assertNull(getCommitAttribute(LuceneDocumentHolder.NAME));
     }
 
     @Test


Reply via email to