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/09/15 07:21:48 UTC
svn commit: r1760874 - in /jackrabbit/oak/trunk/oak-lucene/src:
main/java/org/apache/jackrabbit/oak/plugins/index/lucene/
main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/
test/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/
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