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 ch...@apache.org on 2016/12/05 13:16:08 UTC

svn commit: r1772665 - in /jackrabbit/oak/trunk/oak-lucene/src: main/java/org/apache/jackrabbit/oak/plugins/index/lucene/ main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/ test/java/org/apache/jackrabbit/oak/plugins/index/lucene/

Author: chetanm
Date: Mon Dec  5 13:16:07 2016
New Revision: 1772665

URL: http://svn.apache.org/viewvc?rev=1772665&view=rev
Log:
OAK-5212 - Avoid updating the index nodestate if no change is done in index

Modified:
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java?rev=1772665&r1=1772664&r2=1772665&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectory.java Mon Dec  5 13:16:07 2016
@@ -94,6 +94,7 @@ public class OakDirectory extends Direct
     private final String indexName;
     @Nullable
     private final GarbageCollectableBlobStore blobStore;
+    private volatile boolean dirty;
 
     public OakDirectory(NodeBuilder builder, IndexDefinition definition, boolean readOnly) {
         this(builder, INDEX_DATA_CHILD_NAME, definition, readOnly, null);
@@ -154,6 +155,7 @@ public class OakDirectory extends Direct
             trashEntry.setProperty(JCR_DATA, data, BINARIES);
         }
         f.remove();
+        markDirty();
     }
 
     @Override
@@ -183,6 +185,7 @@ public class OakDirectory extends Direct
             file = directoryBuilder.child(name);
         }
         fileNames.add(name);
+        markDirty();
         return new OakIndexOutput(name, file, indexName, blobStore);
     }
 
@@ -238,6 +241,14 @@ public class OakDirectory extends Direct
         return "Directory for " + definition.getIndexName();
     }
 
+    public boolean isDirty() {
+        return dirty;
+    }
+
+    private void markDirty() {
+        dirty = true;
+    }
+
     private Set<String> getListing(){
         long start = PERF_LOGGER.start();
         Iterable<String> fileNames = null;

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java?rev=1772665&r1=1772664&r2=1772665&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/writer/DefaultIndexWriter.java Mon Dec  5 13:16:07 2016
@@ -21,6 +21,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.io.IOException;
 import java.util.Calendar;
+import java.util.List;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -37,6 +38,7 @@ import org.apache.jackrabbit.oak.util.Pe
 import org.apache.jackrabbit.util.ISO8601;
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexCommit;
 import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.index.IndexableField;
@@ -64,6 +66,8 @@ class DefaultIndexWriter implements Luce
     private IndexWriter writer;
     private Directory directory;
     private GarbageCollectableBlobStore blobStore;
+    private long genAtStart = -1;
+    private boolean indexUpdated = false;
 
     public DefaultIndexWriter(IndexDefinition definition, NodeBuilder definitionBuilder,
         @Nullable IndexCopier indexCopier, String dirName, String suggestDirName,
@@ -86,6 +90,7 @@ class DefaultIndexWriter implements Luce
     @Override
     public void updateDocument(String path, Iterable<? extends IndexableField> doc) throws IOException {
         getWriter().updateDocument(newPathTerm(path), doc);
+        indexUpdated = true;
     }
 
     @Override
@@ -96,6 +101,7 @@ class DefaultIndexWriter implements Luce
 
     void deleteAll() throws IOException {
         getWriter().deleteAll();
+        indexUpdated = true;
     }
 
     @Override
@@ -104,7 +110,6 @@ class DefaultIndexWriter implements Luce
         //it indicates that the index is empty. In such a case trigger
         //creation of write such that an empty Lucene index state is persisted
         //in directory
-        boolean indexUpdated = false;
         if (reindex && writer == null){
             getWriter();
         }
@@ -118,7 +123,6 @@ class DefaultIndexWriter implements Luce
         }
 
         if (writer != null) {
-            indexUpdated = true;
             if (log.isTraceEnabled()) {
                 trackIndexSizeInfo(writer, definition, directory);
             }
@@ -126,13 +130,18 @@ class DefaultIndexWriter implements Luce
             final long start = PERF_LOGGER.start();
 
             if (updateSuggestions) {
-                updateSuggester(writer.getAnalyzer(), currentTime, blobStore);
+                indexUpdated |= updateSuggester(writer.getAnalyzer(), currentTime, blobStore);
                 PERF_LOGGER.end(start, -1, "Completed suggester for directory {}", definition);
             }
 
             writer.close();
             PERF_LOGGER.end(start, -1, "Closed writer for directory {}", definition);
 
+            if (!indexUpdated){
+                long genAtEnd = getLatestGeneration(directory);
+                indexUpdated = genAtEnd != genAtStart;
+            }
+
             directory.close();
             PERF_LOGGER.end(start, -1, "Closed directory for directory {}", definition);
         }
@@ -153,6 +162,7 @@ class DefaultIndexWriter implements Luce
                 config = getIndexWriterConfig(definition, true);
             }
             writer = new IndexWriter(directory, config);
