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 mk...@apache.org on 2021/09/13 10:09:18 UTC
[jackrabbit-oak] branch trunk updated: OAK-9552: Don't index except
if it's oak:QueryIndexDefinition (#367)
This is an automated email from the ASF dual-hosted git repository.
mkataria pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 0558bf2 OAK-9552: Don't index except if it's oak:QueryIndexDefinition (#367)
0558bf2 is described below
commit 0558bf2382de3a6615db7f7524fc625ce809b1f4
Author: Mohit Kataria <mk...@apache.org>
AuthorDate: Mon Sep 13 15:38:48 2021 +0530
OAK-9552: Don't index except if it's oak:QueryIndexDefinition (#367)
---
.../jackrabbit/oak/plugins/index/IndexUpdate.java | 25 ++-
.../oak/plugins/index/AsyncIndexUpdateTest.java | 3 +-
.../oak/plugins/index/lucene/InvalidIndexTest.java | 198 +++++++++++++++++++++
.../index/lucene/SuggestionIntervalTest.java | 2 +-
.../plugins/index/elastic/ElasticFacetTest.java | 3 +-
.../index/elastic/ElasticSpellcheckTest.java | 3 +-
.../jackrabbit/oak/plugins/index/IndexOptions.java | 3 +-
.../oak/plugins/index/SecureFacetCommonTest.java | 1 +
8 files changed, 232 insertions(+), 6 deletions(-)
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java
index b4cce77..80a18e0 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java
@@ -46,6 +46,7 @@ import java.util.Set;
import com.google.common.collect.Iterables;
+import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
@@ -73,6 +74,14 @@ public class IndexUpdate implements Editor, PathSource {
private static final Logger log = LoggerFactory.getLogger(IndexUpdate.class);
private static final String TYPE_ELASTICSEARCH = "elasticsearch";
+ //This is used so that wrong index definitions are sparsely logged. After every 1000 indexing cycles, index definitions
+ // with wrong nodetype will be logged.
+ public static final long INDEX_JCR_TYPE_INVALID_LOG_LIMITER = Long.parseLong(System.getProperty("oak.indexer.indexJcrTypeInvalidLogLimiter", "1000"));
+
+ // Initial value is set at indexJcrTypeInvalidLogLimiter so that first logging start on first cycle/update itself.
+ // This counter is cyclically incremented till indexJcrTypeInvalidLogLimiter and then reset to 0
+ private static volatile long cyclicExecutionCount = INDEX_JCR_TYPE_INVALID_LOG_LIMITER;
+
/**
* <p>
* The value of this flag determines the behavior of the IndexUpdate when
@@ -124,7 +133,6 @@ public class IndexUpdate implements Editor, PathSource {
*/
private final Map<String, Editor> reindex = new HashMap<String, Editor>();
-
public IndexUpdate(
IndexEditorProvider provider, String async,
NodeState root, NodeBuilder builder,
@@ -280,10 +288,25 @@ public class IndexUpdate implements Editor, PathSource {
NodeBuilder definition = definitions.getChildNode(name);
if (isIncluded(rootState.async, definition)) {
String type = definition.getString(TYPE_PROPERTY_NAME);
+ String primaryType = definition.getName(JcrConstants.JCR_PRIMARYTYPE);
if (type == null) {
// probably not an index def
continue;
}
+ /*
+ Log a warning after every indexJcrTypeInvalidLogLimiter cycles of indexer where nodeState changed.
+ and skip further execution for invalid nodetype of index definition.
+ */
+ if (!IndexConstants.INDEX_DEFINITIONS_NODE_TYPE.equals(primaryType)) {
+ // It is a cyclic counter which reset back to 0 after INDEX_JCR_TYPE_INVALID_LOG_LIMITER
+ // This is to sparsely log this warning.
+ if ((cyclicExecutionCount >= INDEX_JCR_TYPE_INVALID_LOG_LIMITER)) {
+ log.warn("jcr:primaryType of index {} should be {} instead of {}", name, IndexConstants.INDEX_DEFINITIONS_NODE_TYPE, primaryType);
+ cyclicExecutionCount = 0;
+ }
+ cyclicExecutionCount++;
+ continue;
+ }
boolean shouldReindex = shouldReindex(definition, before, name);
String indexPath = getIndexPath(getPath(), name);
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java
index 1fdcce4..c8ed74b 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java
@@ -2107,7 +2107,8 @@ public class AsyncIndexUpdateTest {
// Make index
NodeBuilder builder = store.getRoot().builder();
- builder.child("oak:index").child("fooIndex").setProperty("async", "async").setProperty("type", "foo");
+ builder.child("oak:index").child("fooIndex").setProperty("async", "async").setProperty("type", "foo")
+ .setProperty("jcr:primaryType", "oak:QueryIndexDefinition", Type.NAME);
store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
// reset stats
diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/InvalidIndexTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/InvalidIndexTest.java
new file mode 100644
index 0000000..7a2563b
--- /dev/null
+++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/InvalidIndexTest.java
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.jackrabbit.oak.plugins.index.lucene;
+
+import ch.qos.logback.classic.Level;
+import org.apache.jackrabbit.oak.InitialContent;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.*;
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.apache.jackrabbit.oak.plugins.index.AsyncIndexUpdate;
+import org.apache.jackrabbit.oak.plugins.index.IndexUpdate;
+import org.apache.jackrabbit.oak.plugins.index.TrackingCorruptIndexHandler;
+import org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.nodetype.NodeTypeIndexProvider;
+import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider;
+import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexProvider;
+import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.spi.blob.MemoryBlobStore;
+import org.apache.jackrabbit.oak.spi.commit.Observer;
+import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
+import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import static com.google.common.collect.Lists.newArrayList;
+import static org.apache.jackrabbit.oak.plugins.index.CompositeIndexEditorProvider.compose;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+/**
+ * Tests: Logging of invalid jcr:primaryType of index.
+ */
+public class InvalidIndexTest {
+
+ private final long INDEX_CORRUPT_INTERVAL_IN_MILLIS = 100;
+ private MemoryBlobStore blobStore;
+
+ protected Root root;
+
+ private AsyncIndexUpdate asyncIndexUpdate;
+
+ private static final String invalidJcrPrimaryTypeIndexName = "myTestIndexWithWrongJcrPrimaryType";
+ private static final String invalidJcrPrimaryTypeForIndex = NodeTypeConstants.NT_OAK_UNSTRUCTURED;
+
+ private NodeStore nodeStore;
+ private LogCustomizer customLogger;
+
+
+ // Setting contentCount to DEFAULT_indexJcrTypeInvalidLogLimiter + 1, so that invalid indexes are logged atleast once
+ private final long contentCount = IndexUpdate.INDEX_JCR_TYPE_INVALID_LOG_LIMITER + 1;
+
+ @Before
+ public void before() throws Exception {
+ ContentSession session = createRepository().login(null, null);
+ root = session.getLatestRoot();
+ customLogger = LogCustomizer
+ .forLogger(IndexUpdate.class.getName())
+ .enable(Level.WARN).create();
+ customLogger.starting();
+ }
+
+ @After
+ public void after() {
+ customLogger.finished();
+ }
+
+ private ContentRepository createRepository() {
+ nodeStore = new MemoryNodeStore();
+ blobStore = new MemoryBlobStore();
+ blobStore.setBlockSizeMin(48);//make it as small as possible
+
+ LuceneIndexEditorProvider luceneIndexEditorProvider = new LuceneIndexEditorProvider();
+ LuceneIndexProvider provider = new LuceneIndexProvider();
+ luceneIndexEditorProvider.setBlobStore(blobStore);
+
+ asyncIndexUpdate = new AsyncIndexUpdate("async", nodeStore, compose(newArrayList(
+ luceneIndexEditorProvider,
+ new NodeCounterEditorProvider()
+ )));
+ TrackingCorruptIndexHandler trackingCorruptIndexHandler = new TrackingCorruptIndexHandler();
+ trackingCorruptIndexHandler.setCorruptInterval(INDEX_CORRUPT_INTERVAL_IN_MILLIS, TimeUnit.MILLISECONDS);
+ asyncIndexUpdate.setCorruptIndexHandler(trackingCorruptIndexHandler);
+ return new Oak(nodeStore)
+ .with(new InitialContent())
+ .with(new OpenSecurityProvider())
+ .with((QueryIndexProvider) provider)
+ .with((Observer) provider)
+ .with(luceneIndexEditorProvider)
+ .with(new PropertyIndexEditorProvider())
+ .with(new NodeTypeIndexProvider())
+ .with(new PropertyIndexProvider())
+ .with(new PropertyIndexEditorProvider())
+ .createContentRepository();
+ }
+
+ private void runEmptyAsyncIndexerCyclesWithoutNewContent(long count) {
+ for (int i = 0; i < count; i++) {
+ asyncIndexUpdate.run();
+ }
+ }
+
+ private void runIndexerCyclesAfterEachNodeCommit(long count, boolean async) throws CommitFailedException {
+ for (int i = 0; i < count; i++) {
+ root.getTree("/").addChild("testNode" + i);
+ root.commit();
+ if (async) {
+ asyncIndexUpdate.run();
+ }
+ }
+ }
+
+ private boolean assertionLogPresent(List<String> logs, String assertionLog) {
+ for (String log : logs) {
+ if (log.equals(assertionLog)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private String invalidJcrPrimaryTypeLog() {
+ return "jcr:primaryType of index " + invalidJcrPrimaryTypeIndexName + " should be oak:QueryIndexDefinition instead of " + invalidJcrPrimaryTypeForIndex;
+ }
+
+ /**
+ * Logs warning as index is not a valid index definition
+ */
+ @Test
+ public void loggingInvalidJcrPrimaryType() throws Exception {
+ Tree test1 = root.getTree("/").addChild(INDEX_DEFINITIONS_NAME).addChild(invalidJcrPrimaryTypeIndexName);
+ test1.setProperty("jcr:primaryType", invalidJcrPrimaryTypeForIndex, Type.NAME);
+ test1.setProperty("type", "lucene");
+ test1.setProperty("reindex", true);
+ test1.setProperty("async", "async");
+ root.commit();
+ runIndexerCyclesAfterEachNodeCommit(contentCount, true);
+ runEmptyAsyncIndexerCyclesWithoutNewContent(contentCount);
+
+ List<String> logs = customLogger.getLogs();
+ assertTrue(assertionLogPresent(logs, invalidJcrPrimaryTypeLog()));
+ }
+
+ @Test
+ public void noLoggingIfNodeIsNotIndexDefinition() throws Exception {
+ // This is not a valid index definition. Refer method: IndexUpdate.isIncluded
+ Tree test1 = root.getTree("/").addChild(INDEX_DEFINITIONS_NAME).addChild(invalidJcrPrimaryTypeIndexName);
+ test1.setProperty("jcr:primaryType", invalidJcrPrimaryTypeForIndex, Type.NAME);
+ // Here index definition itself is not valid as there is no type property
+ // test1.setProperty("type", "lucene");
+ test1.setProperty("reindex", true);
+ test1.setProperty("async", "async");
+ root.commit();
+
+ runIndexerCyclesAfterEachNodeCommit(contentCount, true);
+ runEmptyAsyncIndexerCyclesWithoutNewContent(contentCount);
+
+ List<String> logs = customLogger.getLogs();
+ assertFalse(assertionLogPresent(logs, invalidJcrPrimaryTypeLog()));
+ }
+
+ @Test
+ public void loggingInCaseOfSyncLuceneIndexDefinition() throws Exception {
+ Tree test1 = root.getTree("/").addChild(INDEX_DEFINITIONS_NAME).addChild(invalidJcrPrimaryTypeIndexName);
+ test1.setProperty("jcr:primaryType", invalidJcrPrimaryTypeForIndex, Type.NAME);
+ test1.setProperty("type", "lucene");
+ test1.setProperty("reindex", true);
+ // test1.setProperty("async", "async");
+ root.commit();
+
+ runIndexerCyclesAfterEachNodeCommit(contentCount, false);
+ List<String> logs = customLogger.getLogs();
+ assertTrue(assertionLogPresent(logs, invalidJcrPrimaryTypeLog()));
+ }
+}
diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/SuggestionIntervalTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/SuggestionIntervalTest.java
index 0f622ec..f33eacd 100644
--- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/SuggestionIntervalTest.java
+++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/SuggestionIntervalTest.java
@@ -87,7 +87,7 @@ public class SuggestionIntervalTest extends AbstractQueryTest {
String indexName = "lucene-suggest";
Tree def = root.getTree("/" + INDEX_DEFINITIONS_NAME)
.addChild(indexName);
- def.setProperty(JcrConstants.JCR_PRIMARYTYPE, INDEX_DEFINITIONS_NODE_TYPE);
+ def.setProperty(JcrConstants.JCR_PRIMARYTYPE, INDEX_DEFINITIONS_NODE_TYPE, Type.NAME);
def.setProperty(TYPE_PROPERTY_NAME, LuceneIndexConstants.TYPE_LUCENE);
def.setProperty(REINDEX_PROPERTY_NAME, true);
def.setProperty("name", indexName);
diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFacetTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFacetTest.java
index ec471e9..e6a01df 100644
--- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFacetTest.java
+++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticFacetTest.java
@@ -62,6 +62,7 @@ import java.util.stream.Collectors;
import static org.apache.jackrabbit.commons.JcrUtils.getOrCreateByPath;
import static org.apache.jackrabbit.oak.InitialContentHelper.INITIAL_CONTENT;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.FACETS;
import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROP_REFRESH_DEFN;
import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROP_SECURE_FACETS;
@@ -159,7 +160,7 @@ public class ElasticFacetTest {
String build() throws RepositoryException {
final String indexName = UUID.randomUUID().toString();
- indexDefinitionBuilder.build(adminSession.getRootNode().getNode(INDEX_DEFINITIONS_NAME).addNode(indexName));
+ indexDefinitionBuilder.build(adminSession.getRootNode().getNode(INDEX_DEFINITIONS_NAME).addNode(indexName, INDEX_DEFINITIONS_NODE_TYPE));
return indexName;
}
}
diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticSpellcheckTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticSpellcheckTest.java
index c3409b8..5688cdb 100644
--- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticSpellcheckTest.java
+++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticSpellcheckTest.java
@@ -57,6 +57,7 @@ import java.util.UUID;
import static org.apache.jackrabbit.commons.JcrUtils.getOrCreateByPath;
import static org.apache.jackrabbit.oak.InitialContentHelper.INITIAL_CONTENT;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
import static org.apache.jackrabbit.oak.plugins.index.elastic.ElasticIndexDefinition.BULK_FLUSH_INTERVAL_MS_DEFAULT;
import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROP_ANALYZED;
import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROP_USE_IN_SPELLCHECK;
@@ -138,7 +139,7 @@ public class ElasticSpellcheckTest {
String build() throws RepositoryException {
final String indexName = UUID.randomUUID().toString();
- indexDefinitionBuilder.build(adminSession.getRootNode().getNode(INDEX_DEFINITIONS_NAME).addNode(indexName));
+ indexDefinitionBuilder.build(adminSession.getRootNode().getNode(INDEX_DEFINITIONS_NAME).addNode(indexName, INDEX_DEFINITIONS_NODE_TYPE));
return indexName;
}
}
diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexOptions.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexOptions.java
index df2fc45..fdf6ffc 100644
--- a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexOptions.java
+++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexOptions.java
@@ -25,6 +25,7 @@ import javax.jcr.RepositoryException;
import javax.jcr.Session;
import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
public abstract class IndexOptions {
@@ -35,7 +36,7 @@ public abstract class IndexOptions {
}
protected Node setIndex(Session session, String idxName, IndexDefinitionBuilder builder) throws RepositoryException {
- return builder.build(session.getRootNode().getNode(INDEX_DEFINITIONS_NAME).addNode(idxName));
+ return builder.build(session.getRootNode().getNode(INDEX_DEFINITIONS_NAME).addNode(idxName, INDEX_DEFINITIONS_NODE_TYPE));
}
protected Node getIndexNode(Session session, String idxName) throws RepositoryException {
diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/SecureFacetCommonTest.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/SecureFacetCommonTest.java
index 14f045c..d33399c 100644
--- a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/SecureFacetCommonTest.java
+++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/SecureFacetCommonTest.java
@@ -75,6 +75,7 @@ public abstract class SecureFacetCommonTest extends AbstractJcrTest {
String indexName = UUID.randomUUID().toString();
IndexDefinitionBuilder builder = indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false);
builder.noAsync();
+ builder.getBuilderTree().setProperty("jcr:primaryType", "oak:QueryIndexDefinition", Type.NAME);
IndexDefinitionBuilder.IndexRule indexRule = builder.indexRule(JcrConstants.NT_BASE);
indexRule.property("cons").propertyIndex();
indexRule.property("foo").propertyIndex().getBuilderTree().setProperty(FACET_PROP, true, Type.BOOLEAN);