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 2021/04/06 17:12:51 UTC
svn commit: r1888445 - in /jackrabbit/oak/branches/1.22: ./
oak-doc/src/site/markdown/nodestore/document/
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/do...
Author: stefanegli
Date: Tue Apr 6 17:12:51 2021
New Revision: 1888445
URL: http://svn.apache.org/viewvc?rev=1888445&view=rev
Log:
OAK-9176 : backported from trunk to 1.22 - backported revision are : 1882480,1882482,1882484,1883226 - plus 1 additional import of SystemPropertySupplier added to DocumentNodeStore that was missing
Added:
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingBcSweeper2.java
- copied unchanged from r1882480, jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingBcSweeper2.java
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2Helper.java
- copied unchanged from r1882480, jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2Helper.java
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java
- copied, changed from r1882480, jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/Sweep2Test.java
- copied unchanged from r1882480, jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/Sweep2Test.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/Sweep2TestHelper.java
- copied unchanged from r1882480, jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/Sweep2TestHelper.java
Modified:
jackrabbit/oak/branches/1.22/ (props changed)
jackrabbit/oak/branches/1.22/RELEASE-NOTES.txt
jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/document/upgrade.md
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeper.java
jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchStateTest.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreSweepIT.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/FailingDocumentStore.java
jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/V18BranchCommitTest.java
Propchange: jackrabbit/oak/branches/1.22/
------------------------------------------------------------------------------
Merged /jackrabbit/oak/trunk:r1882480,1882482,1882484,1883226
Modified: jackrabbit/oak/branches/1.22/RELEASE-NOTES.txt
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/RELEASE-NOTES.txt?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/RELEASE-NOTES.txt (original)
+++ jackrabbit/oak/branches/1.22/RELEASE-NOTES.txt Tue Apr 6 17:12:51 2021
@@ -25,6 +25,8 @@ Bug
Improvement
[OAK-9261] - Upgrade Apache Solr to 8.6.3 and remove Embedded Solr Server
+ [OAK-9176] - Fix for sweep missing _bc (branch commit) entries in some cases - plus sweep2 introduced to clean up repository accordingly.
+ PS: use -Doak.documentMK.disableSweep2=true to disable sweep2 in case it should be skipped explicitly
Task
Modified: jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/document/upgrade.md
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/document/upgrade.md?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/document/upgrade.md (original)
+++ jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/document/upgrade.md Tue Apr 6 17:12:51 2021
@@ -21,6 +21,27 @@ of Oak to simply run on data written by
additional manual steps are needed or recommended to ensure performance and
reduce downtime of a system to a minimum.
+## Upgrade to OAK-9176
+
+OAK-9176 introduces a fix to the sweep functionality. The original sweep
+prior to this fix had cases where it could missing setting the branch commit
+("_bc") properties appropriately. OAK-9176 fixes this for any new sweep.
+However, documents that went through a sweep prior to OAK-9176 might have
+missing "_bc" entries. In order to resolve this, OAK-9176 introduces
+a one-time sweep2.
+
+Sweep2 automatically runs at startup once Oak is upgraded and detects
+whether or not it needs to run. If it needs to run, it will do so
+in the background, occupying only 50% of CPU max - however, it has to
+travers the repository and does therefore take some time.
+When sweep2 finished (or determined it is not necessary), it stores
+this information in the settings collection. Subsequent startups
+will therefore from then on skip sweep2.
+
+If sweep2 should be disabled explicitly for one reason or another,
+eg if it causes problems, it can be done so via a System Property:
+-Doak.documentMK.disableSweep2=true.
+
## <a name="1.8"></a>Upgrade to 1.8
Oak 1.8 introduced some changes in the DocumentNodeStore that require an
@@ -114,4 +135,4 @@ should be replaced. Please note, the par
impact on the system.
[0]: https://issues.apache.org/jira/browse/OAK-4345
-[1]: https://docs.mongodb.com/manual/tutorial/build-indexes-on-replica-sets/#index-building-replica-sets
\ No newline at end of file
+[1]: https://docs.mongodb.com/manual/tutorial/build-indexes-on-replica-sets/#index-building-replica-sets
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Tue Apr 6 17:12:51 2021
@@ -48,6 +48,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -95,6 +96,7 @@ import org.apache.jackrabbit.oak.plugins
import org.apache.jackrabbit.oak.spi.blob.BlobStore;
import org.apache.jackrabbit.oak.commons.json.JsopStream;
import org.apache.jackrabbit.oak.commons.json.JsopWriter;
+import org.apache.jackrabbit.oak.commons.properties.SystemPropertySupplier;
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.cache.CacheStats;
@@ -201,6 +203,10 @@ public final class DocumentNodeStore
private final int createOrUpdateBatchSize =
Integer.getInteger("oak.documentMK.createOrUpdateBatchSize", 1000);
+ public static final String SYS_PROP_DISABLE_SWEEP2 = "oak.documentMK.disableSweep2";
+ private boolean disableSweep2 = SystemPropertySupplier.create(SYS_PROP_DISABLE_SWEEP2, Boolean.FALSE).loggingTo(LOG)
+ .get();
+
/**
* The document store without potentially lease checking wrapper.
*/
@@ -349,6 +355,12 @@ public final class DocumentNodeStore
private Thread backgroundSweepThread;
/**
+ * Extra, one-off sweep2 background task for fixing OAK-9176 ie missing _bc
+ * on parents and root.
+ */
+ private Thread backgroundSweep2Thread;
+
+ /**
* The sweep revision vector. Revisions for trunk commits older than this
* can safely be considered committed without looking up the commit value
* on the commit root document.
@@ -712,6 +724,56 @@ public final class DocumentNodeStore
// triggering sweep
runBackgroundUpdateOperations();
+ // check if we need a sweep2 *before* doing a backgroundSweep.
+ // this enables us to detect a direct Oak <= 1.6 upgrade situation,
+ // where a sweep2 is *not* needed.
+
+ // there are 3 different cases with sweep[1]/sweep2:
+ // 1) Oak <= 1.6 direct upgrade:
+ // -> no sweep2 is needed as a sweep1 is needed anyway and sweep2
+ // from now on happens as part of it (with the OAK-9176 fix)
+ // 2) Oak >= 1.8 which never did an Oak <= 1.6 upgrade:
+ // -> no sweep2 is needed as OAK-9176 doesn't apply (the repository
+ // never ran <= 1.6)
+ // 3) Oak >= 1.8 which was previously doing an Oak <= 1.6 upgrade:
+ // -> A (full) sweep2 is needed. This is the main case of OAK-9176.
+
+ // In case 3 there is a valid, recent "_sweepRev" - and
+ // we can go ahead and do a "quick" backgroundSweep() here
+ // before continuing, to unblock the startup.
+ // After that, an async/background task is started for sweep2.
+
+ // which of cases 1-3 we have is determined via 'sweep2LockIfNecessary'
+ // and recorded in the settings collection.
+
+ // except for this case detection (which acquires a "sweep2 lock" if needed)
+ // we can otherwise continue normally. That means, a sweep1 can
+ // be considered as usual.
+ // Note that for case 3, doing this normal sweep1 can now also
+ // fix some "_bc" - which before OAK-9176 were missing
+ // and which sweep2 would separately fix as well - but this is not a problem.
+
+ // Note that by setting the SYS_PROP_DISABLE_SWEEP2 system property
+ // the sweep2 is bypassed and the sweep2 status is explicitly stored as "swept".
+ final long sweep2Lock;
+ if (disableSweep2) {
+ try {
+ final Sweep2StatusDocument sweep2Status = Sweep2StatusDocument.readFrom(store);
+ if (sweep2Status == null || !sweep2Status.isSwept()) {
+ // setting the disableSweep2 flag stores this in the repository
+ Sweep2StatusDocument.forceReleaseSweep2LockAndMarkSwept(store, clusterId);
+ }
+ } catch(Exception e) {
+ LOG.warn("<init> sweep2 is diabled as instructed by system property ("
+ + SYS_PROP_DISABLE_SWEEP2 + "=true) - however, got an Exception"
+ + " while storing sweep2 status in the settings collection: " + e, e);
+ }
+ sweep2Lock = -1;
+ } else {
+ // So: unless sweep2 is disabled: acquire sweep2 if one is (maybe) necessary
+ sweep2Lock = Sweep2Helper.acquireSweep2LockIfNecessary(store, clusterId);
+ }
+
// perform an initial document sweep if needed
// this may be long running if there is no sweep revision
// for this clusterId (upgrade from Oak <= 1.6).
@@ -720,6 +782,15 @@ public final class DocumentNodeStore
backgroundUpdateThread.start();
backgroundSweepThread.start();
+
+ if (sweep2Lock >= 0) {
+ // sweep2 is necessary - so start a sweep2 background task
+ backgroundSweep2Thread = new Thread(
+ new BackgroundSweep2Operation(this, isDisposed, sweep2Lock),
+ "DocumentNodeStore background sweep2 thread " + threadNamePostfix);
+ backgroundSweep2Thread.setDaemon(true);
+ backgroundSweep2Thread.start();
+ }
}
persistentCache = builder.getPersistentCache();
@@ -756,7 +827,8 @@ public final class DocumentNodeStore
Utils.joinQuietly(backgroundReadThread,
backgroundUpdateThread,
- backgroundSweepThread);
+ backgroundSweepThread,
+ backgroundSweep2Thread);
DocumentStoreException ex = null;
@@ -2496,6 +2568,158 @@ public final class DocumentNodeStore
}
+ /**
+ * Executes the sweep2 (from within a background thread)
+ * @param sweep2Lock the lock value originally acquired
+ * @return true if sweep2 is done or no longer needed, false otherwise (in which case it should be retried)
+ * @throws DocumentStoreException
+ */
+ boolean backgroundSweep2(long sweep2Lock) throws DocumentStoreException {
+ if (sweep2Lock == 0) {
+ sweep2Lock = Sweep2Helper.acquireSweep2LockIfNecessary(store, clusterId);
+ if (sweep2Lock == 0) {
+ // still not well defined, retry in a minute (done in BackgroundSweep2Operation)
+ return false;
+ }
+ if (sweep2Lock == -1) {
+ // then we're done
+ return true;
+ }
+ }
+ // sweep2Lock > 0, the local instance holds the lock
+ Sweep2StatusDocument statusDoc = Sweep2StatusDocument.readFrom(store);
+ if (statusDoc != null /*should never be null as we hold the lock, but let's play safe anyway .. */
+ && statusDoc.isChecking()) {
+ // very likely no 'isSweep2Necessary' might not have been done yet, let's do it now
+ LOG.info("backgroundSweep2: checking whether sweep2 is necessary...");
+ if (!Sweep2Helper.isSweep2Necessary(store)) {
+ LOG.info("backgroundSweep2: sweep2 check determined a sweep2 is NOT necessary. Marking sweep2 status as 'swept'.");
+ if (!Sweep2StatusDocument.forceReleaseSweep2LockAndMarkSwept(store, clusterId)) {
+ LOG.error("backgroundSweep2 : failed to update the sweep2 status to 'swept'. Aborting sweep2 for now.");
+ }
+ return true;
+ }
+ LOG.info("backgroundSweep2: sweep2 check determined a sweep2 IS necessary. Marking sweep2 status as 'sweeping'.");
+ }
+ // update the lock status one more time to ensure no check can conclude sweep2 is not necessary anymore
+ sweep2Lock = Sweep2StatusDocument.acquireOrUpdateSweep2Lock(store, clusterId,
+ true /* this true here is what's relevant: locks-in the 'sweeping' status */);
+ if (sweep2Lock == 0) {
+ // something came in between, retry later
+ LOG.info("backgroundSweep2: could not update the sweep2 lock to sweeping, someone got in between. Retry later.");
+ return false;
+ } else if (sweep2Lock == -1) {
+ // odd, someone else concluded we're done
+ LOG.info("backgroundSweep2: meanwhile, someone else concluded sweep2 is done.");
+ return true;
+ }
+
+ // compile the list of clusterIds for which sweep2 should be done.
+ // in an ideal situation sweep(1) would have been done for all clusterIds,
+ // and in that case "_sweepRev" will contain all clusterIds ever used.
+ // However, it is a supported situation that not sweep(1) was not run for all clusterIds,
+ // and in this case sweep2 must also not be done for all, but only for those clusterIds.
+ List<Integer> includedClusterIds = new LinkedList<>();
+ for (Revision aSweepRev : sweepRevisions) {
+ includedClusterIds.add(aSweepRev.getClusterId());
+ }
+
+ // at this point we did properly acquire a lock and can go ahead doing sweep2
+ LOG.info("backgroundSweep2: starting sweep2 (includedClusterIds={})", includedClusterIds);
+ int num = forceBackgroundSweep2(includedClusterIds);
+ LOG.info("backgroundSweep2: finished sweep2, num swept=" + num);
+
+ // release the lock.
+ // Note that in theory someone else could have released our lock, or that
+ // the sweep2 status was deleted - that actually doesn't matter:
+ // we just went through a full, successful sweep2 and we want to record it
+ // that way - irrespective of any interference with the status
+ // -> hence the 'force' aspect of releasing here
+ if (!Sweep2StatusDocument.forceReleaseSweep2LockAndMarkSwept(store, clusterId)) {
+ LOG.error("backgroundSweep2 : sweep2 finished but we failed to update the sweep2 status accordingly");
+ }
+ return true;
+ }
+
+ /**
+ * Executes a sweep2 either only for the provided or for all clusterIds otherwise
+ * (which case applies depends on the status of sweep(1) in the repository).
+ * @param includedClusterIds restrict sweep2 to only these clusterIds - or do it for
+ * all clusterIds if this list is empty or null.
+ * @return number of documents swept
+ * @throws DocumentStoreException
+ */
+ int forceBackgroundSweep2(List<Integer> includedClusterIds) throws DocumentStoreException {
+ final RevisionVector emptySweepRevision = new RevisionVector();
+ CommitValueResolver cvr = new CachingCommitValueResolver(
+ 0 /* disable caching for sweep2 as caching has a risk of propagating wrong values */,
+ () -> emptySweepRevision);
+ MissingBcSweeper2 sweeper = new MissingBcSweeper2(this, cvr, includedClusterIds, isDisposed);
+ LOG.info("Starting document sweep2. Head: {}, starting at 0", getHeadRevision());
+ Iterable<NodeDocument> docs = lastRevSeeker.getCandidates(0);
+ try {
+ final AtomicInteger numUpdates = new AtomicInteger();
+
+ sweeper.sweep2(docs, new NodeDocumentSweepListener() {
+ @Override
+ public void sweepUpdate(final Map<Path, UpdateOp> updates)
+ throws DocumentStoreException {
+ // create a synthetic commit. this commit does not have any
+ // changes, we just use it to create a journal entry for
+ // cache invalidation and apply the sweep updates
+ backgroundOperationLock.readLock().lock();
+ try {
+ boolean success = false;
+ Revision r = commitQueue.createRevision();
+ try {
+ commitQueue.done(r, new CommitQueue.Callback() {
+ @Override
+ public void headOfQueue(@NotNull Revision revision) {
+ writeUpdates(updates, revision);
+ }
+ });
+ success = true;
+ } finally {
+ if (!success && commitQueue.contains(r)) {
+ commitQueue.canceled(r);
+ }
+ }
+ } finally {
+ backgroundOperationLock.readLock().unlock();
+ }
+ }
+
+ private void writeUpdates(Map<Path, UpdateOp> updates,
+ Revision revision)
+ throws DocumentStoreException {
+ // create journal entry
+ JournalEntry entry = JOURNAL.newDocument(getDocumentStore());
+ entry.modified(updates.keySet());
+ Revision r = newRevision().asBranchRevision();
+ if (!store.create(JOURNAL, singletonList(entry.asUpdateOp(r)))) {
+ String msg = "Unable to create journal entry for " +
+ "document invalidation. Will be retried with " +
+ "next background sweep2 operation.";
+ throw new DocumentStoreException(msg);
+ }
+ changes.invalidate(Collections.singleton(r));
+ unsavedLastRevisions.put(ROOT, revision);
+ RevisionVector newHead = getHeadRevision().update(revision);
+ setRoot(newHead);
+ commitQueue.headRevisionChanged();
+
+ store.createOrUpdate(NODES, Lists.newArrayList(updates.values()));
+ numUpdates.addAndGet(updates.size());
+ LOG.debug("Background sweep2 updated {}", updates.keySet());
+ }
+ });
+
+ return numUpdates.get();
+ } finally {
+ Utils.closeIfCloseable(docs);
+ }
+ }
+
private int backgroundSweep() throws DocumentStoreException {
// decide if it needs to run
Revision head = getHeadRevision().getRevision(clusterId);
@@ -3296,6 +3520,50 @@ public final class DocumentNodeStore
}
}
+ /**
+ * Background sweep2 operation (also see OAK-9176 for details/context).
+ */
+ private static class BackgroundSweep2Operation extends NodeStoreTask {
+
+ private final long sweep2Lock;
+ private int retryCount = 0;
+
+ BackgroundSweep2Operation(DocumentNodeStore nodeStore,
+ AtomicBoolean isDisposed,
+ long sweep2Lock) {
+ // default asyncDelay is 1000ms == 1sec
+ // the sweep2 is fine to run every 60sec by default as it is not time critical
+ // to achieve this we're doing a Math.min(60sec, 60 * getAsyncDelay())
+ super(nodeStore, isDisposed,
+ Suppliers.ofInstance(Math.min(60000, 60 * nodeStore.getAsyncDelay())));
+ if (sweep2Lock < 0) {
+ throw new IllegalArgumentException("sweep2Lock must not be negative");
+ }
+ this.sweep2Lock = sweep2Lock;
+ }
+
+ @Override
+ protected void execute(@NotNull DocumentNodeStore nodeStore) {
+ if (retryCount > 0) {
+ LOG.info("BackgroundSweep2Operation.execute: retrying sweep2. retryCount=" + retryCount);
+ }
+ if (nodeStore.backgroundSweep2(sweep2Lock)) {
+ done();
+ }
+ // log the fact that we noticed an ongoing sweep2 - is only logged once every 5min
+ // until either the other instance finishes or crashes, at which point we or another
+ // instance will pick up
+ if (++retryCount % 5 == 0) {
+ LOG.info("BackgroundSweep2Operation.execute: another instance is currently (still) executing sweep2. "
+ + "Waiting for its outcome. retryCount=" + retryCount);
+ }
+ }
+
+ private void done() {
+ ref.clear();
+ }
+ }
+
private static class BackgroundLeaseUpdate extends NodeStoreTask {
/** OAK-4859 : log if time between two renewClusterIdLease calls is too long **/
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Tue Apr 6 17:12:51 2021
@@ -231,7 +231,7 @@ public final class NodeDocument extends
* and all previous non-branch revisions by this cluster node can be
* considered committed.
*/
- private static final String SWEEP_REV = "_sweepRev";
+ static final String SWEEP_REV = "_sweepRev";
//~----------------------------< Split Document Types >
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeper.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeper.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeper.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeper.java Tue Apr 6 17:12:51 2021
@@ -40,7 +40,7 @@ import static org.apache.jackrabbit.oak.
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.removeCommitRoot;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.removeRevision;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.setDeletedOnce;
-import static org.apache.jackrabbit.oak.plugins.document.util.Utils.PROPERTY_OR_DELETED;
+import static org.apache.jackrabbit.oak.plugins.document.util.Utils.PROPERTY_OR_DELETED_OR_COMMITROOT_OR_REVISIONS;
/**
* The {@code NodeDocumentSweeper} is responsible for removing uncommitted
@@ -56,6 +56,9 @@ final class NodeDocumentSweeper {
private static final long LOGINTERVALMS = TimeUnit.MINUTES.toMillis(1);
+ /** holds the Predicate actually used in sweepOne. This is modifiable ONLY FOR TESTING PURPOSE */
+ static Predicate<String> SWEEP_ONE_PREDICATE = PROPERTY_OR_DELETED_OR_COMMITROOT_OR_REVISIONS;
+
private final RevisionContext context;
private final int clusterId;
@@ -178,7 +181,12 @@ final class NodeDocumentSweeper {
private UpdateOp sweepOne(NodeDocument doc) throws DocumentStoreException {
UpdateOp op = createUpdateOp(doc);
- for (String property : filter(doc.keySet(), PROPERTY_OR_DELETED)) {
+ // go through PROPERTY_OR_DELETED_OR_COMMITROOT_OR_REVISIONS, whereas :
+ // - PROPERTY : for content changes
+ // - DELETED : for new node (this)
+ // - COMMITROOT : for new child (parent)
+ // - REVISIONS : for commit roots (root for branch commits)
+ for (String property : filter(doc.keySet(), SWEEP_ONE_PREDICATE)) {
Map<Revision, String> valueMap = doc.getLocalMap(property);
for (Map.Entry<Revision, String> entry : valueMap.entrySet()) {
Revision rev = entry.getKey();
Copied: jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java (from r1882480, jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java)
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java?p2=jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java&p1=jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java&r1=1882480&r2=1888445&rev=1888445&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Sweep2StatusDocument.java Tue Apr 6 17:12:51 2021
@@ -54,7 +54,7 @@ public class Sweep2StatusDocument {
/**
* Acquires the sweep2 lock.
- * @param documentNodeStore
+ * @param documentStore
* @param clusterId
* @param forceSweepingStatus if false uses the default way to set the sweep2-status,
* if set to true (force-)sets sweep2-status to 'sweeping'.
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java Tue Apr 6 17:12:51 2021
@@ -58,6 +58,8 @@ import org.slf4j.LoggerFactory;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Iterables.transform;
import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isDeletedEntry;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isCommitRootEntry;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.isRevisionsEntry;
/**
* Utility methods.
@@ -106,6 +108,26 @@ public class Utils {
}
};
+ /**
+ * A predicate for property, _deleted, _commitRoot or _revisions names.
+ */
+ public static final Predicate<String> PROPERTY_OR_DELETED_OR_COMMITROOT_OR_REVISIONS = new Predicate<String>() {
+ @Override
+ public boolean apply(@Nullable String input) {
+ return Utils.isPropertyName(input) || isDeletedEntry(input) || isCommitRootEntry(input) || isRevisionsEntry(input);
+ }
+ };
+
+ /**
+ * A predicate for _commitRoot and _revisions names.
+ */
+ public static final Predicate<String> COMMITROOT_OR_REVISIONS = new Predicate<String>() {
+ @Override
+ public boolean apply(@Nullable String input) {
+ return isCommitRootEntry(input) || isRevisionsEntry(input);
+ }
+ };
+
public static int pathDepth(String path) {
if (path.equals("/")) {
return 0;
@@ -983,7 +1005,9 @@ public class Utils {
public static void joinQuietly(Thread... threads) {
for (Thread t : threads) {
try {
- t.join();
+ if (t != null) {
+ t.join();
+ }
} catch (InterruptedException e) {
// ignore
}
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchStateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchStateTest.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchStateTest.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchStateTest.java Tue Apr 6 17:12:51 2021
@@ -68,6 +68,7 @@ public class BranchStateTest {
}
assertNotNull(store.find(Collection.NODES, testId));
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
}
private static final class FailingHook implements CommitHook {
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BranchTest.java Tue Apr 6 17:12:51 2021
@@ -24,6 +24,7 @@ import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.plugins.document.Branch.BranchCommit;
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -31,13 +32,16 @@ import org.apache.jackrabbit.oak.spi.sta
import org.junit.Rule;
import org.junit.Test;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
import static org.apache.jackrabbit.oak.plugins.document.TestUtils.asDocumentState;
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge;
import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch;
import static org.apache.jackrabbit.oak.plugins.document.util.Utils.getRootDocument;
import static org.hamcrest.Matchers.contains;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -88,6 +92,135 @@ public class BranchTest {
}
@Test
+ public void rootBranchCommitChildTest() throws Exception {
+ MemoryDocumentStore store = new MemoryDocumentStore();
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setClusterId(1)
+ .setDocumentStore(store).build();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child("a");
+ persistToBranch(builder);
+
+ merge(ns, builder);
+
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
+ }
+
+ @Test
+ public void childBranchCommitChildTest() throws Exception {
+ MemoryDocumentStore store = new MemoryDocumentStore();
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setClusterId(1)
+ .setDocumentStore(store).build();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child("a");
+ merge(ns, builder);
+
+ builder = ns.getRoot().builder();
+ builder.child("a").child("b");
+ persistToBranch(builder);
+
+ merge(ns, builder);
+
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
+ }
+
+ @Test
+ public void manyBranchCommitsDepthTest() throws Exception {
+ MemoryDocumentStore store = new MemoryDocumentStore();
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setClusterId(1)
+ .setDocumentStore(store).build();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child("a");
+ persistToBranch(builder);
+
+ builder.getChildNode("a").child("b");
+ persistToBranch(builder);
+
+ builder.getChildNode("a").getChildNode("b").child("c");
+ persistToBranch(builder);
+
+ builder.getChildNode("a").getChildNode("b").getChildNode("c").child("d");
+ persistToBranch(builder);
+
+ builder.getChildNode("a").getChildNode("b").getChildNode("c").getChildNode("d").child("e");
+ persistToBranch(builder);
+
+ merge(ns, builder);
+
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
+ }
+
+ @Test
+ public void manyBranchCommitsWidthTest() throws Exception {
+ MemoryDocumentStore store = new MemoryDocumentStore();
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setClusterId(1)
+ .setDocumentStore(store).build();
+
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child("a");
+ persistToBranch(builder);
+
+ builder.child("b");
+ persistToBranch(builder);
+
+ builder.child("c");
+ persistToBranch(builder);
+
+ builder.child("d");
+ persistToBranch(builder);
+
+ builder.child("e");
+ persistToBranch(builder);
+
+ merge(ns, builder);
+
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
+ }
+
+ @Test
+ public void mixedPre18BranchTest() throws Exception {
+ MemoryDocumentStore store = new MemoryDocumentStore();
+ DocumentNodeStore ns = builderProvider.newBuilder()
+ .setClusterId(1)
+ .setDocumentStore(store).build();
+ // step 1 : create an initial structure /a/b/c
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child("a").child("b").child("c").setProperty("commit", "1");
+ merge(ns, builder);
+
+ // step 2 : create a branch commit incl. with a child under an existing node, ie /a/b/c/d..
+ builder = ns.getRoot().builder();
+ builder.child("a").setProperty("commit", "2");
+ builder.child("a").child("b").child("c").child("d").child("e").setProperty("commit", "2");
+ persistToBranch(builder);
+ merge(ns, builder);
+
+ // step 3 : verify that "_bc" are set correctly : on all except /a/b (where nothing was changed)
+ assertNotNull(store.find(NODES, Utils.getIdFromPath("/")).get("_bc"));
+ assertNotNull(store.find(NODES, Utils.getIdFromPath("/a")).get("_bc"));
+ assertNull(store.find(NODES, Utils.getIdFromPath("/a/b")).get("_bc"));
+ assertNotNull(store.find(NODES, Utils.getIdFromPath("/a/b/c")).get("_bc"));
+ assertNotNull(store.find(NODES, Utils.getIdFromPath("/a/b/c/d")).get("_bc"));
+ assertNotNull(store.find(NODES, Utils.getIdFromPath("/a/b/c/d/e")).get("_bc"));
+
+ // step 4 : verify the "_bc" are set correctly by backgroundSweep()/forceBackgroundSweep()
+
+ // step 4b: /a/b/c did not get a "_bc" because it only contained "_commitRoot" but no other changes, and
+ // https://github.com/apache/jackrabbit-oak/blob/d35346d4d446908c7019e931cb54d88824c1a637/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeper.java#L179
+ // only went through document that had changes in properties in PROPERTY_OR_DELETED
+
+ // step 4c: / also failed with a similar reason as the above, except the root
+ // only has changes in "_revisions" (not "_commitRoot")
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
+ }
+
+ @Test
public void orphanedBranchTest() {
MemoryDocumentStore store = new MemoryDocumentStore();
DocumentNodeStore ns = builderProvider.newBuilder()
@@ -101,6 +234,8 @@ public class BranchTest {
ns = builderProvider.newBuilder()
.setDocumentStore(store).build();
assertFalse(ns.getRoot().hasProperty("p"));
+
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
}
@Test
@@ -136,6 +271,8 @@ public class BranchTest {
DocumentNodeState state = root.getNodeAtRevision(ns, br, null);
assertNotNull(state);
assertEquals("a", state.getString("p"));
+
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
}
private void assertModifiedPaths(Iterable<Path> actual, String... expected) {
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreSweepIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreSweepIT.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreSweepIT.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreSweepIT.java Tue Apr 6 17:12:51 2021
@@ -105,7 +105,7 @@ public class DocumentNodeStoreSweepIT ex
}
- private static boolean isClean(DocumentNodeStore ns, String path) {
+ static boolean isClean(DocumentNodeStore ns, String path) {
// use find that also reads from the cache
NodeDocument doc = ns.getDocumentStore().find(NODES, Utils.getIdFromPath(path));
for (Revision c : doc.getAllChanges()) {
@@ -144,7 +144,7 @@ public class DocumentNodeStoreSweepIT ex
store.fail().never();
}
- private String createUncommittedChanges(DocumentNodeStore ns,
+ static String createUncommittedChanges(DocumentNodeStore ns,
FailingDocumentStore store) throws Exception {
ns.setMaxBackOffMillis(0);
NodeBuilder builder = ns.getRoot().builder();
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/FailingDocumentStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/FailingDocumentStore.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/FailingDocumentStore.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/FailingDocumentStore.java Tue Apr 6 17:12:51 2021
@@ -28,6 +28,8 @@ import org.apache.jackrabbit.oak.plugins
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
+import java.util.LinkedList;
+
/**
* Wraps a document store and can be instructed to fail operations.
*/
@@ -43,6 +45,8 @@ class FailingDocumentStore extends Docum
private Type exceptionType = Type.GENERIC;
+ private List<Collection<? extends Document>> collectionIncludeList;
+
class Fail {
private Fail() {
@@ -65,6 +69,7 @@ class FailingDocumentStore extends Docum
numFailures.set(0);
failAfter.set(Long.MAX_VALUE);
exceptionType = Type.GENERIC;
+ collectionIncludeList = null;
}
void once() {
@@ -79,6 +84,14 @@ class FailingDocumentStore extends Docum
p = probability;
return this;
}
+
+ Fail on(Collection<? extends Document> collectionInclude) {
+ if (collectionIncludeList == null) {
+ collectionIncludeList = new LinkedList<>();
+ }
+ collectionIncludeList.add(collectionInclude);
+ return this;
+ }
}
FailingDocumentStore(DocumentStore store, long seed) {
@@ -101,7 +114,7 @@ class FailingDocumentStore extends Docum
@Override
public <T extends Document> void remove(Collection<T> collection,
String key) {
- maybeFail();
+ maybeFail(collection);
super.remove(collection, key);
}
@@ -120,7 +133,7 @@ class FailingDocumentStore extends Docum
int num = 0;
// remove individually
for (Map.Entry<String, Long> rm : toRemove.entrySet()) {
- maybeFail();
+ maybeFail(collection);
num += super.remove(collection, singletonMap(rm.getKey(), rm.getValue()));
}
return num;
@@ -132,7 +145,7 @@ class FailingDocumentStore extends Docum
long startValue,
long endValue)
throws DocumentStoreException {
- maybeFail();
+ maybeFail(collection);
return super.remove(collection, indexedProperty, startValue, endValue);
}
@@ -141,7 +154,7 @@ class FailingDocumentStore extends Docum
List<UpdateOp> updateOps) {
// create individually
for (UpdateOp op : updateOps) {
- maybeFail();
+ maybeFail(collection);
if (!super.create(collection, singletonList(op))) {
return false;
}
@@ -152,7 +165,7 @@ class FailingDocumentStore extends Docum
@Override
public <T extends Document> T createOrUpdate(Collection<T> collection,
UpdateOp update) {
- maybeFail();
+ maybeFail(collection);
return super.createOrUpdate(collection, update);
}
@@ -170,12 +183,13 @@ class FailingDocumentStore extends Docum
@Override
public <T extends Document> T findAndUpdate(Collection<T> collection,
UpdateOp update) {
- maybeFail();
+ maybeFail(collection);
return super.findAndUpdate(collection, update);
}
- private void maybeFail() {
- if (random.nextFloat() < p || failAfter.getAndDecrement() <= 0) {
+ private <T extends Document> void maybeFail(Collection<T> collection) {
+ if ((collectionIncludeList == null || collectionIncludeList.contains(collection)) &&
+ (random.nextFloat() < p || failAfter.getAndDecrement() <= 0)) {
if (numFailures.getAndDecrement() > 0) {
throw new DocumentStoreException("write operation failed", null, exceptionType);
}
Modified: jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/V18BranchCommitTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/V18BranchCommitTest.java?rev=1888445&r1=1888444&r2=1888445&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/V18BranchCommitTest.java (original)
+++ jackrabbit/oak/branches/1.22/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/V18BranchCommitTest.java Tue Apr 6 17:12:51 2021
@@ -22,6 +22,7 @@ import java.util.UUID;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -43,6 +44,16 @@ public class V18BranchCommitTest {
.setAsyncDelay(0).getNodeStore();
}
+ /**
+ * This test piggybacks on all the other tests with the idea that they
+ * leave the repository with some useful "_bc" seeds, which the test
+ * is keen on testing by simulating a pre 1.8 upgrade.
+ */
+ @After
+ public void tearDownPiggybackTest() {
+ Sweep2TestHelper.testPre18UpgradeSimulations(ns, builderProvider);
+ }
+
@Test
public void addNode() throws Exception {
NodeBuilder builder = ns.getRoot().builder();