You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/07/19 14:07:22 UTC
[jackrabbit-filevault] branch master updated: JCRVLT-547
distinguish between recoverable and non-recoverable exceptions during
intermediate save() (#158)
This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git
The following commit(s) were added to refs/heads/master by this push:
new 7fc2c70 JCRVLT-547 distinguish between recoverable and non-recoverable exceptions during intermediate save() (#158)
7fc2c70 is described below
commit 7fc2c70eed341cb788b126487a2f913f81129b04
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Jul 19 16:07:13 2021 +0200
JCRVLT-547 distinguish between recoverable and non-recoverable exceptions during intermediate save() (#158)
Reuse AutoSave helper also in RepositoryCopier.
---
.../apache/jackrabbit/vault/fs/api/ImportInfo.java | 2 +
.../apache/jackrabbit/vault/fs/io/AutoSave.java | 102 +++++++++++++++------
.../apache/jackrabbit/vault/fs/io/Importer.java | 10 +-
.../jackrabbit/vault/util/RepositoryCopier.java | 51 ++++++-----
4 files changed, 105 insertions(+), 60 deletions(-)
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java
index 92227c1..b66d93a 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java
@@ -66,7 +66,9 @@ public interface ImportInfo {
/**
* Marks that the node at {@code path} is missing.
* @param path the path
+ * @deprecated Is no longer issued, as this depends on the registered node type definitions
*/
+ @Deprecated
void onMissing(String path);
/**
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java
index 2d526e2..c2bf40b 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java
@@ -17,11 +17,10 @@
package org.apache.jackrabbit.vault.fs.io;
-import java.util.HashSet;
-import java.util.Set;
-
+import javax.jcr.InvalidItemStateException;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
+import javax.jcr.nodetype.ConstraintViolationException;
import org.apache.jackrabbit.vault.fs.spi.ProgressTracker;
import org.jetbrains.annotations.NotNull;
@@ -55,9 +54,9 @@ public class AutoSave {
private int threshold = 1024;
/**
- * set that records the missing mandatory items. save has to be delay until they are resolved
+ * number of modified nodes to wait for a save retry (only set to a value != 0 after failed save).
*/
- private final Set<String> missingMandatory = new HashSet<String>();
+ private int failedSaveThreshold = 0;
/**
* tracker to use to report progress messages
@@ -93,7 +92,6 @@ public class AutoSave {
ret.lastSave = lastSave;
ret.tracker = tracker;
ret.dryRun = dryRun;
- ret.missingMandatory.addAll(missingMandatory);
ret.debugFailEach = debugFailEach;
// don't copy save count, otherwise it will fail for ever.
// ret.debugSaveCount = debugSaveCount;
@@ -126,12 +124,16 @@ public class AutoSave {
* @return {@code true} if threshold reached.
*/
public boolean needsSave() {
- boolean res = (numModified - lastSave) >= threshold;
- if (res && !missingMandatory.isEmpty()) {
- log.debug("Threshold of {} reached but still unresolved mandatory items.", threshold);
- res = false;
- }
- return res;
+ return (numModified - lastSave) >= threshold + failedSaveThreshold;
+ }
+
+ /**
+ * Same as {@link #save(Session, boolean)} with the second argument being {@code true}.
+ * @param session
+ * @throws RepositoryException
+ */
+ public void save(@Nullable Session session) throws RepositoryException {
+ save(session, true);
}
/**
@@ -139,18 +141,19 @@ public class AutoSave {
* @param session the session to save. can be {@code null}
* @throws RepositoryException if an error occurs.
*/
- public void save(@Nullable Session session) throws RepositoryException {
- if (threshold == Integer.MAX_VALUE) {
- log.trace("Save disabled.");
- return;
- }
+ public void save(@Nullable Session session, boolean isIntermediate) throws RepositoryException {
int diff = numModified - lastSave;
- log.debug("Threshold of {} reached. {} approx {} transient changes. {} unresolved", new Object[]{
- threshold,
- dryRun ? "dry run, reverting" : "saving",
- diff,
- missingMandatory.size()
- });
+ if (isIntermediate) {
+ if (threshold == Integer.MAX_VALUE) {
+ log.trace("Intermediate save disabled.");
+ return;
+ }
+ log.debug("Threshold of {} reached. {} approx {} transient changes.",
+ threshold,
+ dryRun ? "dry run, reverting" : "saving",
+ diff
+ );
+ }
if (tracker != null) {
if (dryRun) {
tracker.track("reverting approx " + diff + " nodes... (dry run)", "");
@@ -158,6 +161,7 @@ public class AutoSave {
tracker.track("saving approx " + diff + " nodes...", "");
}
}
+ // TODO: how can session be null here?
if (session != null) {
if (debugFailEach > 0 && debugSaveCount > 0 && debugSaveCount%debugFailEach == 0) {
String msg = String.format("Debugging provoked failure after %s saves.", debugSaveCount);
@@ -165,21 +169,54 @@ public class AutoSave {
throw new RepositoryException(msg);
}
+ if (!saveWithBackoff(session)) {
+ // either retry after some more nodes have been modified or after throttle
+ // retry with next save() after another 10 nodes have been modified
+ failedSaveThreshold = 10;
+ log.warn("Retry auto-save after {} modified nodes", failedSaveThreshold);
+ }
+ }
+ lastSave = numModified;
+ failedSaveThreshold = 0;
+ }
+
+ /**
+ *
+ * @param session
+ * @return {@code true} in case was successful or {@code false} in case it failed with a potentially recoverable {@link RepositoryException}
+ * @throws RepositoryException in case of unrecoverable exceptions
+ */
+ private boolean saveWithBackoff(@NotNull Session session) throws RepositoryException {
+ try {
if (dryRun) {
session.refresh(false);
} else {
try {
session.save();
- debugSaveCount++;
} catch (RepositoryException e) {
log.error("error during auto save - retrying after refresh...");
session.refresh(true);
session.save();
- debugSaveCount++;
}
+ debugSaveCount++;
+ }
+ } catch (RepositoryException e) {
+ if (isPotentiallyTransientException(e)) {
+ log.warn("could not auto-save due to potentially transient exception {}", e.getCause());
+ log.debug("auto save exception", e);
+ return false;
+ } else {
+ throw e;
}
}
- lastSave = numModified;
+ return true;
+ }
+
+ boolean isPotentiallyTransientException(RepositoryException e) {
+ if (e instanceof InvalidItemStateException || e instanceof ConstraintViolationException) {
+ return true;
+ }
+ return false;
}
/**
@@ -208,16 +245,25 @@ public class AutoSave {
return needsSave();
}
+ /**
+ * As not working reliably it is simply ignored.
+ * @param path
+ */
+ @Deprecated
public void markMissing(@NotNull String path) {
- missingMandatory.add(path);
}
+ /**
+ * As not working reliably it is simply ignored.
+ * @param path
+ */
+ @Deprecated
public void markResolved(@NotNull String path) {
- missingMandatory.remove(path);
}
@Override
public String toString() {
return String.valueOf(threshold);
}
+
}
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java
index e94d8b1..2e4f533 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java
@@ -460,7 +460,7 @@ public class Importer {
while (recoveryRetryCounter++ < 10) {
try {
commit(session, root, skipList);
- autoSave.save(session);
+ autoSave.save(session, false);
break;
} catch (RepositoryException e) {
if (recoveryRetryCounter == 10) {
@@ -834,7 +834,7 @@ public class Importer {
}
if (autoSave.needsSave()) {
- autoSave.save(session);
+ autoSave.save(session, false);
// save checkpoint
cpTxInfo = info;
cpAutosave = autoSave.copy();
@@ -988,27 +988,21 @@ public class Importer {
switch (type) {
case CRE:
track("A", path);
- autoSave.markResolved(path);
break;
case DEL:
track("D", path);
- autoSave.markResolved(path);
break;
case MOD:
track("U", path);
- autoSave.markResolved(path);
break;
case NOP:
track("-", path);
- autoSave.markResolved(path);
break;
case REP:
track("R", path);
- autoSave.markResolved(path);
break;
case MIS:
track("!", path);
- autoSave.markMissing(path);
break;
case ERR:
Exception error = entry.getValue().getError();
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/util/RepositoryCopier.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/util/RepositoryCopier.java
index 6f57b2b..bff161e 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/util/RepositoryCopier.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/util/RepositoryCopier.java
@@ -39,6 +39,8 @@ import javax.jcr.nodetype.NodeType;
import org.apache.jackrabbit.vault.fs.api.ProgressTrackerListener;
import org.apache.jackrabbit.vault.fs.api.RepositoryAddress;
import org.apache.jackrabbit.vault.fs.api.WorkspaceFilter;
+import org.apache.jackrabbit.vault.fs.io.AutoSave;
+import org.apache.jackrabbit.vault.fs.spi.ProgressTracker;
import org.apache.jackrabbit.util.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -264,17 +266,21 @@ public class RepositoryCopier {
currentSize = 0;
totalSize = 0;
start = System.currentTimeMillis();
- copy(srcRoot, dstRoot, dstName, recursive);
+
+ AutoSave autoSave = new AutoSave();
+ autoSave.setThreshold(getBatchSize());
+ autoSave.setTracker(new ProgressTracker(tracker));
+ copy(autoSave, srcRoot, dstRoot, dstName, recursive);
if (numNodes > 0) {
track("", "Saving %d nodes...", numNodes);
- dstSession.save();
+ autoSave.save(dstSession, false);
track("", "Done.");
}
long end = System.currentTimeMillis();
track("", "Copy completed. %d nodes in %dms. %d bytes", totalNodes, end-start, totalSize);
}
- private void copy(Node src, Node dstParent, String dstName, boolean recursive)
+ private void copy(AutoSave autoSave, Node src, Node dstParent, String dstName, boolean recursive)
throws RepositoryException {
if (abort) {
return;
@@ -440,7 +446,7 @@ public class RepositoryCopier {
Node child = niter.nextNode();
String cName = checkNameSpace(child.getName(), src.getSession(), dst.getSession());
names.remove(cName);
- copy(child, dst, cName, true);
+ copy(autoSave, child, dst, cName, true);
}
if (resumeFrom == null) {
// check if we need to order
@@ -471,30 +477,27 @@ public class RepositoryCopier {
if (!skip) {
numNodes++;
+ autoSave.modified(1);
}
// check for save
- if (numNodes >= batchSize) {
- try {
- track("", "Intermediate saving %d nodes (%d kB)...", numNodes, currentSize/1000);
- long now = System.currentTimeMillis();
- dst.getSession().save();
- long end = System.currentTimeMillis();
- track("", "Done in %d ms. Total time: %d, total nodes %d, %d kB", end-now, end-start, totalNodes, totalSize/1000);
- lastKnownGood = currentPath;
- numNodes = 0;
- currentSize = 0;
- if (throttle > 0) {
- track("", "Throttling enabled. Waiting %d second%s...", throttle, throttle == 1 ? "" : "s");
- try {
- Thread.sleep(throttle * 1000);
- } catch (InterruptedException e) {
- log.warn("Interrupted while waiting", e);
- Thread.currentThread().interrupt();
- }
+ if (autoSave.needsSave()) {
+ track("", "Intermediate saving %d nodes (%d kB)...", numNodes, currentSize/1000);
+ long now = System.currentTimeMillis();
+ autoSave.save(dst.getSession(), true);
+ long end = System.currentTimeMillis();
+ track("", "Done in %d ms. Total time: %d, total nodes %d, %d kB", end-now, end-start, totalNodes, totalSize/1000);
+ lastKnownGood = currentPath;
+ numNodes = 0;
+ currentSize = 0;
+ if (throttle > 0) {
+ track("", "Throttling enabled. Waiting %d second%s...", throttle, throttle == 1 ? "" : "s");
+ try {
+ Thread.sleep(throttle * 1000);
+ } catch (InterruptedException e) {
+ log.warn("Interrupted while waiting", e);
+ Thread.currentThread().interrupt();
}
- } catch (RepositoryException e) {
- log.error("Error during intermediate save ({}); try again later: {}", numNodes, e.toString());
}
}
}