Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java (revision 1602815) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java (working copy) @@ -62,7 +62,7 @@ * Name of the hidden node under which information about the checkpoints * seen and indexed by each async indexer is kept. */ - private static final String ASYNC = ":async"; + static final String ASYNC = ":async"; private static final long DEFAULT_LIFETIME = TimeUnit.DAYS.toMillis(1000); @@ -223,7 +223,13 @@ String checkpointToRelease = afterCheckpoint; try { - updateIndex(before, beforeCheckpoint, after, afterCheckpoint); + if (updateIndex(before, beforeCheckpoint, after, afterCheckpoint)) { + // the update succeeded with changes to the index, so we can + // release the earlier checkpoint, otherwise the new checkpoint + // associated with the failed update will get released in the + // finally block + checkpointToRelease = beforeCheckpoint; + } // the update succeeded, i.e. it no longer fails if (failing) { @@ -231,11 +237,6 @@ failing = false; } - // the update succeeded, so we can release the earlier checkpoint - // otherwise the new checkpoint associated with the failed update - // will get released in the finally block - checkpointToRelease = beforeCheckpoint; - } catch (CommitFailedException e) { if (e == CONCURRENT_UPDATE) { log.debug("Concurrent update detected in the {} index update", name); @@ -253,7 +254,7 @@ } } - private void updateIndex( + private boolean updateIndex( NodeState before, String beforeCheckpoint, NodeState after, String afterCheckpoint) throws CommitFailedException { @@ -264,6 +265,7 @@ // and maintaining the update lease AsyncUpdateCallback callback = new AsyncUpdateCallback(beforeCheckpoint); + boolean isDirty = false; try { NodeBuilder builder = store.getRoot().builder(); @@ -279,6 +281,7 @@ builder.child(ASYNC).setProperty(name, afterCheckpoint); mergeWithConcurrencyCheck( builder, beforeCheckpoint, callback.lease); + isDirty = true; if (switchOnSync) { reindexedDefinitions.addAll( @@ -311,6 +314,7 @@ } postAsyncRunStatsStatus(indexStats); + return isDirty; } private void mergeWithConcurrencyCheck( Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java (revision 1602815) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdateTest.java (working copy) @@ -381,6 +381,8 @@ Set checkpoints = newHashSet(store.listCheckpoints()); assertTrue("Expecting the initial checkpoint", checkpoints.size() == 1); + assertEquals(store.getRoot().getChildNode(AsyncIndexUpdate.ASYNC) + .getString("async"), checkpoints.iterator().next()); async.run(); assertEquals("Expecting no checkpoint changes", @@ -418,6 +420,48 @@ String secondCp = store.listCheckpoints().iterator().next(); assertFalse("Store should keep only second checkpoint", secondCp.equals(firstCp)); + assertEquals( + secondCp, + store.getRoot().getChildNode(AsyncIndexUpdate.ASYNC) + .getString("async")); + } + + @Test + public void cpCleanupWUnrelatedChanges() throws Exception { + MemoryNodeStore store = new MemoryNodeStore(); + IndexEditorProvider provider = new PropertyIndexEditorProvider(); + + NodeBuilder builder = store.getRoot().builder(); + createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), + "rootIndex", true, false, ImmutableSet.of("foo"), null) + .setProperty(ASYNC_PROPERTY_NAME, "async"); + builder.child("testRoot").setProperty("foo", "abc"); + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + + assertTrue("Expecting no checkpoints", + store.listCheckpoints().size() == 0); + + AsyncIndexUpdate async = new AsyncIndexUpdate("async", store, provider); + async.run(); + assertTrue("Expecting one checkpoint", + store.listCheckpoints().size() == 1); + String firstCp = store.listCheckpoints().iterator().next(); + + // add content that's hidden from indexing + builder = store.getRoot().builder(); + builder.child("testRoot").child(":hidden"); + store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); + async.run(); + + assertTrue("Expecting one checkpoint", + store.listCheckpoints().size() == 1); + String secondCp = store.listCheckpoints().iterator().next(); + assertTrue("Store should not create extra checkpoints", + secondCp.equals(firstCp)); + assertEquals( + secondCp, + store.getRoot().getChildNode(AsyncIndexUpdate.ASYNC) + .getString("async")); } @Test