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 ca...@apache.org on 2018/08/08 22:16:16 UTC

svn commit: r1837676 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java

Author: catholicon
Date: Wed Aug  8 22:16:16 2018
New Revision: 1837676

URL: http://svn.apache.org/viewvc?rev=1837676&view=rev
Log:
OAK-7686: Partial migration doesn't update Lucene indexing data

For now, we have essentially avoided setting reindex flag for
async+nrt/sync indexes during a sync index run. Otoh, OAK-7696 would be
the right fix for the original problem (admittedly more instrusive and
discussion worthy)

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java?rev=1837676&r1=1837675&r2=1837676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java Wed Aug  8 22:16:16 2018
@@ -274,7 +274,16 @@ public class IndexUpdate implements Edit
                 Editor editor = rootState.provider.getIndexEditor(type, definition, rootState.root,
                         rootState.newCallback(indexPath, shouldReindex, getEstimatedCount(definition)));
                 if (editor == null) {
-                    rootState.missingProvider.onMissingIndex(type, definition, indexPath);
+                    // if this isn't an async cycle AND definition has "async" property
+                    // (and implicitly isIncluded method allows async def in non-async cycle only for nrt/sync defs)
+                    // then we don't need to handle missing handler
+                    if (definition.hasProperty(ASYNC_PROPERTY_NAME) && rootState.async == null) {
+                        log.warn("Missing provider for nrt/sync index: {} (rootState.async: {}). " +
+                                "Please note, it means that index data should be trusted only after this index " +
+                                "is processed in an async indexing cycle.", definition, rootState.async);
+                    } else {
+                        rootState.missingProvider.onMissingIndex(type, definition, indexPath);
+                    }
                 } else if (shouldReindex) {
                     if (definition.getBoolean(REINDEX_ASYNC_PROPERTY_NAME)
                             && definition.getString(ASYNC_PROPERTY_NAME) == null) {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java?rev=1837676&r1=1837675&r2=1837676&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java Wed Aug  8 22:16:16 2018
@@ -29,6 +29,7 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_DISABLED;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback.NOOP;
 import static org.apache.jackrabbit.oak.plugins.index.IndexUtils.createIndexDefinition;
 import static org.apache.jackrabbit.oak.InitialContentHelper.INITIAL_CONTENT;
 import static org.hamcrest.CoreMatchers.instanceOf;
@@ -47,6 +48,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import ch.qos.logback.classic.Level;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.PropertyState;
@@ -522,6 +524,62 @@ public class IndexUpdateTest {
             fail("commit should fail on missing index provider");
         } catch (CommitFailedException ex) {
             // expected
+        }
+    }
+
+    /**
+     * OAK-7686: async def with nrt/sync def should fail on missing provider only when running in
+     * context of an async cycle
+     */
+    @Test
+    public void testMissingProviderWithAsyncDef() throws Exception {
+        final MissingIndexProviderStrategy mips = new MissingIndexProviderStrategy();
+        mips.setFailOnMissingIndexProvider(true);
+
+        // prepare different hooks for different types indexing cycles
+        EditorHook syncHook = new EditorHook((before, after, builder, info) ->
+                new IndexUpdate(emptyProvider(), null, after, builder, NOOP)
+                        .withMissingProviderStrategy(mips));
+        EditorHook asyncHook = new EditorHook((before, after, builder, info) ->
+                new IndexUpdate(emptyProvider(), "async-run", after, builder, NOOP)
+                        .withMissingProviderStrategy(mips));
+        EditorHook otherAsyncHook = new EditorHook((before, after, builder, info) ->
+                new IndexUpdate(emptyProvider(), "other-async-run", after, builder, NOOP)
+                        .withMissingProviderStrategy(mips));
+
+        builder = EmptyNodeState.EMPTY_NODE.builder();
+
+        // create async defs with nrt and sync mixed in
+        createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME),
+                "asyncIndex", true, false, ImmutableSet.of("foo"), null)
+                .setProperty(ASYNC_PROPERTY_NAME, ImmutableList.of("async-run"), Type.STRINGS)
+                .setProperty(REINDEX_PROPERTY_NAME, false);
+        createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME),
+                "nrtIndex", true, false, ImmutableSet.of("foo"), null)
+                .setProperty(ASYNC_PROPERTY_NAME, ImmutableList.of("async-run", "nrt"), Type.STRINGS)
+                .setProperty(REINDEX_PROPERTY_NAME, false);
+        createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME),
+                "asyncSyncIndex", true, false, ImmutableSet.of("foo"), null)
+                .setProperty(ASYNC_PROPERTY_NAME, ImmutableList.of("async-run", "sync"), Type.STRINGS)
+                .setProperty(REINDEX_PROPERTY_NAME, false);
+
+        // node states to run hook on
+        NodeState before = builder.getNodeState();
+        builder.child("testRoot").setProperty("foo", "abc");
+        NodeState after = builder.getNodeState();
+
+        // sync run should be ok with missing provider for an async def
+        syncHook.processCommit(before, after, CommitInfo.EMPTY);
+
+        // unrelated async run should be ok with missing provider
+        otherAsyncHook.processCommit(before, after, CommitInfo.EMPTY);
+
+        // async run matching the def async lane still should fail
+        try {
+            asyncHook.processCommit(before, after, CommitInfo.EMPTY);
+            fail("commit should fail on missing index provider");
+        } catch (CommitFailedException ex) {
+            // expected
         }
     }