[GitHub] [jackrabbit-oak] rishabhdaim commented on pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on PR #690: URL: https://github.com/apache/jackrabbit-oak/pull/690#issuecomment-1249557733 I think we discussed on these lines earlier to create a separate `StatsCollector` for throttling operation but It would essentially mean to duplicate the stats collection logic and its related APIs at 2 different places i.e. `DocumentStoreStatsCollector` & `ThrottlingStatsCollector`. The only thing we achieve from this is having 2 different `StatsCollectors` on basis of whether throttling is enabled or not. If we look closely, then these 2 classes are essentially the same barring the method to calculate the time taken to complete the operation and the actual metrics to store the stats (both of which can be parameterized). IMHO, the exiting APIs suit perfectly well for throttling after adding the time taken logic and throttling metrics. -- 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
[GitHub] [jackrabbit-oak] mreutegg merged pull request #683: OAK-9918: DocumentNodeStore consistency check
mreutegg merged PR #683: URL: https://github.com/apache/jackrabbit-oak/pull/683 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #683: OAK-9918: DocumentNodeStore consistency check
mreutegg commented on code in PR #683:
URL: https://github.com/apache/jackrabbit-oak/pull/683#discussion_r972963822
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/OrphanedNodeCheck.java:
##
@@ -0,0 +1,159 @@
+/*
+ * 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.plugins.document.check;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.apache.jackrabbit.oak.plugins.document.Path;
+import org.apache.jackrabbit.oak.plugins.document.RevisionVector;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * OrphanedNodeCheck...
+ */
+public class OrphanedNodeCheck implements DocumentProcessor, Closeable {
+
+private final DocumentNodeStore ns;
+
+private final RevisionVector headRevision;
+
+private final ExecutorService executorService;
+
+public OrphanedNodeCheck(DocumentNodeStore ns,
+ RevisionVector headRevision,
+ int numThread) {
+this.ns = ns;
+this.headRevision = headRevision;
+this.executorService = new ThreadPoolExecutor(
+numThread, numThread, 1, TimeUnit.MINUTES,
+new LinkedBlockingQueue<>(1000),
+new ThreadPoolExecutor.CallerRunsPolicy()
+);
+}
+
+@Override
+public void processDocument(@NotNull NodeDocument document,
+@NotNull BlockingQueue results) {
+if (!document.isSplitDocument()) {
+executorService.submit(new CheckDocument(ns, headRevision,
document, results));
+}
+}
+
+@Override
+public void end(@NotNull BlockingQueue results)
+throws InterruptedException {
+executorService.shutdown();
+if (!executorService.awaitTermination(5, TimeUnit.MINUTES)) {
+String msg = "Checks still not finished after the last one has
been submitted 5 minutes ago";
+results.put(() -> {
+StringBuilder sb = new StringBuilder("{");
+sb.append("\"time\": \"").append(nowAsISO8601()).append('"');
+sb.append(", \"info\": \"").append(msg).append('"');
+sb.append("}");
+return sb.toString();
+});
+}
+}
+
+@Override
+public void close() throws IOException {
+new ExecutorCloser(executorService, 5, TimeUnit.MINUTES).close();
+}
+
+private static final class CheckDocument implements Callable {
+
+private final DocumentNodeStore ns;
+
+private final RevisionVector headRevision;
+
+private final NodeDocument doc;
+
+private final BlockingQueue results;
+
+CheckDocument(DocumentNodeStore ns,
+ RevisionVector headRevision,
+ NodeDocument doc,
+ BlockingQueue results) {
+this.ns = ns;
+this.headRevision = headRevision;
+this.doc = doc;
+this.results = results;
+}
+
+@Override
+public Void call() throws Exception {
Review Comment:
See my previous 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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #683: OAK-9918: DocumentNodeStore consistency check
mreutegg commented on code in PR #683:
URL: https://github.com/apache/jackrabbit-oak/pull/683#discussion_r972963172
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/OrphanedNodeCheck.java:
##
@@ -0,0 +1,159 @@
+/*
+ * 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.plugins.document.check;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.apache.jackrabbit.oak.plugins.document.Path;
+import org.apache.jackrabbit.oak.plugins.document.RevisionVector;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * OrphanedNodeCheck...
+ */
+public class OrphanedNodeCheck implements DocumentProcessor, Closeable {
+
+private final DocumentNodeStore ns;
+
+private final RevisionVector headRevision;
+
+private final ExecutorService executorService;
+
+public OrphanedNodeCheck(DocumentNodeStore ns,
+ RevisionVector headRevision,
+ int numThread) {
+this.ns = ns;
+this.headRevision = headRevision;
+this.executorService = new ThreadPoolExecutor(
+numThread, numThread, 1, TimeUnit.MINUTES,
+new LinkedBlockingQueue<>(1000),
+new ThreadPoolExecutor.CallerRunsPolicy()
+);
+}
+
+@Override
+public void processDocument(@NotNull NodeDocument document,
+@NotNull BlockingQueue results) {
+if (!document.isSplitDocument()) {
+executorService.submit(new CheckDocument(ns, headRevision,
document, results));
+}
+}
+
+@Override
+public void end(@NotNull BlockingQueue results)
+throws InterruptedException {
+executorService.shutdown();
+if (!executorService.awaitTermination(5, TimeUnit.MINUTES)) {
+String msg = "Checks still not finished after the last one has
been submitted 5 minutes ago";
+results.put(() -> {
+StringBuilder sb = new StringBuilder("{");
+sb.append("\"time\": \"").append(nowAsISO8601()).append('"');
+sb.append(", \"info\": \"").append(msg).append('"');
+sb.append("}");
+return sb.toString();
+});
+}
+}
+
+@Override
+public void close() throws IOException {
+new ExecutorCloser(executorService, 5, TimeUnit.MINUTES).close();
+}
+
+private static final class CheckDocument implements Callable {
+
+private final DocumentNodeStore ns;
+
+private final RevisionVector headRevision;
+
+private final NodeDocument doc;
+
+private final BlockingQueue results;
+
+CheckDocument(DocumentNodeStore ns,
+ RevisionVector headRevision,
+ NodeDocument doc,
+ BlockingQueue results) {
+this.ns = ns;
+this.headRevision = headRevision;
+this.doc = doc;
+this.results = results;
+}
+
+@Override
+public Void call() throws Exception {
+DocumentNodeState state = doc.getNodeAtRevision(ns, headRevision,
null);
+if (state != null) {
+Path path = doc.getPath();
+Path missing = assertAncestorsExist(path.getParent());
+if (missing != null) {
+results.put(new OrphanedNode(path, missing,
state.getLastRevision()));
+}
+}
+return null;
+}
+
+private Path assertAncestorsExist(Path path) {
Review Comment:
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #683: OAK-9918: DocumentNodeStore consistency check
mreutegg commented on code in PR #683:
URL: https://github.com/apache/jackrabbit-oak/pull/683#discussion_r972959861
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/OrphanedNodeCheck.java:
##
@@ -0,0 +1,159 @@
+/*
+ * 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.plugins.document.check;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.apache.jackrabbit.oak.plugins.document.Path;
+import org.apache.jackrabbit.oak.plugins.document.RevisionVector;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * OrphanedNodeCheck...
+ */
+public class OrphanedNodeCheck implements DocumentProcessor, Closeable {
+
+private final DocumentNodeStore ns;
+
+private final RevisionVector headRevision;
+
+private final ExecutorService executorService;
+
+public OrphanedNodeCheck(DocumentNodeStore ns,
+ RevisionVector headRevision,
+ int numThread) {
+this.ns = ns;
+this.headRevision = headRevision;
+this.executorService = new ThreadPoolExecutor(
+numThread, numThread, 1, TimeUnit.MINUTES,
+new LinkedBlockingQueue<>(1000),
+new ThreadPoolExecutor.CallerRunsPolicy()
+);
+}
+
+@Override
+public void processDocument(@NotNull NodeDocument document,
+@NotNull BlockingQueue results) {
+if (!document.isSplitDocument()) {
+executorService.submit(new CheckDocument(ns, headRevision,
document, results));
+}
+}
+
+@Override
+public void end(@NotNull BlockingQueue results)
+throws InterruptedException {
+executorService.shutdown();
+if (!executorService.awaitTermination(5, TimeUnit.MINUTES)) {
+String msg = "Checks still not finished after the last one has
been submitted 5 minutes ago";
+results.put(() -> {
+StringBuilder sb = new StringBuilder("{");
+sb.append("\"time\": \"").append(nowAsISO8601()).append('"');
+sb.append(", \"info\": \"").append(msg).append('"');
+sb.append("}");
+return sb.toString();
+});
+}
+}
+
+@Override
+public void close() throws IOException {
+new ExecutorCloser(executorService, 5, TimeUnit.MINUTES).close();
+}
+
+private static final class CheckDocument implements Callable {
Review Comment:
Unfortunately that's not quite true. The task implementation puts a `Result`
in the `results` queue, which may throw an `InterruptedException`. I'll leave
this as is.
--
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
[GitHub] [jackrabbit-oak] mreutegg closed pull request #226: OAK-9082 Remove unnecessary (un)boxing in oak-store-composite
mreutegg closed pull request #226: OAK-9082 Remove unnecessary (un)boxing in oak-store-composite URL: https://github.com/apache/jackrabbit-oak/pull/226 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on pull request #226: OAK-9082 Remove unnecessary (un)boxing in oak-store-composite
mreutegg commented on PR #226: URL: https://github.com/apache/jackrabbit-oak/pull/226#issuecomment-1249102483 Closing this PR because the proposed changes are unrelated to boxing (`Long.valueOf()` vs `Long.parseLong()`). -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on pull request #227: OAK-9083 Remove unnecessary (un)boxing in oak-store-document
mreutegg commented on PR #227: URL: https://github.com/apache/jackrabbit-oak/pull/227#issuecomment-1249098465 Closing this PR because impact on performance is questionable. See also discussion in OAK-9083. -- 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
[GitHub] [jackrabbit-oak] mreutegg closed pull request #227: OAK-9083 Remove unnecessary (un)boxing in oak-store-document
mreutegg closed pull request #227: OAK-9083 Remove unnecessary (un)boxing in oak-store-document URL: https://github.com/apache/jackrabbit-oak/pull/227 -- 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
[GitHub] [jackrabbit-oak] mreutegg merged pull request #228: OAK-9084 Remove unnecessary (un)boxing in oak-store-spi
mreutegg merged PR #228: URL: https://github.com/apache/jackrabbit-oak/pull/228 -- 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
[GitHub] [jackrabbit-oak] mreutegg merged pull request #229: OAK-9085 Remove unnecessary (un)boxing in oak-webapp
mreutegg merged PR #229: URL: https://github.com/apache/jackrabbit-oak/pull/229 -- 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
[GitHub] [jackrabbit-oak] mreutegg closed pull request #237: s3Connector changes for regions apart from aws default regions
mreutegg closed pull request #237: s3Connector changes for regions apart from aws default regions URL: https://github.com/apache/jackrabbit-oak/pull/237 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on pull request #235: s3Connector changes for regions apart from aws default regions
mreutegg commented on PR #235: URL: https://github.com/apache/jackrabbit-oak/pull/235#issuecomment-1249071511 Change has been committed to SVN: http://svn.apache.org/viewvc?rev=1879352&view=rev -- 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
[GitHub] [jackrabbit-oak] mreutegg closed pull request #235: s3Connector changes for regions apart from aws default regions
mreutegg closed pull request #235: s3Connector changes for regions apart from aws default regions URL: https://github.com/apache/jackrabbit-oak/pull/235 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on pull request #237: s3Connector changes for regions apart from aws default regions
mreutegg commented on PR #237: URL: https://github.com/apache/jackrabbit-oak/pull/237#issuecomment-1249071913 Change has been committed to SVN: http://svn.apache.org/viewvc?rev=1879390&view=rev -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on pull request #694: Bump postgresql from 42.2.18 to 42.4.1 in /oak-parent
mreutegg commented on PR #694: URL: https://github.com/apache/jackrabbit-oak/pull/694#issuecomment-1249064559 @dependabot rebase -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on PR #690: URL: https://github.com/apache/jackrabbit-oak/pull/690#issuecomment-1248989611 I only noticed now that Stefan also [mentioned](https://issues.apache.org/jira/browse/OAK-9913?focusedCommentId=17598459&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17598459) he would keep the two metrics separate. The existing one that gives us timings with the DocumentStore implementation and the throttling metrics. There is also good [comment](https://issues.apache.org/jira/browse/OAK-9913?focusedCommentId=17600332&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17600332) from Jose in the JIRA issue. This is a bit late in the review and I feel bad that I didn't point this out earlier, but wouldn't it be easier and more decoupled if there was a `ThrottlingStatsCollector`? It would be similar to `DocumentStoreStatsCollector`, but without methods for read operations. The `DocumentNodeStoreBuilder` would provide a `ThrottlingStatsCollector` that can be used in the constructor of `DocumentNodeStore` when a `ThrottlingDocumentStoreWrapper` created. The `ThrottlingStatsCollector` is passed as an argument to `ThrottlingDocumentStoreWrapper`. Whenever the `ThrottlingDocumentStoreWrapper` throttles an operation it reports it to the `ThrottlingStatsCollector`. As mentioned earlier, the existing metrics for write operations would be untouched and decoupled from the new throttling metrics. -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r972666014
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStats.java:
##
@@ -157,6 +203,41 @@ public DocumentStoreStats(StatisticsProvider provider) {
prefetchNodes = provider.getMeter(NODES_PREFETCH,
StatsOptions.DEFAULT);
prefetchNodesTimer = provider.getTimer(NODES_PREFETCH_TIMER,
StatsOptions.METRICS_ONLY);
+
+// metrics for throttling
+MeterStats createSplitNodeWithThrottlingMeter =
provider.getMeter(NODES_CREATE_SPLIT_WITH_THROTTLING, StatsOptions.DEFAULT);
Review Comment:
Yes, I would change the naming.
--
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
[GitHub] [jackrabbit-oak] mreutegg merged pull request #688: OAK-9891: Removal (purge) of version of a node does not remove associated labels
mreutegg merged PR #688: URL: https://github.com/apache/jackrabbit-oak/pull/688 -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #702: OAK-9535: Support recovery of large branch merge - backport to 1.8
rishabhdaim commented on code in PR #702:
URL: https://github.com/apache/jackrabbit-oak/pull/702#discussion_r972120239
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RecoveryTest.java:
##
@@ -157,6 +169,69 @@ public void recoverOther() throws Exception {
assertThat(diff.deleted, containsInAnyOrder("/parent/test/c1"));
}
+@Test
+@Ignore(value = "Causes OOM on travis and local as well for 1.8 branch")
Review Comment:
We assign all (1.8, 1.22 & trunk) the same memory while running test
cases[1]. I even tried updating the `-Xmx` size to `1 GB`, but it didn't work
for `1.8` on my local.
We might have some other optimizations/changes that are helping this test
pass on `1.22` & `trunk`.
[1]
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-parent/pom.xml#L39
--
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
[GitHub] [jackrabbit-oak] anchela merged pull request #669: OAK-9902 : Configuration for ExternalUserValidator
anchela merged PR #669: URL: https://github.com/apache/jackrabbit-oak/pull/669 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #702: OAK-9535: Support recovery of large branch merge - backport to 1.8
mreutegg commented on code in PR #702:
URL: https://github.com/apache/jackrabbit-oak/pull/702#discussion_r971858137
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RecoveryTest.java:
##
@@ -157,6 +169,69 @@ public void recoverOther() throws Exception {
assertThat(diff.deleted, containsInAnyOrder("/parent/test/c1"));
}
+@Test
+@Ignore(value = "Causes OOM on travis and local as well for 1.8 branch")
Review Comment:
Why does this work on trunk and 1.22 branch but not on 1.8? Does it get less
memory on 1.8 or need more?
--
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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #702: OAK-9934: Backport of Oak-9535 "Support recovery of large branch merge" to 1.8
mreutegg commented on code in PR #702:
URL: https://github.com/apache/jackrabbit-oak/pull/702#discussion_r971762034
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranch.java:
##
@@ -130,6 +130,15 @@ public NodeState merge(@NotNull CommitHook hook, @NotNull
CommitInfo info)
return merge0(hook, info, true);
}
+/**
+ * For test purposes only!
+ *
+ * Forces the branch to persist the changes to the underlying store.
+ */
+void persist() {
+branchState.persist();
+}
+
Review Comment:
Can you please move this method further down and place it between
`getMergeLock()` and `merge0()`? This matches the location in trunk and reduces
potential conflicts in the future when further changes a ported back.
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/TestUtils.java:
##
@@ -93,6 +93,14 @@ public static DocumentNodeState asDocumentState(NodeState
state){
return null;
}
+public static void persistToBranch(NodeBuilder builder) {
+if (builder instanceof DocumentRootBuilder) {
+((DocumentRootBuilder) builder).persist();
+return;
+}
+fail("Not of type DocumentRootBuilder: " +
builder.getClass().getName());
+}
+
Review Comment:
Similar as above, can you please move this method between
`resetRevisionClockToDefault()` and `disposeQuietly()`?
--
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
[GitHub] [jackrabbit-oak] mreutegg merged pull request #701: OAK-9535: Support recovery of large branch merge - backport to 1.22
mreutegg merged PR #701: URL: https://github.com/apache/jackrabbit-oak/pull/701 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #706: OAK-9938: Fix initialization of BlobStore in GenerateVersionInconsistencyReport
mreutegg commented on code in PR #706:
URL: https://github.com/apache/jackrabbit-oak/pull/706#discussion_r971721247
##
oak-run/src/main/java/org/apache/jackrabbit/oak/run/GenerateVersionInconsistencyReport.java:
##
@@ -95,6 +95,8 @@ private void generateReport(String adminUser, String
adminPwd, String...args) th
DocumentNodeStore dns = null;
try {
DocumentNodeStoreBuilder builder =
Utils.createDocumentMKBuilder(args, closer, h);
+// Getting blobstore to trigger it to use MemoryBlobStore
+builder.getBlobStore();
Review Comment:
I think it would be better to explicitly set a MemoryBlobStore. But I'm also
fine with this change.
--
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
[GitHub] [jackrabbit-oak] smiroslav merged pull request #704: OAK-9936 Optimize AzureJournalFile.AzureJournalWriter#batchWriteLines
smiroslav merged PR #704: URL: https://github.com/apache/jackrabbit-oak/pull/704 -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r970974784
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStats.java:
##
@@ -157,6 +203,41 @@ public DocumentStoreStats(StatisticsProvider provider) {
prefetchNodes = provider.getMeter(NODES_PREFETCH,
StatsOptions.DEFAULT);
prefetchNodesTimer = provider.getTimer(NODES_PREFETCH_TIMER,
StatsOptions.METRICS_ONLY);
+
+// metrics for throttling
+MeterStats createSplitNodeWithThrottlingMeter =
provider.getMeter(NODES_CREATE_SPLIT_WITH_THROTTLING, StatsOptions.DEFAULT);
Review Comment:
Do you want me to update the names of all metrics with throttling?
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r970968969
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateStatsConsumer.java:
##
@@ -0,0 +1,31 @@
+/*
+ * 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.plugins.document.util;
+
+import java.util.List;
+
+/**
+ * {@link FunctionalInterface} to consume Metric Stats for create/upsert
operation
+ * @param the type of the first argument to the consumer
+ * @param the type of the second argument to the consumer
+ * @param the type of the third argument to the consumer
+ */
+public interface CreateStatsConsumer {
+void accept(T t, U u, V v, List ids, long tTN);
Review Comment:
Sure, will make the change.
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r970967351
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateMetricUpdater.java:
##
@@ -0,0 +1,99 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.List;
+import java.util.function.BiPredicate;
+import java.util.function.Predicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneCreate(long, Collection, List, boolean)} for
underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class CreateMetricUpdater {
+
+private final MeterStats createNodeMeter;
+private final MeterStats createSplitNodeMeter;
+private final TimerStats createNodeTimer;
+private final MeterStats createJournal;
+private final TimerStats createJournalTimer;
+
+public CreateMetricUpdater(final MeterStats createNodeMeter,
+ final MeterStats createSplitNodeMeter,
+ final TimerStats createNodeTimer,
+ final MeterStats createJournal,
+ final TimerStats createJournalTimer) {
+this.createNodeMeter = createNodeMeter;
+this.createSplitNodeMeter = createSplitNodeMeter;
+this.createNodeTimer = createNodeTimer;
+this.createJournal = createJournal;
+this.createJournalTimer = createJournalTimer;
+}
+
+public void update(final Collection collection, final
long timeTakenNanos,
+ final List ids, final boolean insertSuccess,
+ final BiPredicate,
Integer> isNodesCollectionUpdated,
+ final CreateStatsConsumer createStatsConsumer,
+ final Predicate>
isJournalCollection,
+ final StatsConsumer
journalStatsConsumer) {
+
+requireNonNull(isNodesCollectionUpdated);
+requireNonNull(isJournalCollection);
+requireNonNull(createStatsConsumer);
+requireNonNull(journalStatsConsumer);
+
+if (isNodesCollectionUpdated.test(collection, ids.size()) &&
insertSuccess) {
+createStatsConsumer.accept(createNodeMeter, createSplitNodeMeter,
createNodeTimer, ids, timeTakenNanos);
+} else if (isJournalCollection.test(collection)) {
+journalStatsConsumer.accept(createJournal, createJournalTimer,
ids.size(), timeTakenNanos);
+}
+}
+
+public static class CreateMetricUpdaterWithoutThrottling extends
CreateMetricUpdater {
Review Comment:
I get your point, will update 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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #688: Removal (purge) of version of a node does not remove associated labels
mreutegg commented on code in PR #688:
URL: https://github.com/apache/jackrabbit-oak/pull/688#discussion_r970721813
##
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
assertFalse("VersionHistory node should have disappeared",
superuser.itemExists(vhrpath));
}
+
+public void testRemoveVersionLabelWithRemovalOfVersion() throws
RepositoryException{
+int createVersions = 3;
+Node n = testRootNode.addNode(nodeName1, testNodeType);
+n.addMixin(mixVersionable);
+superuser.save();
+
+VersionManager vm = superuser.getWorkspace().getVersionManager();
+for (int i = 0; i < createVersions; i++) {
+vm.checkout(n.getPath());
+vm.checkin(n.getPath());
+}
+superuser.save();
Review Comment:
This is unnecessary. Checkin and checkout are workspace operations and do
not need to be saved.
##
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
assertFalse("VersionHistory node should have disappeared",
superuser.itemExists(vhrpath));
}
+
+public void testRemoveVersionLabelWithRemovalOfVersion() throws
RepositoryException{
+int createVersions = 3;
+Node n = testRootNode.addNode(nodeName1, testNodeType);
+n.addMixin(mixVersionable);
+superuser.save();
+
+VersionManager vm = superuser.getWorkspace().getVersionManager();
Review Comment:
There is already a `VersionManager` available as an instance variable
`versionManager`.
##
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
assertFalse("VersionHistory node should have disappeared",
superuser.itemExists(vhrpath));
}
+
+public void testRemoveVersionLabelWithRemovalOfVersion() throws
RepositoryException{
+int createVersions = 3;
+Node n = testRootNode.addNode(nodeName1, testNodeType);
+n.addMixin(mixVersionable);
+superuser.save();
+
+VersionManager vm = superuser.getWorkspace().getVersionManager();
+for (int i = 0; i < createVersions; i++) {
+vm.checkout(n.getPath());
+vm.checkin(n.getPath());
+}
+superuser.save();
+
+VersionHistory vhr = vm.getVersionHistory(n.getPath());
+// initialize versionName
+String versionName = "";
+VersionIterator allversions = vhr.getAllVersions();
+int count = 0;
+while (allversions.hasNext()) {
+Version version = allversions.nextVersion();
+if(count == 1) {
+versionName = version.getName();
+}
+count++;
+}
+int versonLabelCount = 3;
+List versionLabels = new ArrayList<>();
+for(int i = 0; i < versonLabelCount; i++) {
+String labelName = "Label_" + versionName + "_" + i;
+vhr.addVersionLabel(versionName, labelName,false);
+versionLabels.add(labelName);
+}
+
+superuser.save();
+vhr.removeVersion(versionName);
+superuser.save();
+
+for(String label : versionLabels) {
+assertEquals("version label should not exist", false,
vhr.hasVersionLabel(label));
Review Comment:
This assertion would be easier to read if it was using `assertFalse()`.
##
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionHistoryTest.java:
##
@@ -159,4 +162,48 @@ public void testRemoveVHR() throws RepositoryException {
assertFalse("VersionHistory node should have disappeared",
superuser.itemExists(vhrpath));
}
+
+public void testRemoveVersionLabelWithRemovalOfVersion() throws
RepositoryException{
+int createVersions = 3;
+Node n = testRootNode.addNode(nodeName1, testNodeType);
+n.addMixin(mixVersionable);
+superuser.save();
+
+VersionManager vm = superuser.getWorkspace().getVersionManager();
+for (int i = 0; i < createVersions; i++) {
+vm.checkout(n.getPath());
+vm.checkin(n.getPath());
+}
+superuser.save();
+
+VersionHistory vhr = vm.getVersionHistory(n.getPath());
+// initialize versionName
+String versionName = "";
+VersionIterator allversions = vhr.getAllVersions();
+int count = 0;
+while (allversions.hasNext()) {
+Version version = allversions.nextVersion();
+if(count == 1) {
+versionName = version.getName();
+}
+count++;
+}
+int versonLabelCount = 3;
+List versionLabels = new ArrayList<>();
+
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r970706423
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStats.java:
##
@@ -157,6 +203,41 @@ public DocumentStoreStats(StatisticsProvider provider) {
prefetchNodes = provider.getMeter(NODES_PREFETCH,
StatsOptions.DEFAULT);
prefetchNodesTimer = provider.getTimer(NODES_PREFETCH_TIMER,
StatsOptions.METRICS_ONLY);
+
+// metrics for throttling
+MeterStats createSplitNodeWithThrottlingMeter =
provider.getMeter(NODES_CREATE_SPLIT_WITH_THROTTLING, StatsOptions.DEFAULT);
Review Comment:
I'm not sure, but I would rather keep the operation and throttling time
separate. That is, no matter if throttling is on or off, we always have a
metric for the operation with the underlying DocumentStore implementation. When
throttling is on and in effect, then we'd see it in a newly introduced metric.
For this case here, I would name it `NODES_CREATE_SPLIT_THROTTLING`. This
metric would only reflect/cover the throttling part of the operation.
Another advantage of this design is backward compatibility. With the
proposed change the current metric would not get any updates as soon as
throttling kicks in. It will look as if there were no operations at all while
throttling was in effect.
--
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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r970686461
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateStatsConsumer.java:
##
@@ -0,0 +1,31 @@
+/*
+ * 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.plugins.document.util;
+
+import java.util.List;
+
+/**
+ * {@link FunctionalInterface} to consume Metric Stats for create/upsert
operation
+ * @param the type of the first argument to the consumer
+ * @param the type of the second argument to the consumer
+ * @param the type of the third argument to the consumer
+ */
+public interface CreateStatsConsumer {
+void accept(T t, U u, V v, List ids, long tTN);
Review Comment:
I would not use generics here and in the other consumer interfaces, but
simply define the parameter types explicitly in accept(). I think it would make
it easier to read when all the parameters would have an explicit type and
intended use is documented. Currently it just says first, second and third
argument and it could be anything.
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStats.java:
##
@@ -157,6 +203,41 @@ public DocumentStoreStats(StatisticsProvider provider) {
prefetchNodes = provider.getMeter(NODES_PREFETCH,
StatsOptions.DEFAULT);
prefetchNodesTimer = provider.getTimer(NODES_PREFETCH_TIMER,
StatsOptions.METRICS_ONLY);
+
+// metrics for throttling
+MeterStats createSplitNodeWithThrottlingMeter =
provider.getMeter(NODES_CREATE_SPLIT_WITH_THROTTLING, StatsOptions.DEFAULT);
Review Comment:
I'm not sure, but I would rather keep the operation and throttling time
separate. That is no matter if throttling is on or off, we always have a metric
for the operation with the underlying DocumentStore implementation. When
throttling is on and in effect, then we'd see it in a newly introduced metric.
For this case here, I would name it `NODES_CREATE_SPLIT_THROTTLING`. This
metric would only reflect/cover the throttling part of the operation.
Another advantage of this design is backward compatibility. With the
proposed change the current metric would not get any updates as soon as
throttling kicks in. It will look as if there were no operations at all while
throttling was in effect.
--
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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r970674329
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateMetricUpdater.java:
##
@@ -0,0 +1,99 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.List;
+import java.util.function.BiPredicate;
+import java.util.function.Predicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneCreate(long, Collection, List, boolean)} for
underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class CreateMetricUpdater {
+
+private final MeterStats createNodeMeter;
+private final MeterStats createSplitNodeMeter;
+private final TimerStats createNodeTimer;
+private final MeterStats createJournal;
+private final TimerStats createJournalTimer;
+
+public CreateMetricUpdater(final MeterStats createNodeMeter,
+ final MeterStats createSplitNodeMeter,
+ final TimerStats createNodeTimer,
+ final MeterStats createJournal,
+ final TimerStats createJournalTimer) {
+this.createNodeMeter = createNodeMeter;
+this.createSplitNodeMeter = createSplitNodeMeter;
+this.createNodeTimer = createNodeTimer;
+this.createJournal = createJournal;
+this.createJournalTimer = createJournalTimer;
+}
+
+public void update(final Collection collection, final
long timeTakenNanos,
+ final List ids, final boolean insertSuccess,
+ final BiPredicate,
Integer> isNodesCollectionUpdated,
+ final CreateStatsConsumer createStatsConsumer,
+ final Predicate>
isJournalCollection,
+ final StatsConsumer
journalStatsConsumer) {
+
+requireNonNull(isNodesCollectionUpdated);
+requireNonNull(isJournalCollection);
+requireNonNull(createStatsConsumer);
+requireNonNull(journalStatsConsumer);
+
+if (isNodesCollectionUpdated.test(collection, ids.size()) &&
insertSuccess) {
+createStatsConsumer.accept(createNodeMeter, createSplitNodeMeter,
createNodeTimer, ids, timeTakenNanos);
+} else if (isJournalCollection.test(collection)) {
+journalStatsConsumer.accept(createJournal, createJournalTimer,
ids.size(), timeTakenNanos);
+}
+}
+
+public static class CreateMetricUpdaterWithoutThrottling extends
CreateMetricUpdater {
Review Comment:
> I can reuse the code in the base updater class, while using different
metrics based on whether or not throttling is going on.
But you can do this with just `CreateMetricUpdater` if it wasn't abstract. I
still don't see a need for the two sub classes.
--
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
[GitHub] [jackrabbit-oak] joerghoh merged pull request #703: OAK-9935 - Bump Elasticsearch client from 7.17.3 to 7.17.6
joerghoh merged PR #703: URL: https://github.com/apache/jackrabbit-oak/pull/703 -- 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
[GitHub] [jackrabbit-oak] reschke commented on a diff in pull request #683: OAK-9918: DocumentNodeStore consistency check
reschke commented on code in PR #683:
URL: https://github.com/apache/jackrabbit-oak/pull/683#discussion_r970532501
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/OrphanedNodeCheck.java:
##
@@ -0,0 +1,159 @@
+/*
+ * 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.plugins.document.check;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState;
+import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore;
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.apache.jackrabbit.oak.plugins.document.Path;
+import org.apache.jackrabbit.oak.plugins.document.RevisionVector;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * OrphanedNodeCheck...
+ */
+public class OrphanedNodeCheck implements DocumentProcessor, Closeable {
+
+private final DocumentNodeStore ns;
+
+private final RevisionVector headRevision;
+
+private final ExecutorService executorService;
+
+public OrphanedNodeCheck(DocumentNodeStore ns,
+ RevisionVector headRevision,
+ int numThread) {
+this.ns = ns;
+this.headRevision = headRevision;
+this.executorService = new ThreadPoolExecutor(
+numThread, numThread, 1, TimeUnit.MINUTES,
+new LinkedBlockingQueue<>(1000),
+new ThreadPoolExecutor.CallerRunsPolicy()
+);
+}
+
+@Override
+public void processDocument(@NotNull NodeDocument document,
+@NotNull BlockingQueue results) {
+if (!document.isSplitDocument()) {
+executorService.submit(new CheckDocument(ns, headRevision,
document, results));
+}
+}
+
+@Override
+public void end(@NotNull BlockingQueue results)
+throws InterruptedException {
+executorService.shutdown();
+if (!executorService.awaitTermination(5, TimeUnit.MINUTES)) {
+String msg = "Checks still not finished after the last one has
been submitted 5 minutes ago";
+results.put(() -> {
+StringBuilder sb = new StringBuilder("{");
Review Comment:
For better or worse, Oak already has it's own JSON library. IMHO that should
be used.
--
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
[GitHub] [jackrabbit-oak] jsedding commented on pull request #665: Oak 9897 split persistence gc
jsedding commented on PR #665: URL: https://github.com/apache/jackrabbit-oak/pull/665#issuecomment-1246465700 @dulceanu I would appreciate it very much if you could find the time for a review! -- 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
[GitHub] [jackrabbit-oak] jsedding commented on pull request #676: OAK-9785 - Tar SegmentStore can be corrupted during compaction
jsedding commented on PR #676: URL: https://github.com/apache/jackrabbit-oak/pull/676#issuecomment-1246463288 I am planning to merge this PR on Friday. cc @dulceanu @smiroslav -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #683: OAK-9918: DocumentNodeStore consistency check
rishabhdaim commented on code in PR #683:
URL: https://github.com/apache/jackrabbit-oak/pull/683#discussion_r970462572
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/Progress.java:
##
@@ -0,0 +1,62 @@
+/*
+ * 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.plugins.document.check;
+
+import java.util.concurrent.BlockingQueue;
+
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.apache.jackrabbit.oak.plugins.document.Path;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Progress...
+ */
+public class Progress implements DocumentProcessor {
+
+private long numDocuments = 0;
+
+@Override
+public void processDocument(@NotNull NodeDocument document,
+@NotNull BlockingQueue results)
+throws InterruptedException {
+if (++numDocuments % 1 == 0) {
+results.put(newProgressResult(numDocuments, document.getPath()));
+}
+}
+
+protected Result newProgressResult(long numDocs, Path path) {
+return new ProgressResult(numDocs, path);
+}
+
+class ProgressResult implements Result {
+
+protected final String msg;
+
+public ProgressResult(long numDocs, Path path) {
+this.msg = "Checked " + numDocs + " documents so far - " + path;
+}
+
+@Override
+public String toJson() {
+StringBuilder sb = new StringBuilder("{");
Review Comment:
Same as above, please use jsonobject
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/DocumentProcessor.java:
##
@@ -0,0 +1,41 @@
+/*
+ * 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.plugins.document.check;
+
+import java.util.concurrent.BlockingQueue;
+
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.apache.jackrabbit.util.ISO8601;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * DocumentProcessor...
+ */
+public interface DocumentProcessor {
+
+void processDocument(@NotNull NodeDocument document,
Review Comment:
Could you please add JavaDocs for this api.
##
oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/document/check/Summary.java:
##
@@ -0,0 +1,52 @@
+/*
+ * 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.plugins.document.check;
+
+import java.util.concurrent.BlockingQueue;
+
+import com.google.common.base.Stopwatch;
+
+import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Summary...
+ */
+public class Summary implements
[GitHub] [jackrabbit-oak] jelmini opened a new pull request, #704: OAK-9936 Optimize AzureJournalFile.AzureJournalWriter#batchWriteLines
jelmini opened a new pull request, #704: URL: https://github.com/apache/jackrabbit-oak/pull/704 Append multiple lines to the blob in one API call. Update the metadata once, after appending all lines. -- 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
[GitHub] [jackrabbit-oak] Joscorbe merged pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
Joscorbe merged PR #684: URL: https://github.com/apache/jackrabbit-oak/pull/684 -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on pull request #702: OAK-9934 : Backport of Oak-9535 "Support recovery of large branch merge" to 1.8
rishabhdaim commented on PR #702: URL: https://github.com/apache/jackrabbit-oak/pull/702#issuecomment-1244877850 @mreutegg @stefan-egli could you please review this 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] rishabhdaim opened a new pull request, #702: OAK-9535: Support recovery of large branch merge
rishabhdaim opened a new pull request, #702: URL: https://github.com/apache/jackrabbit-oak/pull/702 Squashed commits by Stefan Egli and Marcel Reutegger -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on pull request #701: OAK-9535: Support recovery of large branch merge
rishabhdaim commented on PR #701: URL: https://github.com/apache/jackrabbit-oak/pull/701#issuecomment-1244867385 @mreutegg @stefan-egli could you please review this 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] rishabhdaim opened a new pull request, #701: OAK-9535: Support recovery of large branch merge
rishabhdaim opened a new pull request, #701: URL: https://github.com/apache/jackrabbit-oak/pull/701 Squashed commits by Stefan Egli and Marcel Reutegger -- 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
[GitHub] [jackrabbit-filevault] kwin merged pull request #244: JCRVLT-650: apply default id conflict policy as late as possible
kwin merged PR #244: URL: https://github.com/apache/jackrabbit-filevault/pull/244 -- 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
[GitHub] [jackrabbit-oak] reschke merged pull request #700: OAK-9931: Update Oak 1.8 to Jackrabbit 2.16.10
reschke merged PR #700: URL: https://github.com/apache/jackrabbit-oak/pull/700 -- 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
[GitHub] [jackrabbit-oak] reschke closed pull request #699: OAK-9931: Update Oak 1.8 to Jackrabbit 2.16.10
reschke closed pull request #699: OAK-9931: Update Oak 1.8 to Jackrabbit 2.16.10 URL: https://github.com/apache/jackrabbit-oak/pull/699 -- 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
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #217: OAK-9073 Remove unnecessary (un)boxing in oak-commons
github-actions[bot] commented on PR #217: URL: https://github.com/apache/jackrabbit-oak/pull/217#issuecomment-1242848501 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #218: OAK-9074 Remove unnecessary (un)boxing in oak-core
github-actions[bot] commented on PR #218: URL: https://github.com/apache/jackrabbit-oak/pull/218#issuecomment-1242595547 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #219: OAK-9075 Remove unnecessary (un)boxing in oak-exercise
github-actions[bot] commented on PR #219: URL: https://github.com/apache/jackrabbit-oak/pull/219#issuecomment-1242595541 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r967209445
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStatsTest.java:
##
@@ -49,6 +74,10 @@ public void shutDown(){
statsProvider.close();
new ExecutorCloser(executor).close();
}
+@Before
+public void startUp() {
+stats = new DocumentStoreStats(statsProvider);
+}
Review Comment:
I will revert this change, I agree this is not required.
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r967205310
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/ModifyMetricUpdater.java:
##
@@ -0,0 +1,116 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.function.Consumer;
+import java.util.function.ObjIntConsumer;
+import java.util.function.Predicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneFindAndModify(long, Collection, String,
boolean, boolean, int)} for underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class ModifyMetricUpdater {
+
+private final MeterStats createNodeUpsertMeter;
+private final TimerStats createNodeUpsertTimer;
+private final MeterStats updateNodeMeter;
+private final TimerStats updateNodeTimer;
+private final MeterStats updateNodeRetryCountMeter;
+private final MeterStats updateNodeFailureMeter;
+
+public ModifyMetricUpdater(final MeterStats createNodeUpsertMeter,
+ final TimerStats createNodeUpsertTimer,
+ final MeterStats updateNodeMeter,
+ final TimerStats updateNodeTimer,
+ final MeterStats updateNodeRetryCountMeter,
+ final MeterStats updateNodeFailureMeter) {
+this.createNodeUpsertMeter = createNodeUpsertMeter;
+this.createNodeUpsertTimer = createNodeUpsertTimer;
+this.updateNodeMeter = updateNodeMeter;
+this.updateNodeTimer = updateNodeTimer;
+this.updateNodeRetryCountMeter = updateNodeRetryCountMeter;
+this.updateNodeFailureMeter = updateNodeFailureMeter;
+}
+
+public void update(final Collection collection, final
int retryCount,
+ final long timeTakenNanos, final boolean isSuccess,
final boolean newEntry,
+ final Predicate>
isNodesCollection,
+ final StatsConsumer
createStatsConsumer,
+ final StatsConsumer
updateStatsConsumer,
+ final ObjIntConsumer retryNodesConsumer,
+ final Consumer failureNodesConsumer) {
+
+requireNonNull(isNodesCollection);
+requireNonNull(createStatsConsumer);
+requireNonNull(updateStatsConsumer);
+requireNonNull(retryNodesConsumer);
+requireNonNull(failureNodesConsumer);
+
+if (isNodesCollection.negate().test(collection)) {return;}
Review Comment:
Sure, will update the code.
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r967205008
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateMetricUpdater.java:
##
@@ -0,0 +1,99 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.List;
+import java.util.function.BiPredicate;
+import java.util.function.Predicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneCreate(long, Collection, List, boolean)} for
underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class CreateMetricUpdater {
+
+private final MeterStats createNodeMeter;
+private final MeterStats createSplitNodeMeter;
+private final TimerStats createNodeTimer;
+private final MeterStats createJournal;
+private final TimerStats createJournalTimer;
+
+public CreateMetricUpdater(final MeterStats createNodeMeter,
+ final MeterStats createSplitNodeMeter,
+ final TimerStats createNodeTimer,
+ final MeterStats createJournal,
+ final TimerStats createJournalTimer) {
+this.createNodeMeter = createNodeMeter;
+this.createSplitNodeMeter = createSplitNodeMeter;
+this.createNodeTimer = createNodeTimer;
+this.createJournal = createJournal;
+this.createJournalTimer = createJournalTimer;
+}
+
+public void update(final Collection collection, final
long timeTakenNanos,
+ final List ids, final boolean insertSuccess,
+ final BiPredicate,
Integer> isNodesCollectionUpdated,
+ final CreateStatsConsumer createStatsConsumer,
+ final Predicate>
isJournalCollection,
+ final StatsConsumer
journalStatsConsumer) {
+
+requireNonNull(isNodesCollectionUpdated);
+requireNonNull(isJournalCollection);
+requireNonNull(createStatsConsumer);
+requireNonNull(journalStatsConsumer);
+
+if (isNodesCollectionUpdated.test(collection, ids.size()) &&
insertSuccess) {
+createStatsConsumer.accept(createNodeMeter, createSplitNodeMeter,
createNodeTimer, ids, timeTakenNanos);
+} else if (isJournalCollection.test(collection)) {
+journalStatsConsumer.accept(createJournal, createJournalTimer,
ids.size(), timeTakenNanos);
+}
+}
+
+public static class CreateMetricUpdaterWithoutThrottling extends
CreateMetricUpdater {
Review Comment:
I have created 2 implementations for each updater (abstract) class, one with
& other without throttling.
Their implementations only differ in metrics that are passed into their
respective constructors.
So the one with throttling has metrics with throttling and the other without
(as is the case with their respective names). Rest everything is identical.
In this way, I can reuse the code in the base updater class, while using
different metrics based on whether or not throttling is going on.
--
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
[GitHub] [jackrabbit-oak] gnaresh19 commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
gnaresh19 commented on code in PR #684: URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r967201197 ## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java: ## @@ -81,7 +81,6 @@ import org.apache.jackrabbit.oak.plugins.blob.datastore.SharedDataStoreUtils; import org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCacheStats; import org.apache.jackrabbit.oak.plugins.document.util.MongoConnection; -import org.apache.jackrabbit.oak.plugins.document.util.SystemPropertySupplier; Review Comment: Yeah, this was ununsed import, and since I made some other changes in this file, the unused import was removed too. -- 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
[GitHub] [jackrabbit-oak] Joscorbe commented on pull request #295: Support for Mongo Java Driver 4.7
Joscorbe commented on PR #295: URL: https://github.com/apache/jackrabbit-oak/pull/295#issuecomment-1242029348 Thanks @mreutegg and @rishabhdaim for your reviews. I think I have addressed all the suggestions in the code. I would appreciate if you have more comments or give green light to merge it. -- 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
[GitHub] [jackrabbit-oak] Joscorbe commented on a diff in pull request #295: Support for Mongo Java Driver 4.7
Joscorbe commented on code in PR #295:
URL: https://github.com/apache/jackrabbit-oak/pull/295#discussion_r967120638
##
oak-store-document/pom.xml:
##
@@ -44,8 +44,8 @@
${guava.osgi.import},
- com.mongodb*;version="[3.8, 4)";resolution:=optional,
- org.bson*;version="[3.8, 4)";resolution:=optional,
+ com.mongodb*;version="[3.8, 5)";resolution:=optional,
+ org.bson*;version="[3.8, 5)";resolution:=optional,
Review Comment:
No, they aren't compatible. I have upgraded them in this file. The minimum
MongoDB Driver version is now 4.7. I thought it could be a lower one like 4.0,
but due to some breaking changes introduced in several classes they are not
compatible.
--
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
[GitHub] [jackrabbit-oak] Joscorbe commented on a diff in pull request #295: Support for Mongo Java Driver 4.7
Joscorbe commented on code in PR #295:
URL: https://github.com/apache/jackrabbit-oak/pull/295#discussion_r967118924
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceTest.java:
##
@@ -203,42 +189,6 @@ public void persistentCacheExclude() throws Exception{
assertFalse(dns.getNodeCachePredicate().apply(Path.fromString("/x")));
}
-@Test
-public void preset() throws Exception {
-MockOsgi.setConfigForPid(context.bundleContext(),
-Configuration.PRESET_PID,
-DocumentNodeStoreServiceConfiguration.PROP_SO_KEEP_ALIVE,
true);
-MockOsgi.activate(preset, context.bundleContext());
-
-MockOsgi.setConfigForPid(context.bundleContext(), PID,
newConfig(repoHome));
-MockOsgi.activate(service, context.bundleContext());
-
-DocumentNodeStore store = context.getService(DocumentNodeStore.class);
-MongoDocumentStore mds = getMongoDocumentStore(store);
-assertNotNull(mds);
-MongoClient client = MongoDocumentStoreTestHelper.getClient(mds);
-assertTrue(client.getMongoClientOptions().isSocketKeepAlive());
-}
-
-@Test
-public void presetOverride() throws Exception {
-MockOsgi.setConfigForPid(context.bundleContext(),
-Configuration.PRESET_PID,
-DocumentNodeStoreServiceConfiguration.PROP_SO_KEEP_ALIVE,
true);
-MockOsgi.activate(preset, context.bundleContext());
-
-Map config = newConfig(repoHome);
-config.put(DocumentNodeStoreServiceConfiguration.PROP_SO_KEEP_ALIVE,
false);
-MockOsgi.setConfigForPid(context.bundleContext(), PID, config);
-
-MockOsgi.activate(service, context.bundleContext());
-
-DocumentNodeStore store = context.getService(DocumentNodeStore.class);
-MongoDocumentStore mds = getMongoDocumentStore(store);
-MongoClient client = MongoDocumentStoreTestHelper.getClient(mds);
-assertFalse(client.getMongoClientOptions().isSocketKeepAlive());
-}
-
Review Comment:
Yes, I have re-introduce both tests using the property PROP_UPDATE_LIMIT
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceTest.java:
##
@@ -203,42 +189,6 @@ public void persistentCacheExclude() throws Exception{
assertFalse(dns.getNodeCachePredicate().apply(Path.fromString("/x")));
}
-@Test
-public void preset() throws Exception {
-MockOsgi.setConfigForPid(context.bundleContext(),
-Configuration.PRESET_PID,
-DocumentNodeStoreServiceConfiguration.PROP_SO_KEEP_ALIVE,
true);
-MockOsgi.activate(preset, context.bundleContext());
-
-MockOsgi.setConfigForPid(context.bundleContext(), PID,
newConfig(repoHome));
-MockOsgi.activate(service, context.bundleContext());
-
-DocumentNodeStore store = context.getService(DocumentNodeStore.class);
-MongoDocumentStore mds = getMongoDocumentStore(store);
-assertNotNull(mds);
-MongoClient client = MongoDocumentStoreTestHelper.getClient(mds);
-assertTrue(client.getMongoClientOptions().isSocketKeepAlive());
-}
-
-@Test
-public void presetOverride() throws Exception {
-MockOsgi.setConfigForPid(context.bundleContext(),
-Configuration.PRESET_PID,
-DocumentNodeStoreServiceConfiguration.PROP_SO_KEEP_ALIVE,
true);
-MockOsgi.activate(preset, context.bundleContext());
-
-Map config = newConfig(repoHome);
-config.put(DocumentNodeStoreServiceConfiguration.PROP_SO_KEEP_ALIVE,
false);
-MockOsgi.setConfigForPid(context.bundleContext(), PID, config);
-
-MockOsgi.activate(service, context.bundleContext());
-
-DocumentNodeStore store = context.getService(DocumentNodeStore.class);
-MongoDocumentStore mds = getMongoDocumentStore(store);
-MongoClient client = MongoDocumentStoreTestHelper.getClient(mds);
-assertFalse(client.getMongoClientOptions().isSocketKeepAlive());
-}
-
Review Comment:
Yes, I have re-introduced both tests using the property PROP_UPDATE_LIMIT
--
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
[GitHub] [jackrabbit-oak] fabriziofortino merged pull request #693: OAK-9924: elastic mbean reports both primary and store size
fabriziofortino merged PR #693: URL: https://github.com/apache/jackrabbit-oak/pull/693 -- 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
[GitHub] [jackrabbit-oak] smiroslav merged pull request #696: OAK-9927: Caching persistence recovery
smiroslav merged PR #696: URL: https://github.com/apache/jackrabbit-oak/pull/696 -- 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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r966804973
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStatsTest.java:
##
@@ -49,6 +74,10 @@ public void shutDown(){
statsProvider.close();
new ExecutorCloser(executor).close();
}
+@Before
+public void startUp() {
+stats = new DocumentStoreStats(statsProvider);
+}
Review Comment:
Why is this needed? The instance variable `stats` already gets initialized
like this.
--
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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r966800246
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/UpsertMetricUpdater.java:
##
@@ -0,0 +1,80 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.List;
+import java.util.function.BiPredicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneCreateOrUpdate(long, Collection, List)} for
underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class UpsertMetricUpdater {
+
+private final MeterStats createNodeUpsertMeter;
+private final MeterStats createSplitNodeMeter;
+private final TimerStats createNodeUpsertTimer;
+
+public UpsertMetricUpdater(final MeterStats createNodeUpsertMeter,
+ final MeterStats createSplitNodeMeter,
+ final TimerStats createNodeUpsertTimer) {
+this.createNodeUpsertMeter = createNodeUpsertMeter;
+this.createSplitNodeMeter = createSplitNodeMeter;
+this.createNodeUpsertTimer = createNodeUpsertTimer;
+}
+
+public void update(final Collection collection, final
long timeTakenNanos,
+ final List ids, final BiPredicate, Integer> isNodesCollectionUpdated,
+ final CreateStatsConsumer upsertStatsConsumer) {
+
+requireNonNull(isNodesCollectionUpdated);
+requireNonNull(upsertStatsConsumer);
+
+if (isNodesCollectionUpdated.negate().test(collection, ids.size()))
{return;}
Review Comment:
Same as 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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r966799734
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/RemoveMetricUpdater.java:
##
@@ -0,0 +1,72 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.function.BiPredicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneRemove(long, Collection, int)} for underlying
{@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class RemoveMetricUpdater {
+
+private final MeterStats removeNodes;
+private final TimerStats removeNodesTimer;
+
+public RemoveMetricUpdater(final MeterStats removeNodes,
+ final TimerStats removeNodesTimer) {
+this.removeNodes = removeNodes;
+this.removeNodesTimer = removeNodesTimer;
+}
+
+public void update(final Collection collection, final
int removeCount, final long timeTakenNanos,
+ final BiPredicate,
Integer> isNodesCollectionUpdated,
+ final StatsConsumer
removeStatsConsumer) {
+
+requireNonNull(isNodesCollectionUpdated);
+requireNonNull(removeStatsConsumer);
+
+if (isNodesCollectionUpdated.negate().test(collection, removeCount))
{return;}
Review Comment:
Same as 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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r966799445
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/ModifyMetricUpdater.java:
##
@@ -0,0 +1,116 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.function.Consumer;
+import java.util.function.ObjIntConsumer;
+import java.util.function.Predicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneFindAndModify(long, Collection, String,
boolean, boolean, int)} for underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class ModifyMetricUpdater {
+
+private final MeterStats createNodeUpsertMeter;
+private final TimerStats createNodeUpsertTimer;
+private final MeterStats updateNodeMeter;
+private final TimerStats updateNodeTimer;
+private final MeterStats updateNodeRetryCountMeter;
+private final MeterStats updateNodeFailureMeter;
+
+public ModifyMetricUpdater(final MeterStats createNodeUpsertMeter,
+ final TimerStats createNodeUpsertTimer,
+ final MeterStats updateNodeMeter,
+ final TimerStats updateNodeTimer,
+ final MeterStats updateNodeRetryCountMeter,
+ final MeterStats updateNodeFailureMeter) {
+this.createNodeUpsertMeter = createNodeUpsertMeter;
+this.createNodeUpsertTimer = createNodeUpsertTimer;
+this.updateNodeMeter = updateNodeMeter;
+this.updateNodeTimer = updateNodeTimer;
+this.updateNodeRetryCountMeter = updateNodeRetryCountMeter;
+this.updateNodeFailureMeter = updateNodeFailureMeter;
+}
+
+public void update(final Collection collection, final
int retryCount,
+ final long timeTakenNanos, final boolean isSuccess,
final boolean newEntry,
+ final Predicate>
isNodesCollection,
+ final StatsConsumer
createStatsConsumer,
+ final StatsConsumer
updateStatsConsumer,
+ final ObjIntConsumer retryNodesConsumer,
+ final Consumer failureNodesConsumer) {
+
+requireNonNull(isNodesCollection);
+requireNonNull(createStatsConsumer);
+requireNonNull(updateStatsConsumer);
+requireNonNull(retryNodesConsumer);
+requireNonNull(failureNodesConsumer);
+
+if (isNodesCollection.negate().test(collection)) {return;}
Review Comment:
Can you please reformat and put return on a new line?
--
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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r966791050
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateMetricUpdater.java:
##
@@ -0,0 +1,99 @@
+/*
+ * 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.plugins.document.util;
+
+import org.apache.jackrabbit.oak.plugins.document.Collection;
+import org.apache.jackrabbit.oak.plugins.document.Document;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.DocumentStoreStatsCollector;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+
+import java.util.List;
+import java.util.function.BiPredicate;
+import java.util.function.Predicate;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Abstract class to update the metrics for {@link
DocumentStoreStatsCollector#doneCreate(long, Collection, List, boolean)} for
underlying {@link DocumentStore}
+ *
+ * Concrete implementations provide instances of {@link MeterStats}, {@link
TimerStats} based on whether throttling is ongoing or not
+ */
+public abstract class CreateMetricUpdater {
+
+private final MeterStats createNodeMeter;
+private final MeterStats createSplitNodeMeter;
+private final TimerStats createNodeTimer;
+private final MeterStats createJournal;
+private final TimerStats createJournalTimer;
+
+public CreateMetricUpdater(final MeterStats createNodeMeter,
+ final MeterStats createSplitNodeMeter,
+ final TimerStats createNodeTimer,
+ final MeterStats createJournal,
+ final TimerStats createJournalTimer) {
+this.createNodeMeter = createNodeMeter;
+this.createSplitNodeMeter = createSplitNodeMeter;
+this.createNodeTimer = createNodeTimer;
+this.createJournal = createJournal;
+this.createJournalTimer = createJournalTimer;
+}
+
+public void update(final Collection collection, final
long timeTakenNanos,
+ final List ids, final boolean insertSuccess,
+ final BiPredicate,
Integer> isNodesCollectionUpdated,
+ final CreateStatsConsumer createStatsConsumer,
+ final Predicate>
isJournalCollection,
+ final StatsConsumer
journalStatsConsumer) {
+
+requireNonNull(isNodesCollectionUpdated);
+requireNonNull(isJournalCollection);
+requireNonNull(createStatsConsumer);
+requireNonNull(journalStatsConsumer);
+
+if (isNodesCollectionUpdated.test(collection, ids.size()) &&
insertSuccess) {
+createStatsConsumer.accept(createNodeMeter, createSplitNodeMeter,
createNodeTimer, ids, timeTakenNanos);
+} else if (isJournalCollection.test(collection)) {
+journalStatsConsumer.accept(createJournal, createJournalTimer,
ids.size(), timeTakenNanos);
+}
+}
+
+public static class CreateMetricUpdaterWithoutThrottling extends
CreateMetricUpdater {
Review Comment:
Why are there `CreateMetricUpdaterWithoutThrottling` and
`CreateMetricUpdaterWithThrottling`? There seems to be no difference between
the two, except for a different name. Similar for other updater related classes.
--
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
[GitHub] [jackrabbit-oak] mreutegg commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
mreutegg commented on code in PR #690: URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r966786563 ## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentStoreStats.java: ## @@ -115,6 +152,29 @@ public class DocumentStoreStats implements DocumentStoreStatsCollector, Document private final TimerStats removeNodesTimer; private final MeterStats prefetchNodes; private final TimerStats prefetchNodesTimer; +private final MeterStats createSplitNodeWithThrottlingMeter; +private final MeterStats updateNodeRetryCountWithThrottlingMeter; +private final MeterStats updateNodeFailureWithThrottlingMeter; +private final MeterStats updateNodeWithThrottlingMeter; +private final TimerStats updateNodeWithThrottlingTimer; +private final MeterStats createNodeUpsertWithThrottlingMeter; +private final TimerStats createNodeUpsertWithThrottlingTimer; +private final MeterStats createNodeWithThrottlingMeter; +private final TimerStats createNodeWithThrottlingTimer; +private final MeterStats createJournalWithThrottling; +private final TimerStats createJournalWithThrottlingTimer; +private final MeterStats removeNodesWithThrottling; +private final TimerStats removeNodesWithThrottlingTimer; Review Comment: These and a few other fields can be turned into local variables. -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684: URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r966631406 ## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java: ## @@ -81,7 +81,6 @@ import org.apache.jackrabbit.oak.plugins.blob.datastore.SharedDataStoreUtils; import org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCacheStats; import org.apache.jackrabbit.oak.plugins.document.util.MongoConnection; -import org.apache.jackrabbit.oak.plugins.document.util.SystemPropertySupplier; Review Comment: I think you forgot to address this change. -- 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
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #220: OAK-9076 Remove unnecessary (un)boxing in oak-jcr
github-actions[bot] commented on PR #220: URL: https://github.com/apache/jackrabbit-oak/pull/220#issuecomment-1241420102 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #221: OAK-9077 Remove unnecessary (un)boxing in oak-lucene
github-actions[bot] commented on PR #221: URL: https://github.com/apache/jackrabbit-oak/pull/221#issuecomment-1241420087 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #222: OAK-9078 Remove unnecessary (un)boxing in oak-run
github-actions[bot] commented on PR #222: URL: https://github.com/apache/jackrabbit-oak/pull/222#issuecomment-1241420070 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #223: OAK-9079 Remove unnecessary (un)boxing in oak-security-spi
github-actions[bot] commented on PR #223: URL: https://github.com/apache/jackrabbit-oak/pull/223#issuecomment-1241420049 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] gnaresh19 commented on pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
gnaresh19 commented on PR #684: URL: https://github.com/apache/jackrabbit-oak/pull/684#issuecomment-1241000693 @rishabhdaim : Thanks, addressed the UTC per MongoThrottlerFactoryTest.java#L41-L45 -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r966193381
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
+
+}
+
+@Test (expected = IllegalArgumentException.class)
+public void invalidCollectionStorageOptions() throws Exception {
+
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"Invalid"));
+}
Review Comment:
I mean writing this test as follows:
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoThrottlerFactoryTest.java#L41-L45
--
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
[GitHub] [jackrabbit-oak] gnaresh19 commented on pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
gnaresh19 commented on PR #684: URL: https://github.com/apache/jackrabbit-oak/pull/684#issuecomment-1240962943 Thank you @rishabhdaim for the review. I addressed most of the comments as requested, just had questions on couple of them. can you pls confirm? -- 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
[GitHub] [jackrabbit-oak] gnaresh19 commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
gnaresh19 commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r966186291
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
+
+}
+
+@Test (expected = IllegalArgumentException.class)
+public void invalidCollectionStorageOptions() throws Exception {
+
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"Invalid"));
+}
+
+@Test
+public void overrideDefaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"zstd"));
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.ZSTD.getCompressionType())
> 0);
Review Comment:
same as 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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] gnaresh19 commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
gnaresh19 commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r966186075
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
Review Comment:
I agree with assertTrue() for comparing against expected value, but in this
particular test condition, the expected value (17) is not significant, the
validation is on whether the requested compressor is present. Pls do let me
know your thoughts on this.
--
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
[GitHub] [jackrabbit-oak] Joscorbe commented on a diff in pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
Joscorbe commented on code in PR #690:
URL: https://github.com/apache/jackrabbit-oak/pull/690#discussion_r965846627
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/StatsConsumer.java:
##
@@ -0,0 +1,28 @@
+/*
+ * 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.plugins.document.util;
+
+/**
+ * {@link FunctionalInterface} to consume Metric Stats for update/remove
operation
+ * @param
+ * @param
+ */
+public interface StatsConsumer {
Review Comment:
Same here
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/CreateStatsConsumer.java:
##
@@ -0,0 +1,31 @@
+/*
+ * 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.plugins.document.util;
+
+import java.util.List;
+
+/**
+ * {@link FunctionalInterface} to consume Metric Stats for create/upsert
operation
+ * @param
+ * @param
+ * @param
Review Comment:
Could you add some description to those parameters? I think that would
improve understandability of the interface and help in future cases whether it
could be useful to implement it.
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965790708
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
Review Comment:
I would use `Assert.Equals(string, string)` here.
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
+
+}
+
+@Test (expected = IllegalArgumentException.class)
+public void invalidCollectionStorageOptions() throws Exception {
+
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"Invalid"));
+}
+
+@Test
+public void overrideDefaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"zstd"));
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.ZSTD.getCompressionType())
> 0);
Revi
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965689623
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java:
##
@@ -443,6 +454,19 @@ private void ensureIndexes(@NotNull MongoStatus
mongoStatus) {
createIndex(journal, JournalEntry.MODIFIED, true, false, false);
}
+private void createCollection(MongoDatabase db, String collectionName,
MongoStatus mongoStatus) {
+CreateCollectionOptions options = new CreateCollectionOptions();
+
+if (mongoStatus.isVersion(4, 2)) {
+
options.storageEngineOptions(MongoDBConfig.getCollectionStorageOptions(mongoStorageOptions));
+if (!db.listCollectionNames()
Review Comment:
```suggestion
if (!Iterables.tryFind(localDb.listCollectionNames(), s ->
Objects.equals(collectionName, s)).isPresent())
```
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684: URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965776008 ## oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java: ## @@ -81,7 +81,6 @@ import org.apache.jackrabbit.oak.plugins.blob.datastore.SharedDataStoreUtils; import org.apache.jackrabbit.oak.plugins.document.persistentCache.PersistentCacheStats; import org.apache.jackrabbit.oak.plugins.document.util.MongoConnection; -import org.apache.jackrabbit.oak.plugins.document.util.SystemPropertySupplier; Review Comment: This is an un-intended change, please revert this. -- 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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965774219
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
+
+}
+
+@Test (expected = IllegalArgumentException.class)
+public void invalidCollectionStorageOptions() throws Exception {
+
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"Invalid"));
+}
Review Comment:
Please make this exception explicit by adding `Assert.fail("Shouldn't reach
here")`.
It would make it explicit that we are not expecting this code to reach here.
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfigTest.java:
##
@@ -0,0 +1,59 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.BasicDBObject;
+import com.mongodb.MongoClient;
+import
org.apache.jackrabbit.oak.plugins.document.mongo.MongoDBConfig.CollectionCompressor;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+import org.junit.Test;
+
+import java.util.Collections;
+import static org.junit.Assert.assertTrue;
+
+
+public class MongoDBConfigTest {
+
+private MongoDBConfig mongoDBConfig;
+
+@Test
+public void defaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.emptyMap());
+BsonDocument bsonDocument = bson.toBsonDocument(BasicDBObject.class,
MongoClient.getDefaultCodecRegistry());
+String configuredCompressor =
bsonDocument.getDocument(MongoDBConfig.STORAGE_ENGINE).getString(MongoDBConfig.STORAGE_CONFIG).getValue();
+
assertTrue(configuredCompressor.indexOf(CollectionCompressor.SNAPPY.getCompressionType())
> 0);
+
+}
+
+@Test (expected = IllegalArgumentException.class)
+public void invalidCollectionStorageOptions() throws Exception {
+
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION_TYPE,
"Invalid"));
+}
+
+@Test
+public void overrideDefaultCollectionStorageOptions() {
+Bson bson =
mongoDBConfig.getCollectionStorageOptions(Collections.singletonMap(MongoDBConfig.COLLECTION_COMPRESSION
[GitHub] [jackrabbit-oak] rishabhdaim commented on pull request #690: OAK-9913 : added metrics for mongo document store throttling feature
rishabhdaim commented on PR #690: URL: https://github.com/apache/jackrabbit-oak/pull/690#issuecomment-1240509642 @Joscorbe @stefan-egli @ashokpanghal could you please review 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] Joscorbe commented on a diff in pull request #295: Support for Mongo Java Driver 4.7
Joscorbe commented on code in PR #295:
URL: https://github.com/apache/jackrabbit-oak/pull/295#discussion_r965756037
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoClusterListener.java:
##
@@ -0,0 +1,82 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.mongodb.ServerAddress;
+import com.mongodb.connection.ServerConnectionState;
+import com.mongodb.connection.ServerDescription;
+import com.mongodb.event.ClusterDescriptionChangedEvent;
+import com.mongodb.event.ClusterListener;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+public class MongoClusterListener implements ClusterListener {
+
+// Sometimes we need to wait a few seconds in case the connection was just
created, the listener
+// didn't have time to receive the description from the cluster. This
latch is used to check
+// if the connection was properly initialized.
+private final CountDownLatch LATCH;
+private final int LATCH_AWAIT_TIMEOUT = 15;
+private boolean replicaSet = false;
+private ServerAddress serverAddress;
+private ServerAddress primaryAddress;
+
+public MongoClusterListener() {
+LATCH = new CountDownLatch(1);
+}
+
+@Nullable
+public ServerAddress getServerAddress() {
+waitForLatch();
+return serverAddress;
+}
+
+@Nullable
+public ServerAddress getPrimaryAddress() {
+waitForLatch();
+return primaryAddress;
+}
+
+public boolean isReplicaSet() {
+waitForLatch();
+return replicaSet;
+}
+
+@Override
+public void clusterDescriptionChanged(final ClusterDescriptionChangedEvent
event) {
+for (ServerDescription sd :
event.getNewDescription().getServerDescriptions()) {
Review Comment:
I have changed the first part of the code to this one, I think it's
equivalent and doesn't have to use the Google libraries for Optional and
Iterables, but instead it uses Java API code:
`Optional ol =
event.getNewDescription().getServerDescriptions()
.stream().filter(s -> s.getState() ==
ServerConnectionState.CONNECTED).findFirst();`
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965695309
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfig.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+
+import java.util.Map;
+
+public class MongoDBConfig {
+public static final String COLLECTION_COMPRESSION_TYPE =
"collectionCompressionType";
+public static final String STORAGE_ENGINE = "wiredTiger";
+public static final String STORAGE_CONFIG = "configString";
Review Comment:
Same as 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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965690953
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfig.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+
+import java.util.Map;
+
+public class MongoDBConfig {
+public static final String COLLECTION_COMPRESSION_TYPE =
"collectionCompressionType";
+public static final String STORAGE_ENGINE = "wiredTiger";
Review Comment:
It should be made default scoped.
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965689623
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java:
##
@@ -443,6 +454,19 @@ private void ensureIndexes(@NotNull MongoStatus
mongoStatus) {
createIndex(journal, JournalEntry.MODIFIED, true, false, false);
}
+private void createCollection(MongoDatabase db, String collectionName,
MongoStatus mongoStatus) {
+CreateCollectionOptions options = new CreateCollectionOptions();
+
+if (mongoStatus.isVersion(4, 2)) {
+
options.storageEngineOptions(MongoDBConfig.getCollectionStorageOptions(mongoStorageOptions));
+if (!db.listCollectionNames()
Review Comment:
```suggestion
if (Iterables.tryFind(localDb.listCollectionNames(), s ->
Objects.equals(collectionName, s)).isPresent())
```
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965682682
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfig.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+
+import java.util.Map;
+
+public class MongoDBConfig {
+public static final String COLLECTION_COMPRESSION_TYPE =
"collectionCompressionType";
+public static final String STORAGE_ENGINE = "wiredTiger";
+public static final String STORAGE_CONFIG = "configString";
+
+enum CollectionCompressor {
+SNAPPY("snappy"), ZLIB("zlib"), ZSTD("zstd");
+
+private final String compressionType;
+
+private static final Map
supportedCompressionTypes = ImmutableMap.of(
Review Comment:
```suggestion
private static final Map
COLLECTION_COMPRESSOR_MAP;
static {
ImmutableMap.Builder builder = new
ImmutableMap.Builder<>();
for (CollectionCompressor value : CollectionCompressor.values())
{
builder.put(value.getName().toLowerCase(), value);
}
COLLECTION_COMPRESSOR_MAP = builder.build();
}
```
Using lowerCase as a key, avoids unnecessary errors because of the case, and
using `Enum.values()` would help in avoiding missing a new compressor type (if
added).
--
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
[GitHub] [jackrabbit-oak] fabriziofortino commented on a diff in pull request #693: OAK-9944: elastic mbean reports both primary and store size
fabriziofortino commented on code in PR #693:
URL: https://github.com/apache/jackrabbit-oak/pull/693#discussion_r965687213
##
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexStatistics.java:
##
@@ -205,13 +216,15 @@ private StatsResponse stats(StatsRequestDescriptor crd)
throws IOException {
}
// Assuming a single index matches crd.index
IndicesRecord record = records.get(0);
-String size = record.storeSize();
+String storeSize = record.storeSize();
+String primaryStoreSize = record.storeSize();
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: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [jackrabbit-oak] nit0906 commented on a diff in pull request #693: OAK-9944: elastic mbean reports both primary and store size
nit0906 commented on code in PR #693:
URL: https://github.com/apache/jackrabbit-oak/pull/693#discussion_r965681070
##
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexStatistics.java:
##
@@ -205,13 +216,15 @@ private StatsResponse stats(StatsRequestDescriptor crd)
throws IOException {
}
// Assuming a single index matches crd.index
IndicesRecord record = records.get(0);
-String size = record.storeSize();
+String storeSize = record.storeSize();
+String primaryStoreSize = record.storeSize();
Review Comment:
Shouldn't this be `record.priStoreSize()` instead ?
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965684512
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfig.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+
+import java.util.Map;
+
+public class MongoDBConfig {
+public static final String COLLECTION_COMPRESSION_TYPE =
"collectionCompressionType";
+public static final String STORAGE_ENGINE = "wiredTiger";
+public static final String STORAGE_CONFIG = "configString";
+
+enum CollectionCompressor {
+SNAPPY("snappy"), ZLIB("zlib"), ZSTD("zstd");
+
+private final String compressionType;
+
+private static final Map
supportedCompressionTypes = ImmutableMap.of(
+"snappy", CollectionCompressor.SNAPPY, "zlib",
+CollectionCompressor.ZLIB, "zstd", CollectionCompressor.ZSTD);
+
+CollectionCompressor(String compressionType) {
+this.compressionType = compressionType;
+}
+
+public String getCompressionType() {
+return compressionType;
+}
+
+public static boolean isSupportedCompressor(String compressionType) {
+return supportedCompressionTypes.containsKey(compressionType);
Review Comment:
```suggestion
return
COLLECTION_COMPRESSOR_MAP.containsKey(compressionType.toLowerCase());
```
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965684140
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfig.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+
+import java.util.Map;
+
+public class MongoDBConfig {
+public static final String COLLECTION_COMPRESSION_TYPE =
"collectionCompressionType";
+public static final String STORAGE_ENGINE = "wiredTiger";
+public static final String STORAGE_CONFIG = "configString";
+
+enum CollectionCompressor {
+SNAPPY("snappy"), ZLIB("zlib"), ZSTD("zstd");
+
+private final String compressionType;
+
+private static final Map
supportedCompressionTypes = ImmutableMap.of(
+"snappy", CollectionCompressor.SNAPPY, "zlib",
+CollectionCompressor.ZLIB, "zstd", CollectionCompressor.ZSTD);
+
+CollectionCompressor(String compressionType) {
+this.compressionType = compressionType;
+}
+
+public String getCompressionType() {
Review Comment:
the method should be renamed to `getName()` because we want to know the name
of this compressionType.
--
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
[GitHub] [jackrabbit-oak] rishabhdaim commented on a diff in pull request #684: [Oak 9919] Add support for zstd, zlib to document store with mongodb
rishabhdaim commented on code in PR #684:
URL: https://github.com/apache/jackrabbit-oak/pull/684#discussion_r965682682
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDBConfig.java:
##
@@ -0,0 +1,88 @@
+/*
+ * 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.plugins.document.mongo;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.jackrabbit.oak.commons.json.JsonObject;
+import org.bson.BsonDocument;
+import org.bson.conversions.Bson;
+
+import java.util.Map;
+
+public class MongoDBConfig {
+public static final String COLLECTION_COMPRESSION_TYPE =
"collectionCompressionType";
+public static final String STORAGE_ENGINE = "wiredTiger";
+public static final String STORAGE_CONFIG = "configString";
+
+enum CollectionCompressor {
+SNAPPY("snappy"), ZLIB("zlib"), ZSTD("zstd");
+
+private final String compressionType;
+
+private static final Map
supportedCompressionTypes = ImmutableMap.of(
Review Comment:
```suggestion
private static final Map
COLLECTION_COMPRESSOR_MAP;
static {
ImmutableMap.Builder builder = new
ImmutableMap.Builder<>();
for (CollectionCompressor value : CollectionCompressor.values())
{
builder.put(value.getName().toLowerCase(), value);
}
COLLECTION_COMPRESSOR_MAP = builder.build();
}
```
--
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
[GitHub] [jackrabbit-oak] nit0906 commented on a diff in pull request #693: OAK-9944: elastic mbean reports both primary and store size
nit0906 commented on code in PR #693:
URL: https://github.com/apache/jackrabbit-oak/pull/693#discussion_r965681070
##
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexStatistics.java:
##
@@ -205,13 +216,15 @@ private StatsResponse stats(StatsRequestDescriptor crd)
throws IOException {
}
// Assuming a single index matches crd.index
IndicesRecord record = records.get(0);
-String size = record.storeSize();
+String storeSize = record.storeSize();
+String primaryStoreSize = record.storeSize();
Review Comment:
Should this be `record.priStoreSize()` instead ?
--
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
[GitHub] [jackrabbit-oak] ArunOnCloud commented on pull request #688: Removal (purge) of version of a node does not remove associated labels
ArunOnCloud commented on PR #688: URL: https://github.com/apache/jackrabbit-oak/pull/688#issuecomment-1240237803 @mreutegg can you please review it. -- 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
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #224: OAK-9080 Remove unnecessary (un)boxing in oak-segment-azure
github-actions[bot] commented on PR #224: URL: https://github.com/apache/jackrabbit-oak/pull/224#issuecomment-1240135294 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #225: OAK-9081 Remove unnecessary (un)boxing in oak-solr-core
github-actions[bot] commented on PR #225: URL: https://github.com/apache/jackrabbit-oak/pull/225#issuecomment-1240135275 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #226: OAK-9082 Remove unnecessary (un)boxing in oak-store-composite
github-actions[bot] commented on PR #226: URL: https://github.com/apache/jackrabbit-oak/pull/226#issuecomment-1240135262 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #227: OAK-9083 Remove unnecessary (un)boxing in oak-store-document
github-actions[bot] commented on PR #227: URL: https://github.com/apache/jackrabbit-oak/pull/227#issuecomment-1240135240 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #235: s3Connector changes for regions apart from aws default regions
github-actions[bot] commented on PR #235: URL: https://github.com/apache/jackrabbit-oak/pull/235#issuecomment-1240135169 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #228: OAK-9084 Remove unnecessary (un)boxing in oak-store-spi
github-actions[bot] commented on PR #228: URL: https://github.com/apache/jackrabbit-oak/pull/228#issuecomment-1240135216 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [jackrabbit-oak] github-actions[bot] commented on pull request #229: OAK-9085 Remove unnecessary (un)boxing in oak-webapp
github-actions[bot] commented on PR #229: URL: https://github.com/apache/jackrabbit-oak/pull/229#issuecomment-1240135192 This PR is stale because it has been open 365 days 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