+            genAtStart = getLatestGeneration(directory);
             PERF_LOGGER.end(start, -1, "Created IndexWriter for directory {}", definition);
         }
         return writer;
@@ -164,21 +174,24 @@ class DefaultIndexWriter implements Luce
      * @param analyzer the analyzer used to update the suggester
      * @param blobStore
      */
-    private void updateSuggester(Analyzer analyzer, Calendar currentTime,
+    private boolean updateSuggester(Analyzer analyzer, Calendar currentTime,
         @Nullable GarbageCollectableBlobStore blobStore) throws IOException {
         NodeBuilder suggesterStatus = definitionBuilder.child(suggestDirName);
         DirectoryReader reader = DirectoryReader.open(writer, false);
         final OakDirectory suggestDirectory =
             new OakDirectory(definitionBuilder, suggestDirName, definition, false, blobStore);
+        boolean updated = false;
         try {
             SuggestHelper.updateSuggester(suggestDirectory, analyzer, reader);
             suggesterStatus.setProperty("lastUpdated", ISO8601.format(currentTime), Type.DATE);
         } catch (Throwable e) {
             log.warn("could not update suggester", e);
         } finally {
+            updated = suggestDirectory.isDirty();
             suggestDirectory.close();
             reader.close();
         }
+        return updated;
     }
 
     /**
@@ -226,6 +239,17 @@ class DefaultIndexWriter implements Luce
         }
     }
 
+    private static long getLatestGeneration(Directory directory) throws IOException {
+        if (DirectoryReader.indexExists(directory)) {
+            List<IndexCommit> commits = DirectoryReader.listCommits(directory);
+            if (!commits.isEmpty()) {
+                //Look for that last commit as list is sorted from oldest to latest
+                return commits.get(commits.size() - 1).getGeneration();
+            }
+        }
+        return -1;
+    }
+
     private static void trackIndexSizeInfo(@Nonnull IndexWriter writer,
                                            @Nonnull IndexDefinition definition,
                                            @Nonnull Directory directory) throws IOException {

Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java?rev=1772665&r1=1772664&r2=1772665&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java Mon Dec  5 13:16:07 2016
@@ -88,7 +88,6 @@ import org.apache.lucene.search.TopDocs;
 import org.apache.lucene.store.Directory;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -152,7 +151,6 @@ public class LuceneIndexEditorTest {
                 getPath(NumericRangeQuery.newLongRange("price", 100L, 100L, true, true)));
     }
 
-    @Ignore("OAK-5212")
     @Test
     public void noChangeIfNonIndexedDelete() throws Exception{
         NodeState before = builder.getNodeState();
@@ -446,6 +444,7 @@ public class LuceneIndexEditorTest {
         executorService.shutdown();
     }
 
+
     @Test
     public void multiplexingWriter() throws Exception{
         newLucenePropertyIndex("lucene", "foo");

Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java?rev=1772665&r1=1772664&r2=1772665&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/OakDirectoryTest.java Mon Dec  5 13:16:07 2016
@@ -30,6 +30,7 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent.INITIAL_CONTENT;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -201,7 +202,7 @@ public class OakDirectoryTest {
         return size;
     }
 
-    private Directory createDir(NodeBuilder builder, boolean readOnly, String indexPath){
+    private OakDirectory createDir(NodeBuilder builder, boolean readOnly, String indexPath){
         return new OakDirectory(builder,
                 new IndexDefinition(root, builder.getNodeState(), indexPath), readOnly);
     }
@@ -438,6 +439,23 @@ public class OakDirectoryTest {
         assertEquals(0, dir.listAll().length);
     }
 
+    @Test
+    public void testDirty() throws Exception{
+        OakDirectory dir = createDir(builder, false, "/foo");
+        assertFalse(dir.isDirty());
+        createFile(dir, "a");
+        assertTrue(dir.isDirty());
+        dir.close();
+
+        dir = createDir(builder, false, "/foo");
+        assertFalse(dir.isDirty());
+        dir.openInput("a", IOContext.DEFAULT);
+        assertFalse(dir.isDirty());
+        dir.deleteFile("a");
+        assertTrue(dir.isDirty());
+        dir.close();
+    }
+
     private static void readInputToEnd(long expectedSize, IndexInput input) throws IOException {
         int COPY_BUFFER_SIZE = 16384;
         byte[] copyBuffer = new byte[(int) ONE_MB];