Author: alexparvulescu Date: Fri Jan 13 15:43:40 2017 New Revision: 1778620
URL: http://svn.apache.org/viewvc?rev=1778620&view=rev Log: OAK-5416 Async reindex of a sync property does't release created checkpoint Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/AsyncPropertyIndexTest.java jackrabbit/oak/trunk/oak-doc/src/site/markdown/query/property-index.md Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java?rev=1778620&r1=1778619&r2=1778620&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java Fri Jan 13 15:43:40 2017 @@ -490,7 +490,7 @@ public class AsyncIndexUpdate implements beforeCheckpoint = null; callback.setCheckpoint(beforeCheckpoint); before = MISSING_NODE; - } else if (noVisibleChanges(state, root)) { + } else if (noVisibleChanges(state, root) && !switchOnSync) { log.debug( "[{}] No changes since last checkpoint; skipping the index update", name); @@ -611,11 +611,11 @@ public class AsyncIndexUpdate implements } void cleanUpCheckpoints() { - log.debug("Cleaning up orphaned checkpoints"); + log.debug("[{}] Cleaning up orphaned checkpoints", name); Set<String> keep = newHashSet(); String cp = indexStats.getReferenceCheckpoint(); if (cp == null) { - log.warn("No reference checkpoint set in index stats"); + log.warn("[{}] No reference checkpoint set in index stats", name); return; } keep.add(cp); @@ -639,8 +639,8 @@ public class AsyncIndexUpdate implements && AsyncIndexUpdate.class.getSimpleName().equals(creator) && (created == null || ISO8601.parse(created).getTimeInMillis() + leaseTimeOut < current)) { if (store.release(checkpoint)) { - log.info("Removed orphaned checkpoint '{}' {}", - checkpoint, info); + log.info("[{}] Removed orphaned checkpoint '{}' {}", + name, checkpoint, info); } } } @@ -713,6 +713,12 @@ public class AsyncIndexUpdate implements } } reindexedDefinitions.clear(); + if (store.release(afterCheckpoint)) { + builder.child(ASYNC).removeProperty(name); + builder.child(ASYNC).removeProperty(lastIndexedTo); + } else { + log.debug("[{}] Unable to release checkpoint {}", name, afterCheckpoint); + } } updatePostRunStatus = true; } Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/AsyncPropertyIndexTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/AsyncPropertyIndexTest.java?rev=1778620&r1=1778619&r2=1778620&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/AsyncPropertyIndexTest.java (original) +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/AsyncPropertyIndexTest.java Fri Jan 13 15:43:40 2017 @@ -17,7 +17,6 @@ package org.apache.jackrabbit.oak.plugins.index.property; import static com.google.common.collect.ImmutableSet.of; -import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM; import static org.apache.jackrabbit.JcrConstants.NT_BASE; import static org.apache.jackrabbit.oak.api.Type.STRINGS; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.ASYNC_PROPERTY_NAME; @@ -25,10 +24,10 @@ 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.IndexUtils.createIndexDefinition; -import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_NODE_TYPES; import static org.apache.jackrabbit.oak.spi.commit.CommitInfo.EMPTY; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.Set; @@ -52,6 +51,7 @@ import org.apache.jackrabbit.oak.spi.sta import org.junit.Test; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Sets; /** @@ -66,6 +66,7 @@ public class AsyncPropertyIndexTest { @Test public void testAsyncPropertyLookup() throws Exception { NodeStore store = new MemoryNodeStore(); + assertTrue(Iterables.isEmpty(store.checkpoints())); NodeBuilder builder = store.getRoot().builder(); @@ -101,8 +102,10 @@ public class AsyncPropertyIndexTest { // run async second time, there are no changes, should switch to sync async.run(); + async.close(); assertEquals(null, store.getRoot().getChildNode(INDEX_DEFINITIONS_NAME) .getChildNode("foo").getString(ASYNC_PROPERTY_NAME)); + assertTrue(Iterables.isEmpty(store.checkpoints())); // add content, it should be indexed synchronously builder = store.getRoot().builder(); @@ -127,4 +130,15 @@ public class AsyncPropertyIndexTest { : PropertyValues.newString(value))); } + @Test + public void testAsyncPropertyNoChanges() throws Exception { + NodeStore store = new MemoryNodeStore(); + assertTrue(Iterables.isEmpty(store.checkpoints())); + AsyncIndexUpdate async = new AsyncIndexUpdate(ASYNC_REINDEX_VALUE, + store, provider, true); + async.run(); + async.run(); + async.close(); + assertTrue(Iterables.isEmpty(store.checkpoints())); + } } Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/query/property-index.md URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/query/property-index.md?rev=1778620&r1=1778619&r2=1778620&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-doc/src/site/markdown/query/property-index.md (original) +++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/query/property-index.md Fri Jan 13 15:43:40 2017 @@ -115,6 +115,10 @@ Example: .setProperty("reindex", true); } +When recovering a failed async reindex special care needs to be taken wrt. the created checkpoint and the __`async`__ property. +The checkpoint should be released via the __`CheckpointManager`__ mbean, and the __`async`__ property needs to be manually deleted +while also setting the __`reindex`__ flags to __`true`__ to make sure the index returns to a consistent state, in sync with the head revision. + #### Cost Estimation When running a query, the property index reports its estimated cost to the query engine,
