This is an automated email from the ASF dual-hosted git repository.
baedke pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new d96c9d0174 OAK-10660: DocumentNodeStore: avoid repeated commits of
:childOrder in branch commits (#1317)
d96c9d0174 is described below
commit d96c9d0174120ef9998ff17e93e313c6430668f7
Author: Julian Reschke <[email protected]>
AuthorDate: Wed Feb 28 16:32:03 2024 +0100
OAK-10660: DocumentNodeStore: avoid repeated commits of :childOrder in
branch commits (#1317)
Patched Commit#applyToDocumentStore(RevisionVector) to remove redundant
branch commits to the :childorder property.
Added test cases.
Added feature toggle to optionally disable the new behavior.
Co-Authored-by: stefan-egli <[email protected]>
---
.../apache/jackrabbit/oak/jcr/ManyChildrenIT.java | 65 -----------------
.../jackrabbit/oak/jcr/OrderableNodesTest.java | 85 ++++++++++++++++++++++
.../jackrabbit/oak/plugins/document/Commit.java | 39 ++++++++++
.../oak/plugins/document/DocumentNodeStore.java | 8 ++
.../plugins/document/DocumentNodeStoreBuilder.java | 11 +++
.../plugins/document/DocumentNodeStoreService.java | 5 ++
.../oak/fixture/DocumentMemoryFixture.java | 5 +-
.../oak/fixture/DocumentMongoFixture.java | 2 +
.../jackrabbit/oak/fixture/NodeStoreFixture.java | 8 ++
9 files changed, 162 insertions(+), 66 deletions(-)
diff --git
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ManyChildrenIT.java
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ManyChildrenIT.java
index eeac7c5cc8..a2bb7c0fa6 100644
--- a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ManyChildrenIT.java
+++ b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ManyChildrenIT.java
@@ -24,11 +24,8 @@ import javax.jcr.NodeIterator;
import javax.jcr.Session;
import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
-import org.junit.Ignore;
import org.junit.Test;
-import java.util.UUID;
-
/**
* Test nodes with many child nodes.
*/
@@ -80,66 +77,4 @@ public class ManyChildrenIT extends AbstractRepositoryTest {
writer.save();
assertTrue(test.hasNode("node-x"));
}
-
- @Test
- @Ignore //OAK-10646
- public void orderableAddManyChildrenWithSave() throws Exception {
- int childCount = 1000;
- StringBuilder prefix = new StringBuilder("");
- for (int k = 0; k < 90; k++) {
- prefix.append("0123456789");
- }
- Session session = getAdminSession();
- Node test = session.getRootNode().addNode("test", "nt:unstructured");
- session.save();
- for (int k = 0; k < childCount; k++) {
- test.addNode(prefix.toString() + k, "nt:unstructured");
- }
- }
-
- @Test
- @Ignore //OAK-10646
- public void moveOrderableWithManyChildren() throws Exception {
- int childCount = 1000;
- int moveCount = 1;
- StringBuilder prefix = new StringBuilder("");
- for (int k = 0; k < 90; k++) {
- prefix.append("0123456789");
- }
- Session session = getAdminSession();
- Node test = session.getRootNode().addNode("test-0", "nt:unstructured");
- session.save();
- for (int k = 0; k < childCount; k++) {
- test.addNode(prefix.toString() + k, "nt:unstructured");
- if (k % 100 == 0) {
- session.save();
- }
- }
- session.save();
- session.move("/test-0", "/test-1");
- session.save();
- }
-
- @Test
- @Ignore //OAK-10646
- public void copyOrderableWithManyChildren() throws Exception {
- int childCount = 1000;
- int copyCount = 1;
- StringBuilder prefix = new StringBuilder("");
- for (int k = 0; k < 90; k++) {
- prefix.append("0123456789");
- }
- Session session = getAdminSession();
- Node test = session.getRootNode().addNode("test-0", "nt:unstructured");
- session.save();
- for (int k = 0; k < childCount; k++) {
- test.addNode(prefix.toString() + k, "nt:unstructured");
- if (k % 100 == 0) {
- session.save();
- }
- }
- session.save();
- session.getWorkspace().copy("/test-0", "/test-1");
- session.save();
- }
}
diff --git
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/OrderableNodesTest.java
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/OrderableNodesTest.java
index d2bf0674b6..2e9c17ec89 100644
---
a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/OrderableNodesTest.java
+++
b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/OrderableNodesTest.java
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.jcr;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
@@ -30,6 +31,10 @@ import javax.jcr.Session;
import org.apache.jackrabbit.commons.iterator.NodeIterable;
import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
+import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.spi.toggle.FeatureToggle;
+import org.apache.jackrabbit.oak.spi.whiteboard.Tracker;
import org.junit.Test;
public class OrderableNodesTest extends AbstractRepositoryTest {
@@ -111,6 +116,86 @@ public class OrderableNodesTest extends
AbstractRepositoryTest {
assertEquals("b", it.nextNode().getName());
}
+ @Test
+ public void orderableAddManyChildrenWithSave() throws Exception {
+ int childCount = 1000;
+ StringBuilder prefix = new StringBuilder("");
+ for (int k = 0; k < 90; k++) {
+ prefix.append("0123456789");
+ }
+ Session session = getAdminSession();
+ Node test = session.getRootNode().addNode("test", "nt:unstructured");
+ session.save();
+ for (int k = 0; k < childCount; k++) {
+ test.addNode(prefix.toString() + k, "nt:unstructured");
+ }
+ }
+
+ @Test
+ public void moveOrderableWithManyChildren() throws Exception {
+ int childCount = 1000;
+ StringBuilder prefix = new StringBuilder("");
+ for (int k = 0; k < 90; k++) {
+ prefix.append("0123456789");
+ }
+ Session session = getAdminSession();
+ Node test = session.getRootNode().addNode("test-0", "nt:unstructured");
+ session.save();
+ for (int k = 0; k < childCount; k++) {
+ test.addNode(prefix.toString() + k, "nt:unstructured");
+ if (k % 100 == 0) {
+ session.save();
+ }
+ }
+ session.save();
+ session.move("/test-0", "/test-1");
+ session.save();
+ }
+
+ @Test
+ public void copyOrderableWithManyChildren() throws Exception {
+ int childCount = 1000;
+ StringBuilder prefix = new StringBuilder("");
+ for (int k = 0; k < 90; k++) {
+ prefix.append("0123456789");
+ }
+ Session session = getAdminSession();
+ Node test = session.getRootNode().addNode("test-0", "nt:unstructured");
+ session.save();
+ for (int k = 0; k < childCount; k++) {
+ test.addNode(prefix.toString() + k, "nt:unstructured");
+ if (k % 100 == 0) {
+ session.save();
+ }
+ }
+ session.save();
+ session.getWorkspace().copy("/test-0", "/test-1");
+ session.save();
+ }
+
+ @Test
+ public void childOrderCleanupFeatureToggleTest() throws
RepositoryException {
+ Tracker<FeatureToggle> track =
fixture.getWhiteboard().track(FeatureToggle.class);
+ NodeStore nodeStore = createNodeStore(fixture);
+ assertNotNull(nodeStore);
+ if (nodeStore instanceof DocumentNodeStore) {
+ DocumentNodeStore documentNodeStore = (DocumentNodeStore)
nodeStore;
+ assertTrue(documentNodeStore.isChildOrderCleanupEnabled());
+ for (FeatureToggle toggle : track.getServices()) {
+ if ("FT_NOCOCLEANUP_OAK-10660".equals(toggle.getName())) {
+ assertFalse(toggle.isEnabled());
+ assertTrue(documentNodeStore.isChildOrderCleanupEnabled());
+ toggle.setEnabled(true);
+ assertTrue(toggle.isEnabled());
+
assertFalse(documentNodeStore.isChildOrderCleanupEnabled());
+ toggle.setEnabled(false);
+ assertFalse(toggle.isEnabled());
+ assertTrue(documentNodeStore.isChildOrderCleanupEnabled());
+ }
+ }
+ }
+ }
+
private void doTest(String nodeType) throws RepositoryException {
Session session = getAdminSession();
Node root = session.getRootNode().addNode("test", nodeType);
diff --git
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
index a22e8adb1d..66d6928d04 100644
---
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
+++
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
@@ -24,7 +24,9 @@ import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.NavigableSet;
import java.util.Set;
+import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
import org.apache.jackrabbit.guava.common.base.Function;
@@ -32,6 +34,8 @@ import org.apache.jackrabbit.guava.common.collect.Iterables;
import org.apache.jackrabbit.guava.common.collect.Sets;
import org.apache.jackrabbit.oak.commons.json.JsopStream;
import org.apache.jackrabbit.oak.commons.json.JsopWriter;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Operation;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -57,6 +61,8 @@ public class Commit {
private static final Logger LOG = LoggerFactory.getLogger(Commit.class);
+ private static final String PROPERTY_NAME_CHILDORDER = ":childOrder";
+
protected final DocumentNodeStore nodeStore;
private final RevisionVector baseRevision;
private final RevisionVector startRevisions;
@@ -354,6 +360,39 @@ public class Commit {
boolean commitRootHasChanges = operations.containsKey(commitRootPath);
for (UpdateOp op : operations.values()) {
NodeDocument.setCommitRoot(op, revision, commitRootDepth);
+
+ // special case for :childOrder updates
+ if (nodeStore.isChildOrderCleanupEnabled()) {
+ final Branch localBranch = getBranch();
+ if (localBranch != null) {
+ final NavigableSet<Revision> commits = new
TreeSet<>(localBranch.getCommits());
+ boolean removePreviousSetOperations = false;
+ for (Map.Entry<Key, Operation> change :
op.getChanges().entrySet()) {
+ if
(PROPERTY_NAME_CHILDORDER.equals(change.getKey().getName()) &&
Operation.Type.SET_MAP_ENTRY == change.getValue().type) {
+ // we are setting child order, so we should remove
previous set operations from the same branch
+ removePreviousSetOperations = true;
+ // branch.getCommits contains all revisions of the
branch
+ // including the new one we're about to make
+ // so don't do a removeMapEntry for that
+
commits.remove(change.getKey().getRevision().asBranchRevision());
+ }
+ }
+ if (removePreviousSetOperations) {
+ if (!commits.isEmpty()) {
+ int countRemoves = 0;
+ for (Revision rev : commits.descendingSet()) {
+ op.removeMapEntry(PROPERTY_NAME_CHILDORDER,
rev.asTrunkRevision());
+ if (++countRemoves >= 256) {
+ LOG.debug("applyToDocumentStore : only
cleaning up last {} branch commits.",
+ countRemoves);
+ break;
+ }
+ }
+ LOG.debug("applyToDocumentStore :
childOrder-edited op is: {}", op);
+ }
+ }
+ }
+ }
changedNodes.add(op);
}
// create a "root of the commit" if there is none
diff --git
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
index 85c210c926..481cccbe82 100644
---
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
+++
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
@@ -571,6 +571,8 @@ public final class DocumentNodeStore
private final Feature cancelInvalidationFeature;
+ private final Feature noChildOrderCleanupFeature;
+
private Boolean cancelInvalidationLogged;
private CacheWarming cacheWarming;
@@ -648,6 +650,7 @@ public final class DocumentNodeStore
this.prefetchFeature = builder.getPrefetchFeature();
this.cancelInvalidationFeature =
builder.getCancelInvalidationFeature();
+ this.noChildOrderCleanupFeature =
builder.getNoChildOrderCleanupFeature();
this.cacheWarming = new CacheWarming(s);
this.journalPropertyHandlerFactory =
builder.getJournalPropertyHandlerFactory();
@@ -869,6 +872,11 @@ public final class DocumentNodeStore
}
}
+
+ public boolean isChildOrderCleanupEnabled() {
+ return noChildOrderCleanupFeature == null ||
!noChildOrderCleanupFeature.isEnabled();
+ }
+
public void dispose() {
LOG.info("Starting disposal of DocumentNodeStore with clusterNodeId:
{} ({})", clusterId,
getClusterNodeInfoDisplayString());
diff --git
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
index 070db7ad08..8c05d8b852 100644
---
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
+++
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java
@@ -127,6 +127,7 @@ public class DocumentNodeStoreBuilder<T extends
DocumentNodeStoreBuilder<T>> {
private boolean isReadOnlyMode = false;
private Feature prefetchFeature;
private Feature docStoreThrottlingFeature;
+ private Feature noChildOrderCleanupFeature;
private Feature cancelInvalidationFeature;
private Weigher<CacheValue, CacheValue> weigher = new EmpiricalWeigher();
private long memoryCacheSize = DEFAULT_MEMORY_CACHE_SIZE;
@@ -318,6 +319,16 @@ public class DocumentNodeStoreBuilder<T extends
DocumentNodeStoreBuilder<T>> {
return docStoreThrottlingFeature;
}
+ public T setNoChildOrderCleanupFeature(@Nullable Feature
noChildOrderCleanupFeature) {
+ this.noChildOrderCleanupFeature = noChildOrderCleanupFeature;
+ return thisBuilder();
+ }
+
+ @Nullable
+ public Feature getNoChildOrderCleanupFeature() {
+ return noChildOrderCleanupFeature;
+ }
+
public T setCancelInvalidationFeature(@Nullable Feature
cancelInvalidation) {
this.cancelInvalidationFeature = cancelInvalidation;
return thisBuilder();
diff --git
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
index e0f1222cd0..556c54e882 100644
---
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
+++
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
@@ -181,6 +181,8 @@ public class DocumentNodeStoreService {
*/
private static final String FT_NAME_DOC_STORE_THROTTLING =
"FT_THROTTLING_OAK-9909";
+ private static final String FT_NAME_DOC_STORE_NOCOCLEANUP =
"FT_NOCOCLEANUP_OAK-10660";
+
/**
* Feature toggle name to enable invalidation on cancel (due to a merge
collision)
*/
@@ -221,6 +223,7 @@ public class DocumentNodeStoreService {
private JournalPropertyHandlerFactory journalPropertyHandlerFactory = new
JournalPropertyHandlerFactory();
private Feature prefetchFeature;
private Feature docStoreThrottlingFeature;
+ private Feature noChildOrderCleanupFeature;
private Feature cancelInvalidationFeature;
private ComponentContext context;
private Whiteboard whiteboard;
@@ -256,6 +259,7 @@ public class DocumentNodeStoreService {
documentStoreType =
DocumentStoreType.fromString(this.config.documentStoreType());
prefetchFeature = Feature.newFeature(FT_NAME_PREFETCH, whiteboard);
docStoreThrottlingFeature =
Feature.newFeature(FT_NAME_DOC_STORE_THROTTLING, whiteboard);
+ noChildOrderCleanupFeature =
Feature.newFeature(FT_NAME_DOC_STORE_NOCOCLEANUP, whiteboard);
cancelInvalidationFeature =
Feature.newFeature(FT_NAME_CANCEL_INVALIDATION, whiteboard);
registerNodeStoreIfPossible();
@@ -474,6 +478,7 @@ public class DocumentNodeStoreService {
setLeaseCheckMode(ClusterNodeInfo.DEFAULT_LEASE_CHECK_DISABLED
? LeaseCheckMode.DISABLED : LeaseCheckMode.valueOf(config.leaseCheckMode())).
setPrefetchFeature(prefetchFeature).
setDocStoreThrottlingFeature(docStoreThrottlingFeature).
+ setNoChildOrderCleanupFeature(noChildOrderCleanupFeature).
setCancelInvalidationFeature(cancelInvalidationFeature).
setThrottlingEnabled(config.throttlingEnabled()).
setSuspendTimeoutMillis(config.suspendTimeoutMillis()).
diff --git
a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMemoryFixture.java
b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMemoryFixture.java
index 411814db1d..1b534b13b9 100644
---
a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMemoryFixture.java
+++
b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMemoryFixture.java
@@ -22,12 +22,15 @@ package org.apache.jackrabbit.oak.fixture;
import org.apache.jackrabbit.oak.plugins.document.DocumentMK;
import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.spi.toggle.Feature;
public class DocumentMemoryFixture extends NodeStoreFixture {
@Override
public NodeStore createNodeStore() {
- return new DocumentMK.Builder().getNodeStore();
+ DocumentMK.Builder builder = new DocumentMK.Builder();
+
builder.setNoChildOrderCleanupFeature(Feature.newFeature("FT_NOCOCLEANUP_OAK-10660",
getWhiteboard()));
+ return builder.getNodeStore();
}
@Override
diff --git
a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMongoFixture.java
b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMongoFixture.java
index 8b4d4305d1..1f66cc4113 100644
---
a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMongoFixture.java
+++
b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/fixture/DocumentMongoFixture.java
@@ -28,6 +28,7 @@ import
org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
import org.apache.jackrabbit.oak.plugins.document.MongoUtils;
import org.apache.jackrabbit.oak.spi.blob.BlobStore;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.spi.toggle.Feature;
import org.junit.AssumptionViolatedException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -69,6 +70,7 @@ public class DocumentMongoFixture extends NodeStoreFixture {
}
builder.setPersistentCache("target/persistentCache,time");
builder.setMongoDB(createClient(), getDBName(suffix));
+
builder.setNoChildOrderCleanupFeature(Feature.newFeature("FT_NOCOCLEANUP_OAK-10660",
getWhiteboard()));
DocumentNodeStore ns = builder.getNodeStore();
suffixes.put(ns, suffix);
return ns;
diff --git
a/oak-store-spi/src/test/java/org/apache/jackrabbit/oak/fixture/NodeStoreFixture.java
b/oak-store-spi/src/test/java/org/apache/jackrabbit/oak/fixture/NodeStoreFixture.java
index 2ab4831744..edc2784520 100644
---
a/oak-store-spi/src/test/java/org/apache/jackrabbit/oak/fixture/NodeStoreFixture.java
+++
b/oak-store-spi/src/test/java/org/apache/jackrabbit/oak/fixture/NodeStoreFixture.java
@@ -20,12 +20,16 @@
package org.apache.jackrabbit.oak.fixture;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
+import org.apache.jackrabbit.oak.spi.whiteboard.DefaultWhiteboard;
+import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
/**
* NodeStore fixture for parametrized tests.
*/
public abstract class NodeStoreFixture {
+ private final Whiteboard whiteboard = new DefaultWhiteboard();
+
/**
* Creates a new empty {@link NodeStore} instance. An implementation must
* ensure the returned node store is indeed empty and is independent from
@@ -52,4 +56,8 @@ public abstract class NodeStoreFixture {
return true;
}
+ public Whiteboard getWhiteboard() {
+ return whiteboard;
+ }
+
}
\ No newline at end of file
