diff --git oak-doc/src/site/markdown/nodestore/segment/overview.md oak-doc/src/site/markdown/nodestore/segment/overview.md index ff866ae..4cfdfed 100644 --- oak-doc/src/site/markdown/nodestore/segment/overview.md +++ oak-doc/src/site/markdown/nodestore/segment/overview.md @@ -553,7 +553,6 @@ The `--bin` option has no effect on binary properties stored in an external Blob If the `--filter` option is specified, the tool will traverse only the absolute paths specified as arguments. At least one argument is expected with this option; multiple arguments need to be comma-separated. The paths will be traversed in the same order as they were specified. -If one of the paths is invalid, the consistency check will fail and the traversal will not continue for the rest of the paths. If the option is not specified, the full traversal of the repository (rooted at `/`) will be performed. If the `--io-stats` option is specified, the tool will print some statistics about the I/O operations performed during the execution of the check command. diff --git oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java index bdf155c..f6bd66f 100644 --- oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java +++ oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java @@ -19,7 +19,7 @@ package org.apache.jackrabbit.oak.segment.file.tooling; -import static com.google.common.collect.Sets.newHashSet; +import static com.google.common.collect.Maps.newHashMap; import static org.apache.jackrabbit.oak.api.Type.BINARIES; import static org.apache.jackrabbit.oak.api.Type.BINARY; import static org.apache.jackrabbit.oak.commons.IOUtils.humanReadableByteCount; @@ -36,6 +36,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.PrintWriter; import java.text.MessageFormat; +import java.util.HashSet; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; @@ -120,29 +122,53 @@ public class ConsistencyChecker implements Closeable { JournalReader journal = new JournalReader(new File(directory, journalFileName)); ConsistencyChecker checker = new ConsistencyChecker(directory, debugInterval, ioStatistics, outWriter, errWriter) ) { - Set corruptPaths = newHashSet(); - String latestGoodRevision = null; + Map pathToValidRevision = newHashMap(); + Map> pathToCorruptPaths = newHashMap(); + for (String path : filterPaths) { + pathToCorruptPaths.put(path, new HashSet()); + } + + int count = 0; int revisionCount = 0; - while (journal.hasNext() && latestGoodRevision == null) { + while (journal.hasNext() && count < filterPaths.size()) { String revision = journal.next(); + checker.print("Checking revision {0}", revision); + try { revisionCount++; - String corruptPath = checker.checkRevision(revision, corruptPaths, filterPaths, checkBinaries); - - if (corruptPath == null) { - checker.print("Found latest good revision {0}", revision); - checker.print("Searched through {0} revisions", revisionCount); - latestGoodRevision = revision; - } else { - corruptPaths.add(corruptPath); - checker.print("Broken revision {0}", revision); + for (String path : filterPaths) { + if (pathToValidRevision.get(path) == null) { + + Set corruptPaths = pathToCorruptPaths.get(path); + String corruptPath = checker.checkPathAtRevision(revision, corruptPaths, path, checkBinaries); + + if (corruptPath == null) { + checker.print("Path {0} is consistent", path); + pathToValidRevision.put(path, revision); + count++; + } else { + corruptPaths.add(corruptPath); + } + } } } catch (IllegalArgumentException e) { checker.printError("Skipping invalid record id {0}", revision); } } + + checker.print("Searched through {0} revisions", revisionCount); + + if (count == 0) { + checker.print("No good revision found"); + } else { + for (String path : filterPaths) { + String validRevision = pathToValidRevision.get(path); + checker.print("Latest good revision for path {0} is {1}", path, + validRevision != null ? validRevision : "none"); + } + } if (ioStatistics) { checker.print( @@ -159,10 +185,6 @@ public class ConsistencyChecker implements Closeable { checker.statisticsIOMonitor.readTime ); } - - if (latestGoodRevision == null) { - checker.print("No good revision found"); - } } } @@ -197,50 +219,46 @@ public class ConsistencyChecker implements Closeable { * * @param revision revision to be checked * @param corruptPaths already known corrupt paths from previous revisions - * @param paths paths on which to run consistency check, + * @param path path on which to run consistency check, * provided there are no corrupt paths. * @param checkBinaries if {@code true} full content of binary properties will be scanned * @return {@code null}, if the content tree rooted at path is consistent * in this revision or the path of the first inconsistency otherwise. */ - public String checkRevision(String revision, Set corruptPaths, Set paths, boolean checkBinaries) { - print("Checking revision {0}", revision); + public String checkPathAtRevision(String revision, Set corruptPaths, String path, boolean checkBinaries) { String result = null; - try { - store.setRevision(revision); - NodeState root = SegmentNodeStoreBuilders.builder(store).build().getRoot(); + store.setRevision(revision); + NodeState root = SegmentNodeStoreBuilders.builder(store).build().getRoot(); - for (String corruptPath : corruptPaths) { + for (String corruptPath : corruptPaths) { + try { NodeWrapper wrapper = NodeWrapper.deriveTraversableNodeOnPath(root, corruptPath); result = checkNode(wrapper.node, wrapper.path, checkBinaries); if (result != null) { return result; } + } catch (IllegalArgumentException e) { + debug("Path {0} not found", corruptPath); } + } - nodeCount = 0; - propertyCount = 0; + nodeCount = 0; + propertyCount = 0; - for (String path : paths) { - print("Checking {0}", path); - - NodeWrapper wrapper = NodeWrapper.deriveTraversableNodeOnPath(root, path); - result = checkNodeAndDescendants(wrapper.node, wrapper.path, checkBinaries); - - if (result != null) { - break; - } - } + print("Checking {0}", path); + + try { + NodeWrapper wrapper = NodeWrapper.deriveTraversableNodeOnPath(root, path); + result = checkNodeAndDescendants(wrapper.node, wrapper.path, checkBinaries); + print("Checked {0} nodes and {1} properties", nodeCount, propertyCount); return result; - } catch (RuntimeException e) { - printError("Error while traversing {0}: {1}", revision, e.getMessage()); - return "/"; - } finally { - print("Checked {0} nodes and {1} properties", nodeCount, propertyCount); - } + } catch (IllegalArgumentException e) { + printError("Path {0} not found", path); + return path; + } } /** diff --git oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java index ed2ed8e..c602223 100644 --- oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java +++ oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java @@ -26,7 +26,6 @@ import java.util.Set; import org.apache.jackrabbit.oak.segment.tool.Check; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.Lists; @@ -42,9 +41,8 @@ public class CheckInvalidRepositoryTest extends CheckRepositoryTestBase { super.addInvalidRevision(); } - @Ignore @Test - public void testInvalidRevision() { + public void testInvalidRevisionFallbackOnValid() { StringWriter strOut = new StringWriter(); StringWriter strErr = new StringWriter(); @@ -58,7 +56,7 @@ public class CheckInvalidRepositoryTest extends CheckRepositoryTestBase { .withPath(new File(temporaryFolder.getRoot().getAbsolutePath())) .withJournal("journal.log") .withDebugInterval(Long.MAX_VALUE) - .withCheckBinaries(false) + .withCheckBinaries(true) .withFilterPaths(filterPaths) .withOutWriter(outWriter) .withErrWriter(errWriter) @@ -68,8 +66,37 @@ public class CheckInvalidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Broken revision", - "Checked 7 nodes and 15 properties", "Found latest good revision", "Searched through 2 revisions")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Checked 7 nodes and 21 properties", "Path / is consistent", + "Searched through 2 revisions")); assertExpectedOutput(strErr.toString(), Lists.newArrayList("Error while traversing /z")); } + + @Test + public void testBrokenPathWithoutValidRevision() { + StringWriter strOut = new StringWriter(); + StringWriter strErr = new StringWriter(); + + PrintWriter outWriter = new PrintWriter(strOut, true); + PrintWriter errWriter = new PrintWriter(strErr, true); + + Set filterPaths = new LinkedHashSet<>(); + filterPaths.add("/z"); + + Check.builder() + .withPath(new File(temporaryFolder.getRoot().getAbsolutePath())) + .withJournal("journal.log") + .withDebugInterval(Long.MAX_VALUE) + .withCheckBinaries(true) + .withFilterPaths(filterPaths) + .withOutWriter(outWriter) + .withErrWriter(errWriter) + .build() + .run(); + + outWriter.close(); + errWriter.close(); + + assertExpectedOutput(strOut.toString(), Lists.newArrayList("No good revision found")); + assertExpectedOutput(strErr.toString(), Lists.newArrayList("Error while traversing /z", "Path /z not found")); + } } diff --git oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java index fde8d2c..22284a5 100644 --- oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java +++ oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java @@ -69,13 +69,13 @@ public class CheckRepositoryTestBase { SegmentNodeStore nodeStore = SegmentNodeStoreBuilders.builder(fileStore).build(); NodeBuilder builder = nodeStore.getRoot().builder(); - addChildWithBlobProperties(nodeStore, builder, "a", 5); - addChildWithBlobProperties(nodeStore, builder, "b", 10); - addChildWithBlobProperties(nodeStore, builder, "c", 15); + addChildWithBlobProperties(nodeStore, builder, "a", 1); + addChildWithBlobProperties(nodeStore, builder, "b", 2); + addChildWithBlobProperties(nodeStore, builder, "c", 3); - addChildWithProperties(nodeStore, builder, "d", 5); + addChildWithProperties(nodeStore, builder, "d", 4); addChildWithProperties(nodeStore, builder, "e", 5); - addChildWithProperties(nodeStore, builder, "f", 5); + addChildWithProperties(nodeStore, builder, "f", 6); nodeStore.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); fileStore.close(); diff --git oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java index a751a2b..bd75916 100644 --- oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java +++ oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java @@ -30,7 +30,7 @@ import org.junit.Test; import com.google.common.collect.Lists; /** - * Tests for {@link CheckCommand} assuming a valid repository. + * Tests for {@link CheckCommand} assuming a consistent repository. */ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { @@ -60,7 +60,8 @@ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 7 nodes and 45 properties")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 7 nodes and 21 properties", + "Path / is consistent")); assertExpectedOutput(strErr.toString(), Lists.newArrayList("")); } @@ -95,7 +96,10 @@ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 6 nodes and 45 properties")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", + "Checked 1 nodes and 1 properties", "Checked 1 nodes and 2 properties", "Checked 1 nodes and 3 properties", + "Path /a is consistent", "Path /b is consistent", "Path /c is consistent", "Path /d is consistent", "Path /e is consistent", + "Path /f is consistent")); assertExpectedOutput(strErr.toString(), Lists.newArrayList("")); } @@ -124,7 +128,8 @@ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 7 nodes and 15 properties")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 7 nodes and 15 properties", + "Path / is consistent")); assertExpectedOutput(strErr.toString(), Lists.newArrayList("")); } @@ -156,7 +161,9 @@ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 4 nodes and 10 properties")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 1 nodes and 0 properties", + "Checked 1 nodes and 0 properties", "Checked 1 nodes and 4 properties", "Checked 1 nodes and 5 properties", + "Path /a is consistent", "Path /b is consistent", "Path /d is consistent", "Path /e is consistent")); assertExpectedOutput(strErr.toString(), Lists.newArrayList("")); } @@ -185,8 +192,8 @@ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Broken revision", "Checked 0 nodes and 0 properties", "No good revision found")); - assertExpectedOutput(strErr.toString(), Lists.newArrayList("Invalid path: /g")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "No good revision found")); + assertExpectedOutput(strErr.toString(), Lists.newArrayList("Path /g not found")); } @Test @@ -219,7 +226,9 @@ public class CheckValidRepositoryTest extends CheckRepositoryTestBase { outWriter.close(); errWriter.close(); - assertExpectedOutput(strOut.toString(), Lists.newArrayList("Broken revision", "Checked 2 nodes and 10 properties", "No good revision found")); - assertExpectedOutput(strErr.toString(), Lists.newArrayList("Invalid path: /g")); + assertExpectedOutput(strOut.toString(), Lists.newArrayList("Searched through 1 revisions", "Checked 1 nodes and 1 properties", + "Checked 1 nodes and 6 properties", "Checked 1 nodes and 4 properties", "Checked 1 nodes and 5 properties", + "Path /a is consistent", "Path /f is consistent", "Path /d is consistent", "Path /e is consistent")); + assertExpectedOutput(strErr.toString(), Lists.newArrayList("Path /g not found")); } }