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 md...@apache.org on 2014/12/10 14:10:31 UTC

svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Author: mduerig
Date: Wed Dec 10 13:10:30 2014
New Revision: 1644389

URL: http://svn.apache.org/r1644389
Log:
OAK-2333: SegmentMK startup slow with large journals
Scan the journal.log backwards

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReader.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ConsistencyChecker.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReaderTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java?rev=1644389&r1=1644388&r2=1644389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java Wed Dec 10 13:10:30 2014
@@ -186,8 +186,8 @@ public class FileStore implements Segmen
         this.maxFileSize = maxFileSizeMB * MB;
         this.memoryMapping = memoryMapping;
 
-        journalFile = new RandomAccessFile(
-                new File(directory, JOURNAL_FILE_NAME), "rw");
+        journalFile = new RandomAccessFile(new File(directory, JOURNAL_FILE_NAME), "rw");
+        journalFile.seek(journalFile.length());
         journalLock = journalFile.getChannel().lock();
 
         Map<Integer, Map<Character, File>> map = collectFiles(directory);
@@ -208,18 +208,23 @@ public class FileStore implements Segmen
                 String.format(FILE_NAME_FORMAT, writeNumber, "a"));
         this.writer = new TarWriter(writeFile);
 
-        LinkedList<String> heads = JournalReader.heads(journalFile);
         RecordId id = null;
