Re: [PR] add service principal support while deleting container in oak-run-commons [jackrabbit-oak]
nit0906 commented on code in PR #1542:
URL: https://github.com/apache/jackrabbit-oak/pull/1542#discussion_r1650346400
##
oak-run-commons/src/test/java/org/apache/jackrabbit/oak/fixture/DataStoreUtilsTest.java:
##
@@ -0,0 +1,259 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.fixture;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.Appender;
+import ch.qos.logback.core.read.ListAppender;
+import com.microsoft.azure.storage.StorageException;
+import com.microsoft.azure.storage.blob.CloudBlobContainer;
+import com.microsoft.azure.storage.blob.SharedAccessBlobPermissions;
+import com.microsoft.azure.storage.blob.SharedAccessBlobPolicy;
+import org.apache.commons.lang3.StringUtils;
+import
org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzureBlobContainerProvider;
+import org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzureConstants;
+import
org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzuriteDockerRule;
+import org.jetbrains.annotations.NotNull;
+import org.junit.After;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.slf4j.LoggerFactory;
+
+import java.net.URISyntaxException;
+import java.security.InvalidKeyException;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class DataStoreUtilsTest {
+@ClassRule
+public static AzuriteDockerRule azuriteDockerRule = new
AzuriteDockerRule();
+
+private static final String AZURE_ACCOUNT_NAME = "AZURE_ACCOUNT_NAME";
+private static final String AZURE_TENANT_ID = "AZURE_TENANT_ID";
+private static final String AZURE_CLIENT_ID = "AZURE_CLIENT_ID";
+private static final String AZURE_CLIENT_SECRET = "AZURE_CLIENT_SECRET";
+private static final String AZURE_CONNECTION_STRING =
"DefaultEndpointsProtocol=http;AccountName=%s;AccountKey=%s;BlobEndpoint=%s";
+private static final String CONTAINER_NAME = "oak-blob-test";
+private static final String AUTHENTICATE_VIA_AZURE_CONNECTION_STRING_LOG =
"connecting to azure blob storage via azureConnectionString";
+private static final String AUTHENTICATE_VIA_ACCESS_KEY_LOG = "connecting
to azure blob storage via access key";
+private static final String AUTHENTICATE_VIA_SERVICE_PRINCIPALS_LOG =
"connecting to azure blob storage via service principal credentials";
+private static final String AUTHENTICATE_VIA_SAS_TOKEN_LOG = "connecting
to azure blob storage via sas token";
+private static final String REFRESH_TOKEN_EXECUTOR_SHUTDOWN_LOG = "Refresh
token executor service shutdown completed";
+private static final String CONTAINER_DOES_NOT_EXIST_MESSAGE = "container
[%s] doesn't exists";
+private static final String CONTAINER_DELETED_MESSAGE = "container [%s]
deleted";
+
+private CloudBlobContainer container;
+
+@Before
+public void init() throws URISyntaxException, InvalidKeyException,
StorageException {
+container = azuriteDockerRule.getContainer(CONTAINER_NAME);
Review Comment:
Does this init method assures that the container will always be present
before any test executes ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] add service principal support while deleting container in oak-run-commons [jackrabbit-oak]
nit0906 commented on code in PR #1542:
URL: https://github.com/apache/jackrabbit-oak/pull/1542#discussion_r1650340220
##
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/fixture/DataStoreUtils.java:
##
@@ -146,26 +150,52 @@ public static void deleteBucket(String bucket,
Map map, Date date) th
}
public static void deleteAzureContainer(Map config, String
containerName) throws Exception {
+if (config == null) {
+return;
Review Comment:
cannot initialize*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] add service principal support while deleting container in oak-run-commons [jackrabbit-oak]
nit0906 commented on code in PR #1542:
URL: https://github.com/apache/jackrabbit-oak/pull/1542#discussion_r1650340383
##
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/fixture/DataStoreUtils.java:
##
@@ -146,26 +150,52 @@ public static void deleteBucket(String bucket,
Map map, Date date) th
}
public static void deleteAzureContainer(Map config, String
containerName) throws Exception {
+if (config == null) {
+return;
+}
if (Strings.isNullOrEmpty(containerName)) {
return;
Review Comment:
cannot initialize*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10904: change token refresh executor to use daemon thread [jackrabbit-oak]
t-rana commented on code in PR #1545:
URL: https://github.com/apache/jackrabbit-oak/pull/1545#discussion_r1648425265
##
oak-segment-azure/src/main/java/org/apache/jackrabbit/oak/segment/azure/AzureUtilities.java:
##
@@ -74,7 +74,12 @@ public final class AzureUtilities {
private static final long TOKEN_REFRESHER_DELAY = 1L;
private static final Logger log =
LoggerFactory.getLogger(AzureUtilities.class);
-private static final ScheduledExecutorService executorService =
Executors.newSingleThreadScheduledExecutor();
+
+private static final ScheduledExecutorService executorService =
Executors.newSingleThreadScheduledExecutor(runnable -> {
Review Comment:
> Not closing it properly could cause random issues in tests for example
I did not face any issues in the test suite run, can you please elaborate so
that I can handle the same? I guess the framework running test terminates the
JVM and shutdown method is executed which will close this exectuor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644787553
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -60,6 +73,11 @@ public void before() throws Exception {
ns = builderProvider.newBuilder().setBlobStore(bs).getNodeStore();
}
+@After
+public void tearDown() {
+ns.dispose();
Review Comment:
```suggestion
try {
ns.dispose();
} finally {
DocumentPropertyState.setCompressionThreshold(DISABLED_COMPRESSION);
}
```
to avoid side-effects, the test should ensure it doesn't leak a non default
compression threshold to other tests. DISABLED_COMPRESSION is for illustration
to refer to -1 - ideally the -1 would only be defined in the source class...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on PR #1526: URL: https://github.com/apache/jackrabbit-oak/pull/1526#issuecomment-2176272288 [looks](https://github.com/apache/jackrabbit-oak/blob/14c357120e49d83281dc473d810ca485deb94501/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/plugins/memory/AbstractPropertyState.java#L103-L106) like the original intention was that properties shouldn't or wouldn't be used as keys in hash maps... if that's the case then it should be fine.. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Issue/oak 8848 [jackrabbit-oak]
shodaaan commented on code in PR #1474:
URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1644584397
##
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveVersionableNodeRepositoryTest.java:
##
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr;
+
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.NodeStoreFixtures;
+import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
+import org.apache.jackrabbit.test.NotExecutableException;
+import org.junit.Test;
+import org.junit.runners.Parameterized;
+
+import javax.jcr.Node;
+import javax.jcr.Session;
+import javax.jcr.version.Version;
+import javax.jcr.version.VersionHistory;
+import javax.jcr.version.VersionIterator;
+import javax.jcr.version.VersionManager;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import static java.util.Collections.singleton;
+import static
org.apache.jackrabbit.oak.commons.FixturesHelper.Fixture.DOCUMENT_NS;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for moving versionable nodes over deleted versionable nodes using the
DOCUMENT_NS fixture.
+ */
+public class MoveVersionableNodeRepositoryTest extends AbstractRepositoryTest {
+
+public MoveVersionableNodeRepositoryTest(NodeStoreFixture fixture) {
+super(fixture);
+}
+
+@Parameterized.Parameters(name="{0}")
+public static Collection memoryFixture() {
+return NodeStoreFixtures.asJunitParameters(singleton(DOCUMENT_NS));
+}
+
+/**
+ * Creates a versionable node with the specified name under the given
parent node, then
+ * saves the session.
+ * @param parent
+ * @param nodeName
+ * @return
+ * @throws Exception
+ */
+private Node createVersionableNode(Node parent, String nodeName) throws
Exception {
+Node newNode = (parent.hasNode(nodeName)) ? parent.getNode(nodeName) :
parent.addNode(nodeName);
+if (newNode.canAddMixin(JcrConstants.MIX_VERSIONABLE)) {
+newNode.addMixin(JcrConstants.MIX_VERSIONABLE);
+} else {
+throw new NotExecutableException();
+}
+newNode.getSession().save();
+return newNode;
+}
+
+/**
+ * Checks out the node, sets the property then saves the session and
checks the node back in.
+ * To be used in tests where version history needs to be populated.
+ */
+private void setNodePropertyAndCheckIn(Node node, String propertyName,
String propertyValue) throws Exception {
+node.checkout();
+node.setProperty(propertyName, propertyValue);
+node.getSession().save();
+node.checkin();
+}
+
+/*
+ * 1. Create a versionable unstructured node at
nodeName1/nodeName2/sourceNode
+ * 2. Create a versionable unstructured node at
nodeName1/nodeName3/sourceNode
+ * 3. create version histories for both nodes
+ * 4. remove nodeName1/nodeName3/nodeName1 (that's because move(src,dest)
throws an exception if dest already exists)
+ * 5. move nodeName1/nodeName2/sourceNode to
nodeName1/nodeName3/sourceNode and call session.save()
+ * 6. should work according to JCR specification - reproduces bug for
OAK-8848: will throw ConstraintViolationException
+ * - "Property is protected: jcr:versionHistory
+ */
+@Test
+public void
testMoveNodeWithVersionHistoryOverDeletedNodeWithVersionHistory() throws
Exception {
+
+Session session = getAdminSession();
+Node testRootNode = session.getRootNode().addNode("node1");
+
+String newNodeName = "sourceNode";
+Node sourceParent = testRootNode.addNode("node2"); //
nodeName1/nodeName2
+Node sourceNode = createVersionableNode(sourceParent, newNodeName); //
nodeName1/nodeName2/sourceNode
+Node destParent = testRootNode.addNode("node3"); // nodeName1/nodeName3
+Node destNode = createVersionableNode(destParent, "destNode"); //
nodeName1/nodeName3/sourceNode
+
+String destPath = destNode.getPath();
+
+// add version histories for sourceNode and destNode
+setNodePropertyAndCheckIn(sourceNode,
Re: [PR] Issue/oak 8848 [jackrabbit-oak]
stefan-egli commented on code in PR #1474:
URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1644565491
##
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/MoveVersionableNodeRepositoryTest.java:
##
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr;
+
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.NodeStoreFixtures;
+import org.apache.jackrabbit.oak.fixture.NodeStoreFixture;
+import org.apache.jackrabbit.test.NotExecutableException;
+import org.junit.Test;
+import org.junit.runners.Parameterized;
+
+import javax.jcr.Node;
+import javax.jcr.Session;
+import javax.jcr.version.Version;
+import javax.jcr.version.VersionHistory;
+import javax.jcr.version.VersionIterator;
+import javax.jcr.version.VersionManager;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import static java.util.Collections.singleton;
+import static
org.apache.jackrabbit.oak.commons.FixturesHelper.Fixture.DOCUMENT_NS;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Test for moving versionable nodes over deleted versionable nodes using the
DOCUMENT_NS fixture.
+ */
+public class MoveVersionableNodeRepositoryTest extends AbstractRepositoryTest {
+
+public MoveVersionableNodeRepositoryTest(NodeStoreFixture fixture) {
+super(fixture);
+}
+
+@Parameterized.Parameters(name="{0}")
+public static Collection memoryFixture() {
+return NodeStoreFixtures.asJunitParameters(singleton(DOCUMENT_NS));
+}
+
+/**
+ * Creates a versionable node with the specified name under the given
parent node, then
+ * saves the session.
+ * @param parent
+ * @param nodeName
+ * @return
+ * @throws Exception
+ */
+private Node createVersionableNode(Node parent, String nodeName) throws
Exception {
+Node newNode = (parent.hasNode(nodeName)) ? parent.getNode(nodeName) :
parent.addNode(nodeName);
+if (newNode.canAddMixin(JcrConstants.MIX_VERSIONABLE)) {
+newNode.addMixin(JcrConstants.MIX_VERSIONABLE);
+} else {
+throw new NotExecutableException();
+}
+newNode.getSession().save();
+return newNode;
+}
+
+/**
+ * Checks out the node, sets the property then saves the session and
checks the node back in.
+ * To be used in tests where version history needs to be populated.
+ */
+private void setNodePropertyAndCheckIn(Node node, String propertyName,
String propertyValue) throws Exception {
+node.checkout();
+node.setProperty(propertyName, propertyValue);
+node.getSession().save();
+node.checkin();
+}
+
+/*
+ * 1. Create a versionable unstructured node at
nodeName1/nodeName2/sourceNode
+ * 2. Create a versionable unstructured node at
nodeName1/nodeName3/sourceNode
+ * 3. create version histories for both nodes
+ * 4. remove nodeName1/nodeName3/nodeName1 (that's because move(src,dest)
throws an exception if dest already exists)
+ * 5. move nodeName1/nodeName2/sourceNode to
nodeName1/nodeName3/sourceNode and call session.save()
+ * 6. should work according to JCR specification - reproduces bug for
OAK-8848: will throw ConstraintViolationException
+ * - "Property is protected: jcr:versionHistory
+ */
+@Test
+public void
testMoveNodeWithVersionHistoryOverDeletedNodeWithVersionHistory() throws
Exception {
+
+Session session = getAdminSession();
+Node testRootNode = session.getRootNode().addNode("node1");
+
+String newNodeName = "sourceNode";
+Node sourceParent = testRootNode.addNode("node2"); //
nodeName1/nodeName2
+Node sourceNode = createVersionableNode(sourceParent, newNodeName); //
nodeName1/nodeName2/sourceNode
+Node destParent = testRootNode.addNode("node3"); // nodeName1/nodeName3
+Node destNode = createVersionableNode(destParent, "destNode"); //
nodeName1/nodeName3/sourceNode
+
+String destPath = destNode.getPath();
+
+// add version histories for sourceNode and destNode
+setNodePropertyAndCheckIn(sourceNode,
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644558711
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -127,8 +188,25 @@ public boolean equals(Object object) {
return true;
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
-return this.name.equals(other.name)
-&& this.value.equals(other.value);
+// Compare names and raw un-parsed values
+if (!this.name.equals(other.name) ||
!this.getValue().equals(other.getValue())) {
Review Comment:
Yes, but only then, not always.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644549857
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -127,8 +188,25 @@ public boolean equals(Object object) {
return true;
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
-return this.name.equals(other.name)
-&& this.value.equals(other.value);
+// Compare names and raw un-parsed values
+if (!this.name.equals(other.name) ||
!this.getValue().equals(other.getValue())) {
Review Comment:
"getValue()" will uncompress when the value is compressed, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644545109
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -127,8 +188,25 @@ public boolean equals(Object object) {
return true;
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
-return this.name.equals(other.name)
-&& this.value.equals(other.value);
+// Compare names and raw un-parsed values
+if (!this.name.equals(other.name) ||
!this.getValue().equals(other.getValue())) {
Review Comment:
Not necessarily.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644545109
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -127,8 +188,25 @@ public boolean equals(Object object) {
return true;
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
-return this.name.equals(other.name)
-&& this.value.equals(other.value);
+// Compare names and raw un-parsed values
+if (!this.name.equals(other.name) ||
!this.getValue().equals(other.getValue())) {
Review Comment:
Not necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644499232
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -127,8 +188,25 @@ public boolean equals(Object object) {
return true;
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
-return this.name.equals(other.name)
-&& this.value.equals(other.value);
+// Compare names and raw un-parsed values
+if (!this.name.equals(other.name) ||
!this.getValue().equals(other.getValue())) {
Review Comment:
This will always decompress, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644492664
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,225 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644490150
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,225 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644489555
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -116,7 +163,20 @@ public int count() {
*/
@NotNull
String getValue() {
-return value;
+return value != null ? value : decompress(this.compressedValue);
+}
+
+private String decompress(byte[] value) {
+try {
+return new String(compression.getInputStream(new
ByteArrayInputStream(value)).readAllBytes(), StandardCharsets.UTF_8);
+} catch (IOException e) {
+LOG.error("Failed to decompress property {} value: ", getName(),
e);
+return "\"{}\"";
+}
+}
+
+private byte[] getCompressedValue() {
Review Comment:
Done
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,225 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644489190
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -18,26 +18,45 @@
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
import java.util.List;
+import java.util.Objects;
import java.util.Set;
+import org.apache.commons.lang3.RandomStringUtils;
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.commons.Compression;
import org.apache.jackrabbit.oak.spi.blob.BlobStore;
import org.apache.jackrabbit.oak.spi.blob.MemoryBlobStore;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.*;
+import org.junit.runners.MethodSorters;
import static org.apache.jackrabbit.guava.common.collect.Lists.newArrayList;
import static org.apache.jackrabbit.guava.common.collect.Sets.newHashSet;
import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
public class DocumentPropertyStateTest {
private static final int BLOB_SIZE = 16 * 1024;
+public static final int COMPRESSED_SIZE = 543;
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644488604
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
SystemPropertySupplier
+.create("oak.documentMK.stringCompressionThreshold ",
-1).loggingTo(LOG).get();
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
this.store = store;
this.name = name;
-this.value = value;
+if (compression != null && DEFAULT_COMPRESSION_THRESHOLD == -1) {
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644487143
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644486368
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644485936
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
SystemPropertySupplier
+.create("oak.documentMK.stringCompressionThreshold ",
-1).loggingTo(LOG).get();
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644395621
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -128,7 +188,7 @@ public boolean equals(Object object) {
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
return this.name.equals(other.name)
-&& this.value.equals(other.value);
+&& this.getValue().equals(other.getValue());
Review Comment:
Right, I think there should be distinction between non-compressed,
compressed, mixed cases. Only the compressed one is slightly more involved. For
that one we could use a combination of length comparison, hashcode (calculated
when needed) and/or plain Arrays.equals(byte[],byte[])..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644353388
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -128,7 +188,7 @@ public boolean equals(Object object) {
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
return this.name.equals(other.name)
-&& this.value.equals(other.value);
+&& this.getValue().equals(other.getValue());
Review Comment:
We could special-case the compressed case, and compare the byte arrays.
Still expensive.
Maybe we could keep the String's hashCode in order to avoid comparing the
byte arrays in most cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644348052
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -116,7 +163,20 @@ public int count() {
*/
@NotNull
String getValue() {
-return value;
+return value != null ? value : decompress(this.compressedValue);
+}
+
+private String decompress(byte[] value) {
+try {
+return new String(compression.getInputStream(new
ByteArrayInputStream(value)).readAllBytes(), StandardCharsets.UTF_8);
+} catch (IOException e) {
+LOG.error("Failed to decompress property {} value: ", getName(),
e);
+return "\"{}\"";
+}
+}
+
+private byte[] getCompressedValue() {
Review Comment:
Yes. I did the same idea as Stefan. But Julian insisted to make it
private(it was package-protected). Also, this approach complicated many
tests(trying to solve with reflection)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644336103
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -116,7 +163,20 @@ public int count() {
*/
@NotNull
String getValue() {
-return value;
+return value != null ? value : decompress(this.compressedValue);
+}
+
+private String decompress(byte[] value) {
+try {
+return new String(compression.getInputStream(new
ByteArrayInputStream(value)).readAllBytes(), StandardCharsets.UTF_8);
+} catch (IOException e) {
+LOG.error("Failed to decompress property {} value: ", getName(),
e);
+return "\"{}\"";
+}
+}
+
+private byte[] getCompressedValue() {
Review Comment:
ah, now I remember. Your comment wasn't visible in this particular review
anymore, so I didn't connect the dots.
I'm actually a fan of facilitating testing, and that can mean to introduce
methods in productive classes that are there solely for testing purpose. That
method can still be non-public but package-protected for example. That in my
view makes things more explicit than using reflection in tests which is
brittle. Moving this method to the test class I assume implies opening up the
actual variable, which seems less favourable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644328794
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -116,7 +163,20 @@ public int count() {
*/
@NotNull
String getValue() {
-return value;
+return value != null ? value : decompress(this.compressedValue);
+}
+
+private String decompress(byte[] value) {
+try {
+return new String(compression.getInputStream(new
ByteArrayInputStream(value)).readAllBytes(), StandardCharsets.UTF_8);
+} catch (IOException e) {
+LOG.error("Failed to decompress property {} value: ", getName(),
e);
+return "\"{}\"";
+}
+}
+
+private byte[] getCompressedValue() {
Review Comment:
It actually was my proposal not to expose it. Maybe we can just *copy* or
*move* it to the test class?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1644291222
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
SystemPropertySupplier
+.create("oak.documentMK.stringCompressionThreshold ",
-1).loggingTo(LOG).get();
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
this.store = store;
this.name = name;
-this.value = value;
+if (compression != null && DEFAULT_COMPRESSION_THRESHOLD == -1) {
Review Comment:
if `compression` is null, then it would go into `else` and run into a NPE
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+
Re: [PR] Issue/oak 8848 [jackrabbit-oak]
shodaaan commented on code in PR #1474: URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1644246364 ## RELEASE-NOTES.txt: ## @@ -1,4 +1,4 @@ -Release Notes -- Apache Jackrabbit Oak -- Version 1.64.0 +Release Notes -- Apache Jackrabbit Oak -- Version 1.62.0 Review Comment: Fixed it, thank you for the observation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] add service principal support while deleting container in oak-run-commons [jackrabbit-oak]
t-rana opened a new pull request, #1542: URL: https://github.com/apache/jackrabbit-oak/pull/1542 …mons -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10898 - Allow AzureCheck and AzureCompact to be built directly wi… [jackrabbit-oak]
dulceanu merged PR #1540: URL: https://github.com/apache/jackrabbit-oak/pull/1540 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Issue/oak 8848 [jackrabbit-oak]
stefan-egli commented on code in PR #1474: URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1644227441 ## RELEASE-NOTES.txt: ## @@ -1,4 +1,4 @@ -Release Notes -- Apache Jackrabbit Oak -- Version 1.64.0 +Release Notes -- Apache Jackrabbit Oak -- Version 1.62.0 Review Comment: this file shouldn't be part of the PR -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10867: add service principal support in oak-run commons [jackrabbit-oak]
t-rana commented on PR #1513: URL: https://github.com/apache/jackrabbit-oak/pull/1513#issuecomment-2175731261 Closing this PR for now, will raise a new one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10867: add service principal support in oak-run commons [jackrabbit-oak]
t-rana closed pull request #1513: OAK-10867: add service principal support in oak-run commons URL: https://github.com/apache/jackrabbit-oak/pull/1513 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
stefan-egli merged PR #1535: URL: https://github.com/apache/jackrabbit-oak/pull/1535 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10901 - Bypass the DocumentNodeState cache when resolving paths downloaded from Mongo. [jackrabbit-oak]
nfsantos opened a new pull request, #1541: URL: https://github.com/apache/jackrabbit-oak/pull/1541 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10894 - DocumentNodeStore: expose readNode as package private [jackrabbit-oak]
nfsantos merged PR #1533: URL: https://github.com/apache/jackrabbit-oak/pull/1533 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Issues/oak 10781 [jackrabbit-oak]
amit-jain merged PR #1518: URL: https://github.com/apache/jackrabbit-oak/pull/1518 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10898 - Allow AzureCheck and AzureCompact to be built directly wi… [jackrabbit-oak]
dulceanu opened a new pull request, #1540: URL: https://github.com/apache/jackrabbit-oak/pull/1540 …th a CloudBlobDirectory -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Issues/oak 10781 [jackrabbit-oak]
nit0906 commented on code in PR #1518:
URL: https://github.com/apache/jackrabbit-oak/pull/1518#discussion_r1644093746
##
oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobContainerProvider.java:
##
@@ -205,19 +212,33 @@ private CloudBlobContainer
getBlobContainerFromServicePrincipals(@Nullable BlobR
@NotNull
private StorageCredentialsToken getStorageCredentials() {
-ClientSecretCredential clientSecretCredential = new
ClientSecretCredentialBuilder()
-.clientId(clientId)
-.clientSecret(clientSecret)
-.tenantId(tenantId)
-.build();
-AccessToken accessToken = clientSecretCredential.getTokenSync(new
TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE));
-if (accessToken == null ||
StringUtils.isBlank(accessToken.getToken())) {
-log.error("Access token is null or empty");
-throw new IllegalArgumentException("Could not connect to azure
storage, access token is null or empty");
+boolean isAccessTokenGenerated = false;
+/* generate access token, the same token will be used for subsequent
access
+ * generated token is valid for 1 hour only and will be refreshed in
background
+ * */
+if (accessToken == null) {
+clientSecretCredential = new ClientSecretCredentialBuilder()
+.clientId(clientId)
+.clientSecret(clientSecret)
+.tenantId(tenantId)
+.build();
+accessToken = clientSecretCredential.getTokenSync(new
TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE));
+if (accessToken == null ||
StringUtils.isBlank(accessToken.getToken())) {
+log.error("Access token is null or empty");
+throw new IllegalArgumentException("Could not connect to azure
storage, access token is null or empty");
+}
+storageCredentialsToken = new StorageCredentialsToken(accountName,
accessToken.getToken());
+isAccessTokenGenerated = true;
+}
+
+Objects.requireNonNull(storageCredentialsToken, "storage credentials
token cannot be null");
+
+// start refresh token executor only when the access token is first
generated
+if (isAccessTokenGenerated) {
+log.info("starting refresh token task at: {}",
OffsetDateTime.now());
+TokenRefresher tokenRefresher = new TokenRefresher();
+executorService.scheduleWithFixedDelay(tokenRefresher,
TOKEN_REFRESHER_INITIAL_DELAY, TOKEN_REFRESHER_DELAY, TimeUnit.MINUTES);
Review Comment:
Can you add a comment here describing why the 1 minute delay is needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10889 - Indexing job: do not remove field with size estimation from NodeDocuments downloaded from Mongo [jackrabbit-oak]
nfsantos merged PR #1530: URL: https://github.com/apache/jackrabbit-oak/pull/1530 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
nit0906 merged PR #1522: URL: https://github.com/apache/jackrabbit-oak/pull/1522 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
nit0906 commented on code in PR #1522:
URL: https://github.com/apache/jackrabbit-oak/pull/1522#discussion_r1643657775
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,62 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
+log.info("Aborted and paused async indexing for lane [{}]",
name);
+// Release lease for the paused lane
+this.releaseLeaseForPausedLane();
+log.info("Released lease for paused lane [{}]", name);
+String newReferenceCheckpoint = store.checkpoint(lifetime,
Map.of(
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] JCRVLT-757 Introduce filtering extension SPI [jackrabbit-filevault-package-maven-plugin]
kwin opened a new pull request, #105: URL: https://github.com/apache/jackrabbit-filevault-package-maven-plugin/pull/105 Add filtering feature for escaping FileVault DocView XML Attribute values -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Issue/oak 8848 [jackrabbit-oak]
shodaaan commented on code in PR #1474:
URL: https://github.com/apache/jackrabbit-oak/pull/1474#discussion_r1642962427
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/version/VersionEditor.java:
##
@@ -145,27 +146,71 @@ public void propertyChanged(PropertyState before,
PropertyState after)
return;
}
String propName = after.getName();
+
+// Updates the checked-out / checked-in state of the currently
processed node when
+// the JCR_ISCHECKEDOUT property change is processed.
if (propName.equals(JCR_ISCHECKEDOUT)) {
if (wasCheckedIn()) {
vMgr.checkout(node);
} else {
vMgr.checkin(node);
}
} else if (propName.equals(JCR_BASEVERSION)) {
-String baseVersion = after.getValue(Type.REFERENCE);
-if (baseVersion.startsWith(RESTORE_PREFIX)) {
-baseVersion = baseVersion.substring(RESTORE_PREFIX.length());
-node.setProperty(JCR_BASEVERSION, baseVersion, Type.REFERENCE);
+
+// Completes the restore of a version from version history.
+//
+// When the JCR_BASEVERSION property is processed, a check is made
for the current
+// base version property.
+// If a restore is currently in progress for the current base
version (the check for
+// this is that the current base version name has the format
"restore-[UUID of the
+// version to restore to]"), then the restore is completed for the
current node
+// to the version specified by the UUID.
+//
+// If a node that was moved or copied to the location of a deleted
node is currently
+// being processed (see OAK-8848 for context), the restore
operation must NOT be
+// performed when the JCR_BASEVERSION property change is processed
for the node.
+if (!nodeWasMovedOrCopied()) {
+
+String baseVersion = after.getValue(Type.REFERENCE);
+if (baseVersion.startsWith(RESTORE_PREFIX)) {
+baseVersion =
baseVersion.substring(RESTORE_PREFIX.length());
+node.setProperty(JCR_BASEVERSION, baseVersion,
Type.REFERENCE);
+}
+
+vMgr.restore(node, baseVersion, null);
}
-vMgr.restore(node, baseVersion, null);
} else if (isVersionProperty(after)) {
-throwProtected(after.getName());
+// Checks if a version property is being changed and throws a
CommitFailedException
+// with the message "Constraint Violation Exception" if this is
not allowed.
+// JCR_ISCHECKEDOUT and JCR_BASEVERSION properties should be
ignored, since changes
+// to them are allowed for specific use cases (for example,
completing the check-in
+// / check-out for a node or completing a node restore).
+//
+// The only situation when the update of a version property is
allowed is when this
+// occurs as a result of the current node being moved over a
previously deleted node
+// - see OAK-8848 for context.
+//
+// OAK-8848: moving a versionable node in the same location as a
node deleted in the
+// same session should be allowed.
+// This check works because the only way that moving a node in a
location is allowed
+// is if there is no existing (undeleted) node in that location.
+// Property comparison should not fail for two jcr:versionHistory
properties in this case.
+if (!nodeWasMovedOrCopied()) {
+throwProtected(after.getName());
+}
} else if (isReadOnly && getOPV(after) !=
OnParentVersionAction.IGNORE) {
throwCheckedIn("Cannot change property " + after.getName()
+ " on checked in node");
}
}
+/**
+ * Returns true if and only if the given node was moved or copied from
another location.
+ */
+private boolean nodeWasMovedOrCopied() {
Review Comment:
I just tested the copy instead of move case and it can not be done in one
session, because WorkspaceDelegate.copy throws a javax.jcr.ItemExistsException.
Copy was not mentioned in the ticket requirement either, it was a wrong
assumption on my part that copy will behave the same as move in this case.
I will rename the check method to nodeWasMoved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10691: remove use of Guava Charsets class [jackrabbit-oak]
reschke merged PR #1538: URL: https://github.com/apache/jackrabbit-oak/pull/1538 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10897 - Delete unused class: DocumentStoreSplitter [jackrabbit-oak]
nfsantos merged PR #1537: URL: https://github.com/apache/jackrabbit-oak/pull/1537 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642878592
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
[PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan opened a new pull request, #1539: URL: https://github.com/apache/jackrabbit-oak/pull/1539 - create class FullGCOptions which will hold the different parameters used for running full GC - added variables to Configuration.java for 2 full GC modes: GAP_ORPHANS and EMPTY_PROPERTIES - updated VersionGarbageCollector constructor, as well as DocumentNodeStore Builder / Service and DocumentNodeStore to get values received from Configuration via the FullGCOptions class - applied the new full GC mode options in VersionGarbageCollector via the fullGCOptions parameter passed in the constructor - added unit test methods for testing feature toggle and configuration options set in DocumentNodeStoreBuilder, based on existing testing of feature toggles and configuration settings in unit tests - fixed CommitBuilderTest test failures by changing expected exception type -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
stefan-egli commented on PR #1535: URL: https://github.com/apache/jackrabbit-oak/pull/1535#issuecomment-2173430866 ok - just checking why the build fails - restarted it now at https://ci-builds.apache.org/job/Jackrabbit/job/oak-trunk-pr/job/PR-1535/3/ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642813805
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
[PR] OAK-10691: remove use of Guava Charsets class [jackrabbit-oak]
reschke opened a new pull request, #1538: URL: https://github.com/apache/jackrabbit-oak/pull/1538 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10897 - Delete unused class: DocumentStoreSplitter [jackrabbit-oak]
nfsantos opened a new pull request, #1537: URL: https://github.com/apache/jackrabbit-oak/pull/1537 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642766461
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10691: remove use of Guava Charsets class [jackrabbit-oak]
reschke closed pull request #1342: OAK-10691: remove use of Guava Charsets class URL: https://github.com/apache/jackrabbit-oak/pull/1342 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
ionutzpi commented on PR #1535: URL: https://github.com/apache/jackrabbit-oak/pull/1535#issuecomment-2173297991 > @ionutzpi should I go ahead and merge, i.e. is the PR ready? yes, the PR is ready -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642746647
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642740716
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
+SystemPropertySupplier.create("oak.mongo.compressionThreshold",
-1).get();
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
this.store = store;
this.name = name;
-this.value = value;
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.compression = compression;
+int size = value.length();
+String localValue = value;
+byte[] localCompressedValue = null;
+if (compression != null && size > DEFAULT_COMPRESSION_THRESHOLD) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642740716
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
+SystemPropertySupplier.create("oak.mongo.compressionThreshold",
-1).get();
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
this.store = store;
this.name = name;
-this.value = value;
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.compression = compression;
+int size = value.length();
+String localValue = value;
+byte[] localCompressedValue = null;
+if (compression != null && size > DEFAULT_COMPRESSION_THRESHOLD) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642736298
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642735284
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
+SystemPropertySupplier.create("oak.mongo.compressionThreshold",
-1).get();
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
this.store = store;
this.name = name;
-this.value = value;
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.compression = compression;
+int size = value.length();
+String localValue = value;
+byte[] localCompressedValue = null;
+if (compression != null && size > DEFAULT_COMPRESSION_THRESHOLD) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
stefan-egli commented on PR #1535: URL: https://github.com/apache/jackrabbit-oak/pull/1535#issuecomment-2173265117 @ionutzpi should I go ahead and merge, i.e. is the PR ready? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642734832
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
+SystemPropertySupplier.create("oak.mongo.compressionThreshold",
-1).get();
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642734255
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -128,7 +188,7 @@ public boolean equals(Object object) {
} else if (object instanceof DocumentPropertyState) {
DocumentPropertyState other = (DocumentPropertyState) object;
return this.name.equals(other.name)
-&& this.value.equals(other.value);
+&& this.getValue().equals(other.getValue());
Review Comment:
Yes.
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] OAK-10845 : exclude another flaky test combination [jackrabbit-oak]
stefan-egli opened a new pull request, #1536: URL: https://github.com/apache/jackrabbit-oak/pull/1536 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642721700
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenReturn(mockOutputStream);
+
when(mockCompression.getInputStream(any(InputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", STRING_HUGEVALUE,
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING), "{}");
+
+verify(mockCompression,
times(1)).getInputStream(any(InputStream.class));
+
+
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
ionutzpi commented on code in PR #1535:
URL: https://github.com/apache/jackrabbit-oak/pull/1535#discussion_r1642721575
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java:
##
@@ -328,6 +332,7 @@ private void checkUniquenessConstraints() throws
CommitFailedException {
String msg = String.format(
"Uniqueness constraint violated property %s having
value %s",
propertyNames, failed);
+log.warn(msg);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642713859
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
+SystemPropertySupplier.create("oak.mongo.compressionThreshold",
-1).get();
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
this.store = store;
this.name = name;
-this.value = value;
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.compression = compression;
+int size = value.length();
+String localValue = value;
+byte[] localCompressedValue = null;
+if (compression != null && size > DEFAULT_COMPRESSION_THRESHOLD) {
Review Comment:
I think the `compression != null` part could be move to the `if` above?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
stefan-egli commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642708995
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
+SystemPropertySupplier.create("oak.mongo.compressionThreshold",
-1).get();
Review Comment:
the property isn't mongo specific. this isn't handled entirely consistent in
the existing code base neither - perhaps it should be `oak.documentMK.` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
stefan-egli commented on PR #1535: URL: https://github.com/apache/jackrabbit-oak/pull/1535#issuecomment-2173190927 +1 other than the minor comment. PS: there's another case of 0030 in [UniquenessConstraintValidator](https://github.com/apache/jackrabbit-oak/blob/0e327a27eb988a0f7656535275f3735ef8474e5d/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/property/UniquenessConstraintValidator.java#L81) but I'd go with this PropertyIndexEditor as a start. Was also wondering if the fact that it contains the effected paths is a leak of sorts - but given the CommitFailedException is frequently logged anyway, that doesn't seem to be a problem. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10890 - Added log warning [jackrabbit-oak]
stefan-egli commented on code in PR #1535:
URL: https://github.com/apache/jackrabbit-oak/pull/1535#discussion_r1642692392
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java:
##
@@ -328,6 +332,7 @@ private void checkUniquenessConstraints() throws
CommitFailedException {
String msg = String.format(
"Uniqueness constraint violated property %s having
value %s",
propertyNames, failed);
+log.warn(msg);
Review Comment:
I'm wondering if we should add a prefix to the log. Without the prefix it
might be hard for down-stream logging to distinguish between the two sources of
this message (the log.warn and the exception), as they contain a large common
part. What about something like below (or maybe a better variant of it) ?
```suggestion
log.warn("checkUniquenessConstraints: " + msg);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] OAK-10890 - Added log warning [jackrabbit-oak]
ionutzpi opened a new pull request, #1535: URL: https://github.com/apache/jackrabbit-oak/pull/1535 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Oak 10890 -- Logging for constraint violations (UUID already exists) [jackrabbit-oak]
ionutzpi closed pull request #1534: Oak 10890 -- Logging for constraint violations (UUID already exists) URL: https://github.com/apache/jackrabbit-oak/pull/1534 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1642564726
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +45,64 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Review Comment:
I'd set the logger here as well ("logTo()")
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -81,4 +100,179 @@ public void multiValuedBinarySize() throws Exception {
assertEquals(0, reads.size());
}
-}
+@Test
+public void multiValuedAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+List blobs = newArrayList();
+for (int i = 0; i < 13; i++) {
+blobs.add(builder.createBlob(new RandomStream(BLOB_SIZE, i)));
+}
+builder.child(TEST_NODE).setProperty("p", blobs, Type.BINARIES);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.BINARIES, Objects.requireNonNull(p).getType());
+assertEquals(13, p.count());
+
+reads.clear();
+assertEquals(BLOB_SIZE, p.size(0));
+// must not read the blob via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringBelowThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", "dummy", Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(5, p.size(0));
+// must not read the string via stream
+assertEquals(0, reads.size());
+}
+
+@Test
+public void stringAboveThresholdSize() throws Exception {
+NodeBuilder builder = ns.getRoot().builder();
+builder.child(TEST_NODE).setProperty("p", STRING_HUGEVALUE,
Type.STRING);
+TestUtils.merge(ns, builder);
+
+PropertyState p =
ns.getRoot().getChildNode(TEST_NODE).getProperty("p");
+assertEquals(Type.STRING, Objects.requireNonNull(p).getType());
+assertEquals(1, p.count());
+
+reads.clear();
+assertEquals(10050, p.size(0));
+// must not read the string via streams
+assertEquals(0, reads.size());
+}
+
+@Test
+public void compressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+
when(mockCompression.getOutputStream(any(OutputStream.class))).thenThrow(new
IOException("Compression failed"));
+
+Field compressionThreshold =
DocumentPropertyState.class.getDeclaredField("DEFAULT_COMPRESSION_THRESHOLD");
+compressionThreshold.setAccessible(true);
+Field modifiersField = Field.class.getDeclaredField("modifiers");
+modifiersField.setAccessible(true);
+modifiersField.setInt(compressionThreshold,
compressionThreshold.getModifiers() & ~Modifier.FINAL);
+
+compressionThreshold.set(null, DEFAULT_COMPRESSION_THRESHOLD);
+
+DocumentPropertyState documentPropertyState = new
DocumentPropertyState(mockDocumentStore, "p", "\"" + STRING_HUGEVALUE + "\"",
mockCompression);
+
+assertEquals(documentPropertyState.getValue(Type.STRING),
STRING_HUGEVALUE);
+
+verify(mockCompression,
times(1)).getOutputStream(any(OutputStream.class));
+
+compressionThreshold.set(null, -1);
+
+}
+
+@Test
+public void uncompressValueThrowsException() throws IOException,
NoSuchFieldException, IllegalAccessException {
+
+DocumentNodeStore mockDocumentStore = mock(DocumentNodeStore.class);
+Compression mockCompression = mock(Compression.class);
+OutputStream mockOutputStream= mock(OutputStream.class);
+
[PR] Oak 10890 -- Logging for constraint violations (UUID already exists) [jackrabbit-oak]
ionutzpi opened a new pull request, #1534: URL: https://github.com/apache/jackrabbit-oak/pull/1534 Assets sync - ConstraintViolationException: OakConstraint0030: Uniqueness constraint violated property [jcr:uuid] -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10894 - MongoDocumentStore: add a method to retrieve a node bypassing the DocumentNodeState cache. [jackrabbit-oak]
nfsantos opened a new pull request, #1533: URL: https://github.com/apache/jackrabbit-oak/pull/1533 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-9806 - optimised use of the Elastic index [jackrabbit-oak]
github-actions[bot] commented on PR #597: URL: https://github.com/apache/jackrabbit-oak/pull/597#issuecomment-2171007876 This PR is stale because it has been open 24 months with no activity. Remove stale label or comment or this will be closed in 30 days. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] JCRVLT-759 Update to ASF Parent 32 [jackrabbit-filevault]
kwin merged PR #338: URL: https://github.com/apache/jackrabbit-filevault/pull/338 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10892: Update (shaded) Guava to 33.2.1 [jackrabbit-oak]
reschke merged PR #1531: URL: https://github.com/apache/jackrabbit-oak/pull/1531 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10886: Update Oak trunk to Jackrabbit 2.22.0 [jackrabbit-oak]
reschke merged PR #1532: URL: https://github.com/apache/jackrabbit-oak/pull/1532 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10886: Update Oak trunk to Jackrabbit 2.22.0 [jackrabbit-oak]
reschke opened a new pull request, #1532: URL: https://github.com/apache/jackrabbit-oak/pull/1532 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10893: standalone: remove RMI dependency [jackrabbit-oak]
reschke merged PR #1529: URL: https://github.com/apache/jackrabbit-oak/pull/1529 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10892: Update (shaded) Guava to 33.2.1 [jackrabbit-oak]
reschke opened a new pull request, #1531: URL: https://github.com/apache/jackrabbit-oak/pull/1531 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1639713253
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
-this.store = store;
-this.name = name;
-this.value = value;
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.store = store;
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526: URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1639707803 ## oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java: ## @@ -18,26 +18,43 @@ import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.List; +import java.util.Objects; import java.util.Set; +import org.apache.commons.lang3.RandomStringUtils; import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.Compression; import org.apache.jackrabbit.oak.spi.blob.BlobStore; import org.apache.jackrabbit.oak.spi.blob.MemoryBlobStore; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; +import org.junit.*; +import org.junit.runners.MethodSorters; import static org.apache.jackrabbit.guava.common.collect.Lists.newArrayList; import static org.apache.jackrabbit.guava.common.collect.Sets.newHashSet; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; Review Comment: Done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
ionutzpi commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1639707557
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
Review Comment:
Done
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
-this.store = store;
-this.name = name;
-this.value = value;
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.store = store;
+this.name = name;
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.store = store;
+this.name = name;
+this.compression = compression;
+
+int size = value.getBytes().length;
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] OAK-10889 - Indexing job: do not remove field with size estimation from NodeDocuments downloaded from Mongo [jackrabbit-oak]
nfsantos opened a new pull request, #1530: URL: https://github.com/apache/jackrabbit-oak/pull/1530 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1639705992
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
-this.store = store;
-this.name = name;
-this.value = value;
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.store = store;
Review Comment:
things that are the same should be moved out of the condition
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on PR #1526: URL: https://github.com/apache/jackrabbit-oak/pull/1526#issuecomment-2167814526 I believe the main concern is that we don't know yet exactly why we are doing this. IMHO it would be better to first collect data on the size of the properties. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10803 -- compress/uncompress property [jackrabbit-oak]
reschke commented on code in PR #1526:
URL: https://github.com/apache/jackrabbit-oak/pull/1526#discussion_r1639674435
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
-this.store = store;
-this.name = name;
-this.value = value;
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.store = store;
+this.name = name;
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.store = store;
+this.name = name;
+this.compression = compression;
+
+int size = value.getBytes().length;
Review Comment:
That serializes the String into a byte array, obtains the lenght, and then
throws the byte array away.
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
Review Comment:
please use SystemPropertySupplier
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyState.java:
##
@@ -38,24 +44,66 @@
import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.plugins.value.Conversions;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
* PropertyState implementation with lazy parsing of the JSOP encoded value.
*/
final class DocumentPropertyState implements PropertyState {
+private static final Logger LOG =
LoggerFactory.getLogger(DocumentPropertyState.class);
+
private final DocumentNodeStore store;
private final String name;
private final String value;
private PropertyState parsed;
+private final byte[] compressedValue;
+private final Compression compression;
+
+private static final int DEFAULT_COMPRESSION_THRESHOLD =
Integer.getInteger("oak.mongo.compressionThreshold", -1);
DocumentPropertyState(DocumentNodeStore store, String name, String value) {
-this.store = store;
-this.name = name;
-this.value = value;
+this(store, name, value, Compression.GZIP);
+}
+
+DocumentPropertyState(DocumentNodeStore store, String name, String value,
Compression compression) {
+if (DEFAULT_COMPRESSION_THRESHOLD == -1) {
+this.store = store;
+this.name = name;
+this.value = value;
+this.compression = null;
+this.compressedValue = null;
+} else {
+this.store = store;
+this.name = name;
+this.compression = compression;
+
+int size = value.getBytes().length;
Review Comment:
(also it needs to specify the charset)
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentPropertyStateTest.java:
##
@@ -18,26 +18,43 @@
import
[PR] OAK-10893: standalone: remove RMI dependency [jackrabbit-oak]
reschke opened a new pull request, #1529: URL: https://github.com/apache/jackrabbit-oak/pull/1529 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10891: disable flaky test BranchCommitGCTest.testDeletedPropsAndUnmergedBCfor mode ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS [jackrabbit-oak]
Joscorbe merged PR #1528: URL: https://github.com/apache/jackrabbit-oak/pull/1528 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10891: disable flaky test BranchCommitGCTest.testDeletedPropsAndUnmergedBCfor mode ORPHANS_EMPTYPROPS_KEEP_ONE_ALL_PROPS [jackrabbit-oak]
Joscorbe opened a new pull request, #1528: URL: https://github.com/apache/jackrabbit-oak/pull/1528 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Issues/oak 10781 [jackrabbit-oak]
reschke commented on PR #1518: URL: https://github.com/apache/jackrabbit-oak/pull/1518#issuecomment-2167737652 when merging please make sure to squash and to have the JIRA ticket properly in the commit message -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10887: webapp: remove RMI support [jackrabbit-oak]
reschke merged PR #1527: URL: https://github.com/apache/jackrabbit-oak/pull/1527 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10887: webapp: remove RMI support [jackrabbit-oak]
reschke opened a new pull request, #1527: URL: https://github.com/apache/jackrabbit-oak/pull/1527 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
thomasmueller commented on code in PR #1522:
URL: https://github.com/apache/jackrabbit-oak/pull/1522#discussion_r1639552708
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,62 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
Review Comment:
I wouldn't mind if this is just an additional preconditions... instead of
doing it here, just verify it's done. But it's OK, no need to change it now.
(the idea is: We don't want this feature is used a lot, so making it a
little bit harder to use might help that it is used less often... The "CONFIRM"
is a great example of that!... Do not make it easy to delete stuff, so that
it's less likely that stuff is deleted by mistake.)
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,62 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
Review Comment:
In theory (not now, but maybe later) we could add an additional check: only
allow if the indexing lane is behind more than 30 minutes, or something like
that.
But again, not needed right now.
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,62 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
+log.info("Aborted and paused async indexing for lane [{}]",
name);
+// Release lease for the paused lane
+this.releaseLeaseForPausedLane();
+log.info("Released lease for paused lane [{}]", name);
+String newReferenceCheckpoint = store.checkpoint(lifetime,
Map.of(
Review Comment:
Ah, DEFAULT_LIFETIME is 1000 days! could you change that to 100 days please?
That should still be plenty, but (hopefully) will result in slightly less
"orphaned checkpoints".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
nit0906 commented on code in PR #1522:
URL: https://github.com/apache/jackrabbit-oak/pull/1522#discussion_r1639441493
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,61 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
+log.info("Aborted and paused async indexing for lane [{}]",
name);
+// Release lease for the paused lane
+this.releaseLeaseForPausedLane();
+log.info("Released lease for paused lane [{}]", name);
+String newReferenceCheckpoint = store.checkpoint(lifetime,
Map.of(
+"creator", AsyncIndexUpdate.class.getSimpleName(),
+"created", now(),
+"thread", Thread.currentThread().getName(),
+"name", name + "-forceModified"));
+String existingReferenceCheckpoint = this.referenceCp;
+log.info("Modifying the referred checkpoint for lane [{}] from
{} to {}." +
+" This means that any content modifications between
these checkpoints will not reflect in the indexes on this lane." +
+" Reindexing is needed to get this content indexed.",
name, existingReferenceCheckpoint, newReferenceCheckpoint);
+NodeBuilder builder = store.getRoot().builder();
+builder.child(ASYNC).setProperty(name, newReferenceCheckpoint);
+this.referenceCp = newReferenceCheckpoint;
+mergeWithConcurrencyCheck(store, validatorProviders, builder,
existingReferenceCheckpoint, null, name);
+// Remove the existing reference checkpoint
+if (store.release(existingReferenceCheckpoint)) {
+log.info("Old reference checkpoint {} removed or didn't
exist", existingReferenceCheckpoint);
+} else {
+log.warn("Unable to remove old reference checkpoint {}.
This can result in orphaned checkpoints and would need to be removed
manually.", existingReferenceCheckpoint);
+}
+// Resume the paused lane;
+this.resume();
+log.info("Resumed async indexing for lane [{}]", name);
+return "Lane successfully forced to catch-up. New reference
checkpoint is " + newReferenceCheckpoint + " . Please make sure to perform
reindexing to get the diff content indexed.";
+} catch (Exception e) {
+log.error("Exception while trying to force update the indexing
lane [{}]", name, e);
+if (this.isPaused()) {
+log.info("Resuming the lane [{}] as it was paused during
the operation", name);
Review Comment:
good catch. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
nfsantos commented on code in PR #1522:
URL: https://github.com/apache/jackrabbit-oak/pull/1522#discussion_r1639428734
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,61 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
+log.info("Aborted and paused async indexing for lane [{}]",
name);
+// Release lease for the paused lane
+this.releaseLeaseForPausedLane();
+log.info("Released lease for paused lane [{}]", name);
+String newReferenceCheckpoint = store.checkpoint(lifetime,
Map.of(
+"creator", AsyncIndexUpdate.class.getSimpleName(),
+"created", now(),
+"thread", Thread.currentThread().getName(),
+"name", name + "-forceModified"));
+String existingReferenceCheckpoint = this.referenceCp;
+log.info("Modifying the referred checkpoint for lane [{}] from
{} to {}." +
+" This means that any content modifications between
these checkpoints will not reflect in the indexes on this lane." +
+" Reindexing is needed to get this content indexed.",
name, existingReferenceCheckpoint, newReferenceCheckpoint);
+NodeBuilder builder = store.getRoot().builder();
+builder.child(ASYNC).setProperty(name, newReferenceCheckpoint);
+this.referenceCp = newReferenceCheckpoint;
+mergeWithConcurrencyCheck(store, validatorProviders, builder,
existingReferenceCheckpoint, null, name);
+// Remove the existing reference checkpoint
+if (store.release(existingReferenceCheckpoint)) {
+log.info("Old reference checkpoint {} removed or didn't
exist", existingReferenceCheckpoint);
+} else {
+log.warn("Unable to remove old reference checkpoint {}.
This can result in orphaned checkpoints and would need to be removed
manually.", existingReferenceCheckpoint);
+}
+// Resume the paused lane;
+this.resume();
+log.info("Resumed async indexing for lane [{}]", name);
+return "Lane successfully forced to catch-up. New reference
checkpoint is " + newReferenceCheckpoint + " . Please make sure to perform
reindexing to get the diff content indexed.";
+} catch (Exception e) {
+log.error("Exception while trying to force update the indexing
lane [{}]", name, e);
+if (this.isPaused()) {
+log.info("Resuming the lane [{}] as it was paused during
the operation", name);
Review Comment:
Should there be a call to resume here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
nfsantos commented on code in PR #1522:
URL: https://github.com/apache/jackrabbit-oak/pull/1522#discussion_r1639426483
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,61 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
+log.info("Aborted and paused async indexing for lane [{}]",
name);
+// Release lease for the paused lane
+this.releaseLeaseForPausedLane();
+log.info("Released lease for paused lane [{}]", name);
+String newReferenceCheckpoint = store.checkpoint(lifetime,
Map.of(
+"creator", AsyncIndexUpdate.class.getSimpleName(),
+"created", now(),
+"thread", Thread.currentThread().getName(),
+"name", name + "-forceModified"));
+String existingReferenceCheckpoint = this.referenceCp;
+log.info("Modifying the referred checkpoint for lane [{}] from
{} to {}." +
+" This means that any content modifications between
these checkpoints will not reflect in the indexes on this lane." +
+" Reindexing is needed to get this content indexed.",
name, existingReferenceCheckpoint, newReferenceCheckpoint);
+NodeBuilder builder = store.getRoot().builder();
+builder.child(ASYNC).setProperty(name, newReferenceCheckpoint);
+this.referenceCp = newReferenceCheckpoint;
+mergeWithConcurrencyCheck(store, validatorProviders, builder,
existingReferenceCheckpoint, null, name);
+// Remove the existing reference checkpoint
+if (store.release(existingReferenceCheckpoint)) {
+log.info("Old reference checkpoint {} removed or didn't
exist", existingReferenceCheckpoint);
+} else {
+log.warn("Unable to remove old reference checkpoint {}.
This can result in orphaned checkpoints and would need to be removed
manually.", existingReferenceCheckpoint);
+}
+// Resume the paused lane;
+this.resume();
+log.info("Resumed async indexing for lane [{}]", name);
+return "Lane successfully forced to catch-up. New reference
checkpoint is " + newReferenceCheckpoint + " . Please make sure to perform
reindexing to get the diff content indexed.";
+} catch (Exception e) {
+log.error("Exception while trying to force update the indexing
lane [{}]", name, e);
+if (this.isPaused()) {
+log.info("Resuming the lane [{}] as it was paused during
the operation", name);
+}
+return "Unable to complete the force update due to" +
e.getMessage() + "Please check logs for more details";
Review Comment:
```suggestion
return "Unable to complete the force update due to " +
e.getMessage() + ". Please check logs for more details";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Oak-10874 | Add jmx function for bringing forward a delayed async lane to a latest checkpoint. [jackrabbit-oak]
nfsantos commented on code in PR #1522:
URL: https://github.com/apache/jackrabbit-oak/pull/1522#discussion_r1639426483
##
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/AsyncIndexUpdate.java:
##
@@ -1242,6 +1242,61 @@ public String getReferenceCheckpoint() {
return referenceCp;
}
+@Override
+public String forceIndexLaneCatchup(String confirmMessage) throws
CommitFailedException {
+
+if (!"CONFIRM".equals(confirmMessage)) {
+String msg = "Please confirm that you want to force the lane
catch-up by passing 'CONFIRM' as argument";
+log.warn(msg);
+return msg;
+}
+
+if (!this.isFailing()) {
+String msg = "The lane is not failing. This operation should
only be performed if the lane is failing, it should first be allowed to catch
up on its own.";
+log.warn(msg);
+return msg;
+}
+
+try {
+log.info("Running a forced catch-up for indexing lane [{}]. ",
name);
+// First we need to abort and pause the running indexing task
+this.abortAndPause();
+log.info("Aborted and paused async indexing for lane [{}]",
name);
+// Release lease for the paused lane
+this.releaseLeaseForPausedLane();
+log.info("Released lease for paused lane [{}]", name);
+String newReferenceCheckpoint = store.checkpoint(lifetime,
Map.of(
+"creator", AsyncIndexUpdate.class.getSimpleName(),
+"created", now(),
+"thread", Thread.currentThread().getName(),
+"name", name + "-forceModified"));
+String existingReferenceCheckpoint = this.referenceCp;
+log.info("Modifying the referred checkpoint for lane [{}] from
{} to {}." +
+" This means that any content modifications between
these checkpoints will not reflect in the indexes on this lane." +
+" Reindexing is needed to get this content indexed.",
name, existingReferenceCheckpoint, newReferenceCheckpoint);
+NodeBuilder builder = store.getRoot().builder();
+builder.child(ASYNC).setProperty(name, newReferenceCheckpoint);
+this.referenceCp = newReferenceCheckpoint;
+mergeWithConcurrencyCheck(store, validatorProviders, builder,
existingReferenceCheckpoint, null, name);
+// Remove the existing reference checkpoint
+if (store.release(existingReferenceCheckpoint)) {
+log.info("Old reference checkpoint {} removed or didn't
exist", existingReferenceCheckpoint);
+} else {
+log.warn("Unable to remove old reference checkpoint {}.
This can result in orphaned checkpoints and would need to be removed
manually.", existingReferenceCheckpoint);
+}
+// Resume the paused lane;
+this.resume();
+log.info("Resumed async indexing for lane [{}]", name);
+return "Lane successfully forced to catch-up. New reference
checkpoint is " + newReferenceCheckpoint + " . Please make sure to perform
reindexing to get the diff content indexed.";
+} catch (Exception e) {
+log.error("Exception while trying to force update the indexing
lane [{}]", name, e);
+if (this.isPaused()) {
+log.info("Resuming the lane [{}] as it was paused during
the operation", name);
+}
+return "Unable to complete the force update due to" +
e.getMessage() + "Please check logs for more details";
Review Comment:
```suggestion
return "Unable to complete the force update due to" +
e.getMessage() + ". Please check logs for more details";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
