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 ad...@apache.org on 2017/02/24 13:44:22 UTC
svn commit: r1784277 - in /jackrabbit/oak/trunk:
oak-doc/src/site/markdown/nodestore/segment/
oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/
oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/
Author: adulceanu
Date: Fri Feb 24 13:44:22 2017
New Revision: 1784277
URL: http://svn.apache.org/viewvc?rev=1784277&view=rev
Log:
OAK-5753 - Consistency check incorrectly fails for broken partial paths
Each partial path is now handled on its own
Changed assertions in tests
Modified:
jackrabbit/oak/trunk/oak-doc/src/site/markdown/nodestore/segment/overview.md
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java
jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java
jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java
jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java
Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/nodestore/segment/overview.md
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/nodestore/segment/overview.md?rev=1784277&r1=1784276&r2=1784277&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-doc/src/site/markdown/nodestore/segment/overview.md (original)
+++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/nodestore/segment/overview.md Fri Feb 24 13:44:22 2017
@@ -553,7 +553,6 @@ The `--bin` option has no effect on bina
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.
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java?rev=1784277&r1=1784276&r2=1784277&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java Fri Feb 24 13:44:22 2017
@@ -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 implemen
JournalReader journal = new JournalReader(new File(directory, journalFileName));
ConsistencyChecker checker = new ConsistencyChecker(directory, debugInterval, ioStatistics, outWriter, errWriter)
) {
- Set<String> corruptPaths = newHashSet();
- String latestGoodRevision = null;
+ Map<String, String> pathToValidRevision = newHashMap();
+ Map<String, Set<String>> pathToCorruptPaths = newHashMap();
+ for (String path : filterPaths) {
+ pathToCorruptPaths.put(path, new HashSet<String>());
+ }
+
+ 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<String> 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 implemen
checker.statisticsIOMonitor.readTime
);
}
-
- if (latestGoodRevision == null) {
- checker.print("No good revision found");
- }
}
}
@@ -197,50 +219,46 @@ public class ConsistencyChecker implemen
*
* @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<String> corruptPaths, Set<String> paths, boolean checkBinaries) {
- print("Checking revision {0}", revision);
+ public String checkPathAtRevision(String revision, Set<String> 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;
+ }
}
/**
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java?rev=1784277&r1=1784276&r2=1784277&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckInvalidRepositoryTest.java Fri Feb 24 13:44:22 2017
@@ -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
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
.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
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<String> 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"));
+ }
}
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java?rev=1784277&r1=1784276&r2=1784277&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckRepositoryTestBase.java Fri Feb 24 13:44:22 2017
@@ -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();
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java?rev=1784277&r1=1784276&r2=1784277&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/tooling/CheckValidRepositoryTest.java Fri Feb 24 13:44:22 2017
@@ -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 ex
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 ex
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 ex
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 ex
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 ex
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 ex
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"));
}
}