-        while (id == null && !heads.isEmpty()) {
-            RecordId last = RecordId.fromString(tracker, heads.removeLast());
-            SegmentId segmentId = last.getSegmentId();
-            if (containsSegment(
-                    segmentId.getMostSignificantBits(),
-                    segmentId.getLeastSignificantBits())) {
-                id = last;
-            } else {
-                log.warn("Unable to access revision {}, rewinding...", last);
+        JournalReader journalReader = new JournalReader(new File(directory, JOURNAL_FILE_NAME));
+        try {
+            Iterator<String> heads = journalReader.iterator();
+            while (id == null && heads.hasNext()) {
+                RecordId last = RecordId.fromString(tracker, heads.next());
+                SegmentId segmentId = last.getSegmentId();
+                if (containsSegment(
+                        segmentId.getMostSignificantBits(),
+                        segmentId.getLeastSignificantBits())) {
+                    id = last;
+                } else {
+                    log.warn("Unable to access revision {}, rewinding...", last);
+                }
             }
+        } finally {
+            journalReader.close();
         }
 
         if (id != null) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReader.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReader.java?rev=1644389&r1=1644388&r2=1644389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReader.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReader.java Wed Dec 10 13:10:30 2014
@@ -19,36 +19,56 @@
 
 package org.apache.jackrabbit.oak.plugins.segment.file;
 
-import java.io.DataInput;
+import java.io.Closeable;
+import java.io.File;
 import java.io.IOException;
-import java.util.LinkedList;
+import java.util.Iterator;
 
-import com.google.common.collect.Lists;
+import com.google.common.collect.AbstractIterator;
+import org.apache.commons.io.input.ReversedLinesFileReader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Reader for journal files of the SegmentMK.
  */
-public final class JournalReader {
+public final class JournalReader implements Closeable, Iterable<String> {
+    private static final Logger LOG = LoggerFactory.getLogger(JournalReader.class);
 
-    private JournalReader() { }
+    private final ReversedLinesFileReader journal;
+
+    public JournalReader(File journalFile) throws IOException {
+        journal = new ReversedLinesFileReader(journalFile);
+    }
 
     /**
-     * Read a journal file
-     * @param journal  name of the journal file
-     * @return  list of revisions listed in the journal. Oldest revision first.
-     * @throws IOException
+     * @return Iterator over the revisions in the journal in reverse order
+     *         (end of the file to beginning).
      */
-    public static LinkedList<String> heads(DataInput journal) throws IOException {
-        LinkedList<String> heads = Lists.newLinkedList();
-        String line = journal.readLine();
-        while (line != null) {
-            int space = line.indexOf(' ');
-            if (space != -1) {
-                heads.add(line.substring(0, space));
+    @Override
+    public Iterator<String> iterator() {
+        return new AbstractIterator<String>() {
+            @Override
+            protected String computeNext() {
+                try {
+                    String line = journal.readLine();
+                    while (line != null) {
+                        int space = line.indexOf(' ');
+                        if (space != -1) {
+                            return line.substring(0, space);
+                        }
+                        line = journal.readLine();
+                    }
+                } catch (IOException e) {
+                    LOG.error("Error reading journal file", e);
+                }
+                return endOfData();
             }
-            line = journal.readLine();
-        }
-        return heads;
+        };
     }
 
+    @Override
+    public void close() throws IOException {
+        journal.close();
+    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ConsistencyChecker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ConsistencyChecker.java?rev=1644389&r1=1644388&r2=1644389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ConsistencyChecker.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ConsistencyChecker.java Wed Dec 10 13:10:30 2014
@@ -19,7 +19,6 @@
 
 package org.apache.jackrabbit.oak.plugins.segment.file.tooling;
 
-import static com.google.common.collect.Lists.reverse;
 import static com.google.common.collect.Sets.newHashSet;
 import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
 import static org.apache.jackrabbit.oak.commons.PathUtils.denotesRoot;
@@ -29,8 +28,6 @@ import static org.apache.jackrabbit.oak.
 
 import java.io.File;
 import java.io.IOException;
-import java.io.RandomAccessFile;
-import java.util.List;
 import java.util.Set;
 
 import org.apache.jackrabbit.oak.api.PropertyState;
@@ -67,22 +64,13 @@ public class ConsistencyChecker {
      */
     public static String checkConsistency(File directory, String journalFileName,
             boolean fullTraversal, long debugInterval) throws IOException {
-        RandomAccessFile journalFile = new RandomAccessFile(
-            new File(directory, journalFileName), "r");
-
-        List<String> revisions;
-        try {
-            revisions = reverse(JournalReader.heads(journalFile));
-        } finally {
-            journalFile.close();
-        }
-
         print("Searching for last good revision in {}", journalFileName);
+        JournalReader journal = new JournalReader(new File(directory, journalFileName));
         Set<String> badPaths = newHashSet();
         ConsistencyChecker checker = new ConsistencyChecker(directory, debugInterval);
         try {
             int revisionCount = 0;
-            for (String revision : revisions) {
+            for (String revision : journal) {
                 print("Checking revision {}", revision);
                 revisionCount++;
                 String badPath = checker.check(revision, badPaths);
@@ -91,7 +79,7 @@ public class ConsistencyChecker {
                 }
                 if (badPath == null) {
                     print("Found latest good revision {}", revision);
-                    print("Searched through {} of {} revisions", revisionCount, revisions.size());
+                    print("Searched through {} revisions", revisionCount);
                     return revision;
                 } else {
                     badPaths.add(badPath);
@@ -100,6 +88,7 @@ public class ConsistencyChecker {
             }
         } finally {
             checker.close();
+            journal.close();
         }
 
         print("No good revision found");

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReaderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReaderTest.java?rev=1644389&r1=1644388&r2=1644389&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReaderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReaderTest.java Wed Dec 10 13:10:30 2014
@@ -19,13 +19,15 @@
 
 package org.apache.jackrabbit.oak.plugins.segment.file;
 
-import static com.google.common.io.ByteStreams.newDataInput;
+import static java.io.File.createTempFile;
+import static org.apache.commons.io.FileUtils.write;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-import java.io.DataInput;
+import java.io.File;
 import java.io.IOException;
-import java.util.LinkedList;
+import java.util.Iterator;
 
 import org.junit.Test;
 
@@ -33,49 +35,82 @@ public class JournalReaderTest {
 
     @Test
     public void testEmpty() throws IOException {
-        assertTrue(JournalReader.heads(getJournal("")).isEmpty());
+        JournalReader journalReader = createJournalReader("");
+        try {
+            assertFalse(journalReader.iterator().hasNext());
+        } finally {
+            journalReader.close();
+        }
     }
 
     @Test
     public void testSingleton() throws IOException {
-        String journal = "one 1";
-        LinkedList<String> heads = JournalReader.heads(getJournal(journal));
-        assertEquals(1, heads.size());
-        assertEquals("one", heads.get(0));
+        JournalReader journalReader = createJournalReader("one 1");
+        try {
+            Iterator<String> journal = journalReader.iterator();
+            assertTrue(journal.hasNext());
+            assertEquals("one", journal.next());
+            assertFalse(journal.hasNext());
+        } finally {
+            journalReader.close();
+        }
     }
 
     @Test
     public void testMultiple() throws IOException {
-        String journal = "one 1\ntwo 2\nthree 3";
-        LinkedList<String> heads = JournalReader.heads(getJournal(journal));
-        assertEquals(3, heads.size());
-        assertEquals("one", heads.get(0));
-        assertEquals("two", heads.get(1));
-        assertEquals("three", heads.get(2));
+        JournalReader journalReader = createJournalReader("one 1\ntwo 2\nthree 3");
+        try {
+            Iterator<String> journal = journalReader.iterator();
+            assertTrue(journal.hasNext());
+            assertEquals("three", journal.next());
+            assertTrue(journal.hasNext());
+            assertEquals("two", journal.next());
+            assertTrue(journal.hasNext());
+            assertEquals("one", journal.next());
+            assertFalse(journal.hasNext());
+        } finally {
+            journalReader.close();
+        }
     }
 
     @Test
     public void testSpaces() throws IOException {
-        String journal = "\n \n  \n   ";
-        LinkedList<String> heads = JournalReader.heads(getJournal(journal));
-        assertEquals(3, heads.size());
-        assertEquals("", heads.get(0));
-        assertEquals("", heads.get(1));
-        assertEquals("", heads.get(2));
+        JournalReader journalReader = createJournalReader("\n \n  \n   ");
+        try {
+            Iterator<String> journal = journalReader.iterator();
+            assertTrue(journal.hasNext());
+            assertEquals("", journal.next());
+            assertTrue(journal.hasNext());
+            assertEquals("", journal.next());
+            assertTrue(journal.hasNext());
+            assertEquals("", journal.next());
+            assertFalse(journal.hasNext());
+        } finally {
+            journalReader.close();
+        }
     }
 
     @Test
     public void testIgnoreInvalid() throws IOException {
-        String journal = "one 1\ntwo 2\ninvalid\nthree 3";
-        LinkedList<String> heads = JournalReader.heads(getJournal(journal));
-        assertEquals(3, heads.size());
-        assertEquals("one", heads.get(0));
-        assertEquals("two", heads.get(1));
-        assertEquals("three", heads.get(2));
-    }
-
-    private static DataInput getJournal(String s) {
-        return newDataInput(s.getBytes());
+        JournalReader journalReader = createJournalReader("one 1\ntwo 2\ninvalid\nthree 3");
+        try {
+            Iterator<String> journal = journalReader.iterator();
+            assertTrue(journal.hasNext());
+            assertEquals("three", journal.next());
+            assertTrue(journal.hasNext());
+            assertEquals("two", journal.next());
+            assertTrue(journal.hasNext());
+            assertEquals("one", journal.next());
+            assertFalse(journal.hasNext());
+        } finally {
+            journalReader.close();
+        }
+    }
+
+    private static JournalReader createJournalReader(String s) throws IOException {
+        File journalFile = createTempFile("jrt", null);
+        write(journalFile, s);
+        return new JournalReader(journalFile);
     }
 
 }



Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Michael Dürig <md...@apache.org>.

>> Could you try with the attached patch?
>>
>
> Looks good. First run passed.
>


Applied patch at http://svn.apache.org/r1644610.

Thanks for testing.

Michael

Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Julian Reschke <ju...@gmx.de>.
On 2014-12-11 09:47, Michael Dürig wrote:
>
> Julian,
>
> Could you try with the attached patch?
>
> Michael

Looks good. First run passed.



Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Michael Dürig <md...@apache.org>.
Julian,

Could you try with the attached patch?

Michael

On 10.12.14 11:40 , Julian Reschke wrote:
> On 2014-12-10 21:15, Michael Dürig wrote:
>>
>>
>> On 10.12.14 7:59 , Julian Reschke wrote:
>>>> segmentOverflow(org.apache.jackrabbit.oak.plugins.segment.file.FileStoreTest):
>>>>
>>>>
>>>>
>>>>  The process cannot access the file because another process has locked
>>>> a portion
>>>>  of the file
>>>
>>>
>>>
>>> Maybe related to this change?
>>
>> Probably yes. As I can't reproduce this, could you send a stack trace?
>>
>> Michael
>
> This one?
>

Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Vikas Saurabh <vi...@gmail.com>.
>>
>> Probably yes. As I can't reproduce this, could you send a stack trace?
>>
>> Michael
>
I am seeing the same issue on my laptop while travis works fine. I
have win7 with me... while Travis, I guess, would be running some
container on linux.

Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Julian Reschke <ju...@gmx.de>.
On 2014-12-10 21:15, Michael Dürig wrote:
>
>
> On 10.12.14 7:59 , Julian Reschke wrote:
>>> segmentOverflow(org.apache.jackrabbit.oak.plugins.segment.file.FileStoreTest):
>>>
>>>
>>>  The process cannot access the file because another process has locked
>>> a portion
>>>  of the file
>>
>>
>>
>> Maybe related to this change?
>
> Probably yes. As I can't reproduce this, could you send a stack trace?
>
> Michael

This one?


Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Michael Dürig <md...@apache.org>.

On 10.12.14 7:59 , Julian Reschke wrote:
>> segmentOverflow(org.apache.jackrabbit.oak.plugins.segment.file.FileStoreTest):
>>
>>  The process cannot access the file because another process has locked
>> a portion
>>  of the file
>
>
>
> Maybe related to this change?

Probably yes. As I can't reproduce this, could you send a stack trace?

Michael

Re: svn commit: r1644389 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/file/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ test/java/org/apache/jackrabbit/oak/plugins/segment/file/

Posted by Julian Reschke <ju...@gmx.de>.
On 2014-12-10 14:10, mduerig@apache.org wrote:
> Author: mduerig
> Date: Wed Dec 10 13:10:30 2014
> New Revision: 1644389
>
> URL: http://svn.apache.org/r1644389
> Log:
> OAK-2333: SegmentMK startup slow with large journals
> Scan the journal.log backwards
>
> Modified:
>      jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
>      jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReader.java
>      jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/tooling/ConsistencyChecker.java
>      jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/JournalReaderTest.java
> ...

I now see failures in FileStoreBackupTest -- reproducibly on Windows:

> Tests in error:
>   testBackup(org.apache.jackrabbit.oak.plugins.backup.FileStoreBackupTest): The
> process cannot access the file because another process has locked a portion of t
> he file
>   testRestore(org.apache.jackrabbit.oak.plugins.backup.FileStoreBackupTest): The
>  process cannot access the file because another process has locked a portion of
> the file
>   testRestartAndGCWithoutMM(org.apache.jackrabbit.oak.plugins.segment.file.FileS
> toreTest): The process cannot access the file because another process has locked
>  a portion of the file
>   testRestartAndGCWithMM(org.apache.jackrabbit.oak.plugins.segment.file.FileStor
> eTest): The process cannot access the file because another process has locked a
> portion of the file
>   testCompaction(org.apache.jackrabbit.oak.plugins.segment.file.FileStoreTest):
> The process cannot access the file because another process has locked a portion
> of the file
>   testRecovery(org.apache.jackrabbit.oak.plugins.segment.file.FileStoreTest): Th
> e process cannot access the file because another process has locked a portion of
>  the file
>   segmentOverflow(org.apache.jackrabbit.oak.plugins.segment.file.FileStoreTest):
>  The process cannot access the file because another process has locked a portion
>  of the file



Maybe related to this change?

Best regards, Julian