Author: tomekr Date: Thu Aug 9 08:38:03 2018 New Revision: 1837700 URL: http://svn.apache.org/viewvc?rev=1837700&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/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateCallback.java jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java Modified: jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java?rev=1837700&r1=1837699&r2=1837700&view=diff ============================================================================== --- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java (original) +++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdate.java Thu Aug 9 08:38:03 2018 @@ -240,7 +240,16 @@ public class IndexUpdate implements Edit Editor editor = rootState.provider.getIndexEditor(type, definition, rootState.root, rootState.newCallback(indexPath, shouldReindex)); 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/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateCallback.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateCallback.java?rev=1837700&r1=1837699&r2=1837700&view=diff ============================================================================== --- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateCallback.java (original) +++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateCallback.java Thu Aug 9 08:38:03 2018 @@ -20,6 +20,12 @@ import org.apache.jackrabbit.oak.api.Com public interface IndexUpdateCallback { + IndexUpdateCallback NOOP = new IndexUpdateCallback() { + @Override + public void indexUpdate() { + } + }; + /** * Invoked by the {@link org.apache.jackrabbit.oak.plugins.index.IndexEditor} for every NodeState * indexed it. Modified: jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java?rev=1837700&r1=1837699&r2=1837700&view=diff ============================================================================== --- jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java (original) +++ jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexUpdateTest.java Thu Aug 9 08:38:03 2018 @@ -26,6 +26,7 @@ import static org.apache.jackrabbit.oak. import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_ASYNC_PROPERTY_NAME; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_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.plugins.nodetype.write.InitialContent.INITIAL_CONTENT; import static org.hamcrest.CoreMatchers.instanceOf; @@ -42,8 +43,10 @@ import java.util.Calendar; import java.util.Map; import java.util.Set; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +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; @@ -443,6 +446,77 @@ 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(new EditorProvider() { + @CheckForNull + @Override + public Editor getRootEditor(NodeState before, NodeState after, NodeBuilder builder, CommitInfo info) { + return new IndexUpdate(emptyProvider(), null, after, builder, NOOP) + .withMissingProviderStrategy(mips); + } + }); + EditorHook asyncHook = new EditorHook(new EditorProvider() { + @CheckForNull + @Override + public Editor getRootEditor(NodeState before, NodeState after, NodeBuilder builder, CommitInfo info) { + return new IndexUpdate(emptyProvider(), "async-run", after, builder, NOOP) + .withMissingProviderStrategy(mips); + } + }); + EditorHook otherAsyncHook = new EditorHook(new EditorProvider() { + @CheckForNull + @Override + public Editor getRootEditor(NodeState before, NodeState after, NodeBuilder builder, CommitInfo info) { + return 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 } }