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/05/05 07:22:27 UTC
svn commit: r1742363 - 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: Thu May 5 07:22:27 2016
New Revision: 1742363
URL: http://svn.apache.org/viewvc?rev=1742363&view=rev
Log:
OAK-4347 - Use the indexPath from hidden property instead of taking this as input as part of index config
-- Refactored logic which had to take care of case when index path can be optional. Now as indexPath would always be there such cases would not occur
-- Index path would not be determined either from builder or nodeState.
-- Props like 'name' and 'indexPath' would not be used
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/IndexDefinition.java
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.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=1742363&r1=1742362&r2=1742363&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 Thu May 5 07:22:27 2016
@@ -161,29 +161,15 @@ public class IndexCopier implements Copy
protected Directory createLocalDirForIndexWriter(IndexDefinition definition) throws IOException {
String indexPath = definition.getIndexPathFromConfig();
- File indexWriterDir;
- if (indexPath == null){
- //If indexPath is not known create a unique directory for work
- indexWriterDir = new File(indexWorkDir, String.valueOf(UNIQUE_COUNTER.incrementAndGet()));
- } else {
- File indexDir = getIndexDir(indexPath);
- String newVersion = String.valueOf(definition.getReindexCount());
- indexWriterDir = getVersionedDir(indexPath, indexDir, newVersion);
- }
+ File indexDir = getIndexDir(indexPath);
+ String newVersion = String.valueOf(definition.getReindexCount());
+ File indexWriterDir = getVersionedDir(indexPath, indexDir, newVersion);
//By design indexing in Oak is single threaded so Lucene locking
//can be disabled
Directory dir = FSDirectory.open(indexWriterDir, NoLockFactory.getNoLockFactory());
log.debug("IndexWriter would use {}", indexWriterDir);
-
- if (indexPath == null) {
- dir = new DeleteOldDirOnClose(dir, indexWriterDir);
- log.debug("IndexPath [{}] not configured in index definition {}. Writer would create index " +
- "files in temporary dir {} which would be deleted upon close. For better performance do " +
- "configure the 'indexPath' as part of your index definition", LuceneIndexConstants.INDEX_PATH,
- definition, indexWriterDir);
- }
return dir;
}
@@ -255,12 +241,6 @@ public class IndexCopier implements Copy
private Set<String> getSharedWorkingSet(IndexDefinition defn){
String indexPath = defn.getIndexPathFromConfig();
- if (indexPath == null){
- //With indexPath null the working directory would not
- //be shared between COR and COW. So just return a new set
- return new HashSet<String>();
- }
-
Set<String> sharedSet;
synchronized (sharedWorkingSetMap){
sharedSet = sharedWorkingSetMap.get(indexPath);
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java Thu May 5 07:22:27 2016
@@ -68,6 +68,7 @@ import org.apache.lucene.codecs.Codec;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Lists.newArrayListWithCapacity;
import static com.google.common.collect.Maps.newHashMap;
@@ -78,6 +79,7 @@ import static org.apache.jackrabbit.oak.
import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.DECLARING_NODE_TYPES;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.ENTRY_COUNT_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_PATH;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_COUNT;
import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.*;
import static org.apache.jackrabbit.oak.plugins.index.lucene.PropertyDefinition.DEFAULT_BOOST;
@@ -216,23 +218,23 @@ class IndexDefinition implements Aggrega
private final boolean spellcheckEnabled;
- public IndexDefinition(NodeState root, NodeState defn) {
- this(root, defn, null);
- }
+ private final String indexPath;
public IndexDefinition(NodeState root, NodeBuilder defn) {
- this(root, defn.getBaseState(), defn, null);
+ this(root, defn.getBaseState(), defn);
}
- public IndexDefinition(NodeState root, NodeState defn, @Nullable String indexPath) {
- this(root, defn, null, indexPath);
+ public IndexDefinition(NodeState root, NodeState defn) {
+ this(root, defn, null);
}
- public IndexDefinition(NodeState root, NodeState defn, @Nullable NodeBuilder defnb, @Nullable String indexPath) {
+ public IndexDefinition(NodeState root, NodeState defn, @Nullable NodeBuilder defnb) {
this.root = root;
this.version = determineIndexFormatVersion(defn, defnb);
this.definition = defn;
- this.indexName = determineIndexName(defn, indexPath);
+ this.indexPath = determineIndexPath(defn, defnb);
+ this.indexName = indexPath;
+
this.blobSize = getOptionalValue(defn, BLOB_SIZE, DEFAULT_BLOB_SIZE);
this.activeDelete = getOptionalValue(defn, ACTIVE_DELETE, DEFAULT_ACTIVE_DELETE);
this.testMode = getOptionalValue(defn, LuceneIndexConstants.TEST_MODE, false);
@@ -658,9 +660,8 @@ class IndexDefinition implements Aggrega
return spellcheckEnabled;
}
- @CheckForNull
public String getIndexPathFromConfig() {
- return definition.getString(LuceneIndexConstants.INDEX_PATH);
+ return checkNotNull(indexPath, "Index path property [%s] not found", IndexConstants.INDEX_PATH);
}
private boolean evaluateSuggestAnalyzed(NodeState defn, boolean defaultValue) {
@@ -1147,7 +1148,7 @@ class IndexDefinition implements Aggrega
indexDefn.removeProperty(ORDERED_PROP_NAMES);
indexDefn.removeProperty(FULL_TEXT_ENABLED);
indexDefn.child(PROP_NODE).remove();
- log.info("Updated index definition for {}", determineIndexName(defn, null));
+ log.info("Updated index definition for {}", indexDefn.getString(INDEX_PATH));
}
return indexDefn;
}
@@ -1303,22 +1304,12 @@ class IndexDefinition implements Aggrega
return codec;
}
- private static String determineIndexName(NodeState defn, String indexPath) {
- if (indexPath == null){
- indexPath = defn.getString(LuceneIndexConstants.INDEX_PATH);
- }
- String indexName = defn.getString(PROP_NAME);
- if (indexName == null){
- if (indexPath != null) {
- return indexPath;
- }
- return "<No 'name' property defined>";
- }
-
- if (indexPath != null){
- return indexName + "(" + indexPath + ")";
+ private static String determineIndexPath(NodeState defn, @Nullable NodeBuilder defnb) {
+ String indexPath = defn.getString(IndexConstants.INDEX_PATH);
+ if (indexPath == null && defnb != null){
+ indexPath = defnb.getString(IndexConstants.INDEX_PATH);
}
- return indexName;
+ return indexPath;
}
private static Set<String> getMultiProperty(NodeState definition, String propName){
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexNode.java Thu May 5 07:22:27 2016
@@ -45,7 +45,7 @@ class IndexNode {
static IndexNode open(String indexPath, NodeState root, NodeState defnNodeState, @Nullable IndexCopier cloner)
throws IOException {
Directory directory = null;
- IndexDefinition definition = new IndexDefinition(root, defnNodeState, indexPath);
+ IndexDefinition definition = new IndexDefinition(root, defnNodeState);
NodeState data = defnNodeState.getChildNode(INDEX_DATA_CHILD_NAME);
if (data.exists()) {
directory = new OakDirectory(new ReadOnlyBuilder(defnNodeState), definition, true);
Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java Thu May 5 07:22:27 2016
@@ -294,7 +294,10 @@ public interface LuceneIndexConstants {
* definition is defined is not known to IndexEditor. To make use of CopyOnWrite
* feature its required to know the indexPath to optimize the lookup and read of
* existing index files
+ *
+ * @deprecated With OAK-4152 no need to explicitly define indexPath property
*/
+ @Deprecated
String INDEX_PATH = "indexPath";
/**
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierTest.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexCopierTest.java Thu May 5 07:22:27 2016
@@ -52,6 +52,7 @@ import com.google.common.util.concurrent
import com.google.common.util.concurrent.MoreExecutors;
import org.apache.commons.io.FileUtils;
import org.apache.jackrabbit.oak.commons.IOUtils;
+import org.apache.jackrabbit.oak.plugins.index.IndexConstants;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.lucene.store.Directory;
@@ -60,6 +61,7 @@ import org.apache.lucene.store.IOContext
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.RAMDirectory;
+import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -90,6 +92,11 @@ public class IndexCopierTest {
private NodeBuilder builder = root.builder();
+ @Before
+ public void setUp(){
+ builder.setProperty(IndexConstants.INDEX_PATH, "/oak:index/test");
+ }
+
@Test
public void basicTest() throws Exception{
Directory baseDir = new RAMDirectory();
@@ -563,7 +570,7 @@ public class IndexCopierTest {
IndexCopier copier = new IndexCopier(sameThreadExecutor(), getWorkDir());
- builder.setProperty(LuceneIndexConstants.INDEX_PATH, "foo");
+ builder.setProperty(IndexConstants.INDEX_PATH, "foo");
IndexDefinition defn = new IndexDefinition(root, builder.getNodeState());
Directory dir = copier.wrapForWrite(defn, remote, false);
@@ -937,7 +944,7 @@ public class IndexCopierTest {
Directory baseDir = new CloseSafeDir();
String indexPath = "/foo";
- builder.setProperty(LuceneIndexConstants.INDEX_PATH, indexPath);
+ builder.setProperty(IndexConstants.INDEX_PATH, indexPath);
IndexDefinition defn = new IndexDefinition(root, builder.getNodeState());
IndexCopier copier = new RAMIndexCopier(baseDir, executor, getWorkDir(), true);
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java Thu May 5 07:22:27 2016
@@ -349,7 +349,7 @@ public class LuceneIndexEditorTest {
NodeBuilder index = builder.child(INDEX_DEFINITIONS_NAME);
NodeBuilder nb = newLuceneIndexDefinitionV2(index, "lucene", of(TYPENAME_STRING));
- nb.setProperty(LuceneIndexConstants.INDEX_PATH, "foo");
+ nb.setProperty(IndexConstants.INDEX_PATH, "foo");
IndexUtils.createIndexDefinition(index, "failingIndex", false, false, of("foo"), null);
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java Thu May 5 07:22:27 2016
@@ -881,6 +881,22 @@ public class LuceneIndexTest {
}
}
+ @Test
+ public void indexNameIsIndexPath() throws Exception {
+ NodeBuilder index = builder.child(INDEX_DEFINITIONS_NAME);
+ newLucenePropertyIndexDefinition(index, "lucene", ImmutableSet.of("foo"), null);
+
+ NodeState before = builder.getNodeState();
+ builder.setProperty("foo", "bar");
+ NodeState after = builder.getNodeState();
+
+ NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY);
+
+ IndexDefinition defn = new IndexDefinition(root, indexed.getChildNode("oak:index").getChildNode("lucene"));
+ assertEquals("/oak:index/lucene", defn.getIndexName());
+ assertEquals("/oak:index/lucene", defn.getIndexPathFromConfig());
+ }
+
@After
public void cleanUp(){
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=1742363&r1=1742362&r2=1742363&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 Thu May 5 07:22:27 2016
@@ -2004,33 +2004,6 @@ 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"));
Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java?rev=1742363&r1=1742362&r2=1742363&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java Thu May 5 07:22:27 2016
@@ -32,6 +32,7 @@ import org.apache.commons.io.IOUtils;
import org.apache.commons.io.input.NullInputStream;
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.plugins.index.IndexConstants;
import org.apache.jackrabbit.oak.plugins.memory.ArrayBasedBlob;
import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState;
import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
@@ -336,7 +337,7 @@ public class OakDirectoryTest {
@Test
public void dirNameInExceptionMessage() throws Exception{
String indexPath = "/foo/bar";
- builder.setProperty(LuceneIndexConstants.INDEX_PATH, indexPath);
+ builder.setProperty(IndexConstants.INDEX_PATH, indexPath);
Directory dir = createDir(builder, false);
try {
@@ -382,7 +383,7 @@ public class OakDirectoryTest {
int blobSize = minFileSize + 1000;
builder = nodeStore.getRoot().builder();
- builder.setProperty(LuceneIndexConstants.INDEX_PATH, indexPath);
+ builder.setProperty(IndexConstants.INDEX_PATH, indexPath);
builder.setProperty(LuceneIndexConstants.BLOB_SIZE, blobSize);
Directory dir = createDir(builder, false);