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 st...@apache.org on 2024/03/21 13:59:36 UTC
(jackrabbit-oak) branch OAK-10715 created (now 8ac79414e2)
This is an automated email from the ASF dual-hosted git repository.
stefanegli pushed a change to branch OAK-10715
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
at 8ac79414e2 OAK-10715 : embedded verification should use traversed nodeState
This branch includes the following new commits:
new 8ac79414e2 OAK-10715 : embedded verification should use traversed nodeState
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(jackrabbit-oak) 01/01: OAK-10715 : embedded verification should use traversed nodeState
Posted by st...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
stefanegli pushed a commit to branch OAK-10715
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 8ac79414e28550813af1975c3f14b1fe572643de
Author: stefan-egli <st...@apache.org>
AuthorDate: Thu Mar 21 14:59:31 2024 +0100
OAK-10715 : embedded verification should use traversed nodeState
---
.../plugins/document/VersionGarbageCollector.java | 71 +++++++++++++++++++++-
1 file changed, 70 insertions(+), 1 deletion(-)
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
index 5727a9f9b7..5322ca98f6 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
@@ -55,6 +55,7 @@ import org.apache.jackrabbit.oak.plugins.document.util.TimeInterval;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.apache.jackrabbit.oak.spi.gc.DelegatingGCMonitor;
import org.apache.jackrabbit.oak.spi.gc.GCMonitor;
+import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.stats.Clock;
import org.apache.jackrabbit.oak.api.PropertyState;
@@ -1644,16 +1645,43 @@ public class VersionGarbageCollector {
update.getId());
continue;
}
+ NodeState traversedParent = null;
+ NodeState traversedState = root;
+ for (String name : oldDoc.getPath().elements()) {
+ traversedParent = traversedState;
+ traversedState = traversedState.getChildNode(name);
+ }
+
NodeDocument newDoc = Collection.NODES.newDocument(ds);
oldDoc.deepCopy(newDoc);
UpdateUtils.applyChanges(newDoc, update);
- if (!verify(oldDoc, newDoc, update)) {
+ // for the time being, verify both with classic and traversed
+ final boolean classic = verify(oldDoc, newDoc, update);
+ final boolean verified = verify(traversedState, traversedParent,
+ newDoc);
+ if (classic != verified) {
+ throw new Error("totally unacceptable : classic = " + classic
+ + " while traversed = " + verified);
+ }
+ if (!verified) {
// verification failure
// let's skip this document
it.remove();
stats.skippedDetailedGCDocsCount++;
}
};
+ for (Entry<String, Long> e : orphanOrDeletedRemovalMap.entrySet()) {
+ NodeState traversedState = root;
+ for (String name : Path.fromString(Utils.getPathFromId(e.getKey())).elements()) {
+ traversedState = traversedState.getChildNode(name);
+ }
+ if (!verifyDeletion(traversedState)) {
+ // verification failure
+ // let's skip this document
+ it.remove();
+ stats.skippedDetailedGCDocsCount++;
+ }
+ }
}
if (!isDetailedGCDryRun) {
// only delete these in case it is not a dryRun
@@ -1719,6 +1747,47 @@ public class VersionGarbageCollector {
}
}
+ private boolean verify(NodeState traversedState, NodeState traversedParent,
+ NodeDocument newDoc) {
+ final Path path = newDoc.getPath();
+ final Revision lastRevision = nodeStore.getPendingModifications().get(path);
+ if (traversedParent == null && !newDoc.getPath().isRoot()) {
+ log.error("verify : no parent but not root for path : {}", newDoc.getPath());
+ return false;
+ }
+ final RevisionVector lastRev;
+ if (traversedParent == null && newDoc.getPath().isRoot()) {
+ if (!(traversedState instanceof DocumentNodeState)) {
+ log.error("verify : traversedState not a DocumentNodeState : {}",
+ traversedState.getClass());
+ return false;
+ }
+ lastRev = ((DocumentNodeState) traversedState).getLastRevision();
+ } else {
+ if (!traversedParent.exists()) {
+ // if the parent doesn't exist we shouldn't reach this point at all
+ log.error("verify : no parent but not marked for removal for path : {}", newDoc.getPath());
+ return false;
+ }
+ if (!(traversedParent instanceof DocumentNodeState)) {
+ log.error("verify : traversedParent not a DocumentNodeState : {}",
+ traversedParent.getClass());
+ return false;
+ }
+ lastRev = ((DocumentNodeState) traversedParent).getLastRevision();
+ }
+ final NodeState actual = newDoc.getNodeAtRevision(nodeStore, lastRev, lastRevision);
+ // use more thorough version of equals to ensure properties are checked
+ // (the faster state.equals() would stop if lastRev matches,
+ // but as we're fiddling with immuability rule of a document,
+ // we need to do a full check)
+ return AbstractNodeState.equals(traversedState, actual);
+ }
+
+ private boolean verifyDeletion(NodeState traversedState) {
+ return !traversedState.exists();
+ }
+
private boolean verify(NodeDocument oldDoc, NodeDocument newDoc, UpdateOp update) {
if (oldDoc.entrySet().equals(newDoc.entrySet())) {
return true;