[GH] (jackrabbit-oak): Workflow run "SonarCloud" is working again!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has succeeded. Run started by GitHub user shodaaan (triggered by rishabhdaim). Head commit for run: 252a66295be3d9ced6520dbb28d0660191bb2196 / shodaaan - fixed camel case Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9647461022 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user stefan-egli (triggered by stefan-egli). Head commit for run: a0294fdda242cbc553ad1dc066c9a7084e1cf695 / stefan-egli OAK-10843 : exclude a flaky test combination (#1554) Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9648562521 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user Joscorbe (triggered by Joscorbe). Head commit for run: a32c7dd5bd768174d8c3a9d0a2a80e6b6da8d324 / José Andrés Cordero Benítez OAK-10748: Added report indentation like in collect method Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644544403 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user Joscorbe (triggered by Joscorbe). Head commit for run: 66188405e2949aba3f4dc425e612c64f5afafebc / Jose Cordero Unneeded import. Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9647893201 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user Joscorbe (triggered by Joscorbe). Head commit for run: 1d700cfb1c6673593c2ec8be5f84d7d6e6236f25 / Jose Cordero OAK-10915: Avoid deleting expired checkpoints when running FullGC on a read-only NodeStore Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9647855990 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user dulceanu (triggered by dulceanu). Head commit for run: 2b734cfba6e8300e9846097c459398c65d75cdb0 / Andrei Dulceanu OAK-10917 - Make persistent cache related arguments optional for Azure compaction (#1555) Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9647486810 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user stefan-egli (triggered by stefan-egli). Head commit for run: b1c9659fe6e8832987b20b805cfe3384573a8546 / Stefan Egli OAK-10843 : exclude a flaky test combination Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9647333526 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user dulceanu (triggered by dulceanu). Head commit for run: 262973e58288f843f2bd76d0d69a7223f1580f7e / Andrei Dulceanu OAK-10917 - Make persistent cache related arguments optional for Azure compaction Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9646842326 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10843 : exclude a flaky test combination [jackrabbit-oak]
Ccscsce@÷@÷e@ ÷se""÷csse,,ec,scce,se,cse,se,se,cse,csse,ccsce,ssse,cssce,se,csce,scce,e,cscc,sce,csccssess@ ÷÷@@ßeéwrŕrrrŕ On Mon, 24 Jun, 2024, 19:00 stefan-egli (via GitHub), wrote: > > stefan-egli opened a new pull request, #1554: > URL: https://github.com/apache/jackrabbit-oak/pull/1554 > >(no comment) > > > -- > This is an automated message from the Apache Git Service. > To respond to the message, please log on to GitHub and use the > URL above to go to the specific comment. > > To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org > > For queries about this service, please contact Infrastructure at: > us...@infra.apache.org > >
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1651122982
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java:
##
@@ -236,6 +237,30 @@ public void fullGCDisabledForRDB() {
assertFalse("Full GC is disabled for RDB Document Store",
fullGCEnabled);
}
+@Test
+public void fullGCModeDefaultValue() {
+int fullGCModeDefaultValue =
getFullGCMode(newDocumentNodeStoreBuilder());
+final int FULL_GC_MODE_NONE = 0;
Review Comment:
Done.
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java:
##
@@ -236,6 +237,30 @@ public void fullGCDisabledForRDB() {
assertFalse("Full GC is disabled for RDB Document Store",
fullGCEnabled);
}
+@Test
+public void fullGCModeDefaultValue() {
+int fullGCModeDefaultValue =
getFullGCMode(newDocumentNodeStoreBuilder());
+final int FULL_GC_MODE_NONE = 0;
+assertEquals("Full GC mode has NONE value by default",
fullGCModeDefaultValue, FULL_GC_MODE_NONE);
+}
+
+@Test
+public void fullGCModeSetViaConfiguration() {
+DocumentNodeStoreBuilder builder = newDocumentNodeStoreBuilder();
+final int FULL_GC_MODE_GAP_ORPHANS = 2;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1651122588
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java:
##
@@ -1009,6 +1009,16 @@ public static boolean
isEmbeddedVerificationEnabled(final DocumentNodeStoreBuild
return builder.isEmbeddedVerificationEnabled() ||
(docStoreEmbeddedVerificationFeature != null &&
docStoreEmbeddedVerificationFeature.isEnabled());
}
+/**
+ * Returns the full GC mode value for the DocumentNodeStore.
+ *
+ * @param builder instance for DocumentNodeStoreBuilder
+ * @return true if full GC is enabled else false
+ */
+public static int getFullGCMode(final DocumentNodeStoreBuilder builder)
{
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539: URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r165738 ## oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java: ## @@ -47,13 +47,13 @@ import org.apache.jackrabbit.oak.plugins.document.NodeDocument; import org.apache.jackrabbit.oak.plugins.document.RevisionContextWrapper; import org.apache.jackrabbit.oak.plugins.document.SweepHelper; +import org.apache.jackrabbit.oak.plugins.document.VersionGCOptions; import org.apache.jackrabbit.oak.plugins.document.VersionGCSupport; +import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector; import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCInfo; import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats; import org.apache.jackrabbit.oak.plugins.document.util.MongoConnection; import org.apache.jackrabbit.oak.run.commons.Command; -import org.apache.jackrabbit.oak.plugins.document.VersionGCOptions; -import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector; Review Comment: done -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10917 - Make persistent cache related arguments optional for Azur… [jackrabbit-oak]
dulceanu opened a new pull request, #1555: URL: https://github.com/apache/jackrabbit-oak/pull/1555 …e compaction -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1651098372
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -2603,7 +2605,7 @@ private void cleanOrphanedBranches() {
}
private void cleanRootCollisions() {
-String id = Utils.getIdFromPath(ROOT);
+String id = getIdFromPath(ROOT);
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1651096514
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -692,7 +694,7 @@ public int getMemory() {
diffCache = builder.getDiffCache(this.clusterId);
// check if root node exists
-NodeDocument rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
+NodeDocument rootDoc = store.find(NODES, getIdFromPath(ROOT));
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
rishabhdaim commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1651080682
##
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##
@@ -69,10 +69,11 @@
import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
import static
org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreHelper.createVersionGC;
import static
org.apache.jackrabbit.oak.plugins.document.FormatVersion.versionOf;
+import static
org.apache.jackrabbit.oak.plugins.document.util.Utils.getFullGCMode;
import static
org.apache.jackrabbit.oak.plugins.document.util.Utils.getIdFromPath;
import static
org.apache.jackrabbit.oak.plugins.document.util.Utils.getRootDocument;
-import static
org.apache.jackrabbit.oak.plugins.document.util.Utils.isFullGCEnabled;
Review Comment:
same as above.
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##
@@ -211,6 +211,29 @@ static FullGCMode getFullGcMode() {
return fullGcMode;
}
+/**
+ * Set the full GC mode to be used according to the provided configuration
value.
+ * The configuration value will be ignored and fullGCMode will be reset to
NONE
+ * if it is set to any other values than the supported ones.
+ * @param fullGcModeConfig
+ */
+static void setFullGcMode(int fullGcModeConfig) {
Review Comment:
I would suggest renaming variable name to `fullGcMode`
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java:
##
@@ -236,6 +237,30 @@ public void fullGCDisabledForRDB() {
assertFalse("Full GC is disabled for RDB Document Store",
fullGCEnabled);
}
+@Test
+public void fullGCModeDefaultValue() {
+int fullGCModeDefaultValue =
getFullGCMode(newDocumentNodeStoreBuilder());
+final int FULL_GC_MODE_NONE = 0;
Review Comment:
please use camelCase here.
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java:
##
@@ -1009,6 +1009,16 @@ public static boolean
isEmbeddedVerificationEnabled(final DocumentNodeStoreBuild
return builder.isEmbeddedVerificationEnabled() ||
(docStoreEmbeddedVerificationFeature != null &&
docStoreEmbeddedVerificationFeature.isEnabled());
}
+/**
+ * Returns the full GC mode value for the DocumentNodeStore.
+ *
+ * @param builder instance for DocumentNodeStoreBuilder
+ * @return true if full GC is enabled else false
+ */
+public static int getFullGCMode(final DocumentNodeStoreBuilder builder)
{
Review Comment:
This utility method doesn't make sense, it only returns `fullGcMode` from
the `builder` object. I would remove this.
##
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##
@@ -47,13 +47,13 @@
import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
import org.apache.jackrabbit.oak.plugins.document.RevisionContextWrapper;
import org.apache.jackrabbit.oak.plugins.document.SweepHelper;
+import org.apache.jackrabbit.oak.plugins.document.VersionGCOptions;
import org.apache.jackrabbit.oak.plugins.document.VersionGCSupport;
+import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector;
import
org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCInfo;
import
org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
import org.apache.jackrabbit.oak.plugins.document.util.MongoConnection;
import org.apache.jackrabbit.oak.run.commons.Command;
-import org.apache.jackrabbit.oak.plugins.document.VersionGCOptions;
-import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector;
Review Comment:
please revert these import changes.
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java:
##
@@ -236,6 +237,30 @@ public void fullGCDisabledForRDB() {
assertFalse("Full GC is disabled for RDB Document Store",
fullGCEnabled);
}
+@Test
+public void fullGCModeDefaultValue() {
+int fullGCModeDefaultValue =
getFullGCMode(newDocumentNodeStoreBuilder());
+final int FULL_GC_MODE_NONE = 0;
+assertEquals("Full GC mode has NONE value by default",
fullGCModeDefaultValue, FULL_GC_MODE_NONE);
+}
+
+@Test
+public void fullGCModeSetViaConfiguration() {
+DocumentNodeStoreBuilder builder = newDocumentNodeStoreBuilder();
+final int FULL_GC_MODE_GAP_ORPHANS = 2;
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: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user stefan-egli (triggered by stefan-egli). Head commit for run: 512cf2f2c77f6656cf7ee72a5b7dc7ca1a02e500 / Stefan Egli OAK-10843 : exclude a flaky test combination Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9646292798 With regards, GitHub Actions via GitBox
[PR] OAK-10843 : exclude a flaky test combination [jackrabbit-oak]
stefan-egli opened a new pull request, #1554: URL: https://github.com/apache/jackrabbit-oak/pull/1554 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user rishabhdaim (triggered by rishabhdaim). Head commit for run: 922d17ea5d15911837a58da877bb2de5b30b7eb5 / Rishabh Kumar Merge pull request #1551 from apache/OAK-10912 OAK-10912 : make fullGc include/exclude paths configurable via OSGI config Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9645634693 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" is working again!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has succeeded. Run started by GitHub user shodaaan (triggered by rishabhdaim). Head commit for run: cda3b5c40513709599073a5844749906ea0032c4 / shodaaan - made some of the requested PR changes - reverted CommitBuilderTest to previous version Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644856242 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim merged PR #1551: URL: https://github.com/apache/jackrabbit-oak/pull/1551 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10896: - Add OSGI configuration variable for removal of deleted properties and orphaned nodes [jackrabbit-oak]
stefan-egli commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1650969435
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -692,7 +694,7 @@ public int getMemory() {
diffCache = builder.getDiffCache(this.clusterId);
// check if root node exists
-NodeDocument rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
+NodeDocument rootDoc = store.find(NODES, getIdFromPath(ROOT));
Review Comment:
but it's in a random unrelated code - which is against our goal to not do
unrelated changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user Joscorbe (triggered by Joscorbe). Head commit for run: a32c7dd5bd768174d8c3a9d0a2a80e6b6da8d324 / José Andrés Cordero Benítez OAK-10748: Added report indentation like in collect method Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644544403 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user Joscorbe (triggered by Joscorbe). Head commit for run: d33dd23ae5569e84fe6b4fb4585685dd197f73d4 / José Andrés Cordero Benítez OAK-10748: Restore public on setStatisticsProvider method Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644510888 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user stefan-egli (triggered by stefan-egli). Head commit for run: 9d4d35cf14c241e6474a513c22bf43327eee3a12 / stefan-egli Merge pull request #1553 from apache/OAK-10914-1 OAK-10914 : disable mongo-server-side exclude paths until query perfo… Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644496669 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user stefan-egli (triggered by stefan-egli). Head commit for run: dc8bdb8c000d7c68d66dfcc1bb2ac4cb5f1c9ae4 / Stefan Egli OAK-10914 : disable mongo-server-side exclude paths until query performance is fixed Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644367856 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user rishabhdaim (triggered by rishabhdaim). Head commit for run: 55f310e2d65cfe3208ff7688ce93787eccb0cfba / Rishabh Kumar OAK-10912 : changed config names and updated documentation Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9644296504 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650892053
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java:
##
@@ -83,7 +84,9 @@ public void defaultValues() throws Exception {
assertEquals("MONGO", config.documentStoreType());
assertEquals(DocumentNodeStoreService.DEFAULT_BUNDLING_DISABLED,
config.bundlingDisabled());
assertEquals(DocumentMK.Builder.DEFAULT_UPDATE_LIMIT,
config.updateLimit());
-assertEquals(Arrays.asList("/"),
Arrays.asList(config.persistentCacheIncludes()));
+assertEquals(of("/"), Arrays.asList(config.persistentCacheIncludes()));
Review Comment:
Yes, it can be avoided, but IntelliJ insisted on making 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: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1650889773
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java:
##
@@ -531,6 +531,57 @@ public void testResetFullGCDryRunMode() throws Exception {
// OAK-10370 END
+// OAK-10896
+
+@Test
+public void testVersionGCLoadGCModeConfigurationNotApplicable() throws
Exception {
+int fullGcModeNotAllowedValue = 5;
+int fullGcModeGapOrphans = 2;
+
+// set fullGcMode to allowed value that is different than NONE
+VersionGarbageCollector.setFullGcMode(fullGcModeGapOrphans);
+
+// reinitialize VersionGarbageCollector with not allowed value
+VersionGarbageCollector gc = new VersionGarbageCollector(
+ns, new VersionGCSupport(store), true, false, false,
+fullGcModeNotAllowedValue);
+
+assertEquals("Starting VersionGarbageCollector with not applicable /
not allowed value" +
+"will set fullGcMode to default NONE", gc.getFullGcMode(),
VersionGarbageCollector.FullGCMode.NONE);
Review Comment:
Fixed the comment, unit tests and static change, thank you for the
suggestions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1650887782
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java:
##
@@ -238,6 +239,8 @@ static DocumentStoreType fromString(String type) {
private Feature cancelInvalidationFeature;
private Feature docStoreFullGCFeature;
private Feature docStoreEmbeddedVerificationFeature;
+private Feature docStoreFullGCModeGapOrphansFeature;
+private Feature docStoreFullGCModeEmptyPropertiesFeature;
Review Comment:
Fixed, thank you for the comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1650887172
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -692,7 +694,7 @@ public int getMemory() {
diffCache = builder.getDiffCache(this.clusterId);
// check if root node exists
-NodeDocument rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
+NodeDocument rootDoc = store.find(NODES, getIdFromPath(ROOT));
Review Comment:
This is consistent with how we are using static method calls. The consistent
approach would be IMHO to actually do this everywhere, but I think a
refactoring ticket is the best place for that.
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -716,7 +718,7 @@ public int getMemory() {
setRoot(head);
// make sure _lastRev is written back to store
backgroundWrite();
-rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
+rootDoc = store.find(NODES, getIdFromPath(ROOT));
Review Comment:
This is consistent with how we are using static method calls. The consistent
approach would be IMHO to actually do this everywhere, but I think a
refactoring ticket is the best place for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10748: Improve statistics to collect which type of garbage is sent/deleted [jackrabbit-oak]
Joscorbe commented on PR #1543: URL: https://github.com/apache/jackrabbit-oak/pull/1543#issuecomment-2186350396 I will squash this PR into a single commit once approved. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10914 : disable mongo-server-side exclude paths until query perfo… [jackrabbit-oak]
stefan-egli merged PR #1553: URL: https://github.com/apache/jackrabbit-oak/pull/1553 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10748: Improve statistics to collect which type of garbage is sent/deleted [jackrabbit-oak]
Joscorbe commented on code in PR #1543:
URL: https://github.com/apache/jackrabbit-oak/pull/1543#discussion_r1650867475
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##
@@ -240,7 +240,21 @@ static FullGCMode getFullGcMode() {
AUDIT_LOG.info(" VersionGarbageCollector created with fullGcMode
= {}", fullGcMode);
}
-public void setStatisticsProvider(StatisticsProvider provider) {
+/**
+ * Please note that at the moment the includes do not
+ * take long paths into account. That is, if a long path was
+ * supposed to be included via an include, it is not.
+ * Reason for this is that long paths would require
+ * the mongo query to include a '_path' condition - which disallows
+ * mongo from using the '_modified_id' index. IOW long paths
+ * would result in full scans - which results in bad performance.
+ */
+void setFullGCPaths(@NotNull Set includes, @NotNull Set
excludes) {
+this.fullGCIncludePaths = requireNonNull(includes);
+this.fullGCExcludePaths = requireNonNull(excludes);
+}
+
+void setStatisticsProvider(StatisticsProvider provider) {
Review Comment:
Oh, the public was lost here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10914 : disable mongo-server-side exclude paths until query perfo… [jackrabbit-oak]
stefan-egli commented on PR #1553: URL: https://github.com/apache/jackrabbit-oak/pull/1553#issuecomment-2186346121 > I think we might need to update the test for this as well. Client-side incl/exclude always also applies (perhaps something to improve at some point), so if mongo doesn't do server-side exclude, the client-side still applies, so tests still run fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
stefan-egli commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650864494
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java:
##
@@ -83,7 +84,9 @@ public void defaultValues() throws Exception {
assertEquals("MONGO", config.documentStoreType());
assertEquals(DocumentNodeStoreService.DEFAULT_BUNDLING_DISABLED,
config.bundlingDisabled());
assertEquals(DocumentMK.Builder.DEFAULT_UPDATE_LIMIT,
config.updateLimit());
-assertEquals(Arrays.asList("/"),
Arrays.asList(config.persistentCacheIncludes()));
+assertEquals(of("/"), Arrays.asList(config.persistentCacheIncludes()));
Review Comment:
nitpick : looks unrelated/unnecessary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10914 : disable mongo-server-side exclude paths until query perfo… [jackrabbit-oak]
rishabhdaim commented on PR #1553: URL: https://github.com/apache/jackrabbit-oak/pull/1553#issuecomment-2186335794 I think we might need to update the test for this as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] OAK-10914 : disable mongo-server-side exclude paths until query perfo… [jackrabbit-oak]
stefan-egli opened a new pull request, #1553: URL: https://github.com/apache/jackrabbit-oak/pull/1553 …rmance is fixed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10748: Improve statistics to collect which type of garbage is sent/deleted [jackrabbit-oak]
Joscorbe commented on PR #1543: URL: https://github.com/apache/jackrabbit-oak/pull/1543#issuecomment-2186329449 > actually, there are compilation errors: > > ``` > [INFO] - > Error: COMPILATION ERROR : > [INFO] - > Error: /home/runner/work/jackrabbit-oak/jackrabbit-oak/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:[367,11] setStatisticsProvider(org.apache.jackrabbit.oak.stats.StatisticsProvider) is not public in org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector; cannot be accessed from outside package > Error: /home/runner/work/jackrabbit-oak/jackrabbit-oak/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:[538,11] setStatisticsProvider(org.apache.jackrabbit.oak.stats.StatisticsProvider) is not public in org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector; cannot be accessed from outside package > [INFO] 2 errors > ``` I will double-check this, sounds weird, I built it and ran all the integration tests locally... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10748: Improve statistics to collect which type of garbage is sent/deleted [jackrabbit-oak]
Joscorbe commented on code in PR #1543:
URL: https://github.com/apache/jackrabbit-oak/pull/1543#discussion_r1650854809
##
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##
@@ -535,8 +544,8 @@ private void collectDocument(RevisionsOptions options,
Closer closer, String pat
}
gc.collectGarbageOnDocument(documentNodeStore, workingDocument,
options.isVerbose());
-//TODO: Probably we should output some details of fullGCStats. Could
be done after OAK-10378
-//gc.getFullGCStats();
+System.out.println("Full GC Stats:");
+System.out.println(gc.getFullGCStatsReport());
Review Comment:
Not on purpose, I will add them here 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: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650849974
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
+"Any path which is added to both include & exclude paths,
" +
+"would be removed from included paths." +
Review Comment:
fixed in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/55f310e2d65cfe3208ff7688ce93787eccb0cfba
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
Review Comment:
fixed in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/55f310e2d65cfe3208ff7688ce93787eccb0cfba
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
Review Comment:
fixed in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/55f310e2d65cfe3208ff7688ce93787eccb0cfba
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650849122
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java:
##
@@ -170,6 +174,8 @@ public class DocumentNodeStoreBuilder> {
private boolean clusterInvisible;
private boolean throttlingEnabled;
private boolean fullGCEnabled;
+private Set fullGCIncludePaths = of();
+private Set fullGCExcludePaths = of();
Review Comment:
fixed in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/55f310e2d65cfe3208ff7688ce93787eccb0cfba
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
+"Any path which is added to both include & exclude paths,
" +
+"would be removed from included paths." +
+"Note that this value can be overridden with a system
property " +
+"'oak.documentstore.fullGCIncludes' where paths " +
+"are separated with '::'. Example:
-Doak.documentstore.fullGCIncludes=/content::/var")
+String[] fullGCIncludes() default {};
+
+@AttributeDefinition(
+name = "Full GC Exclude Paths",
+description = "Paths which should be excluded from full Garbage
collection." +
+"Empty value means no paths are excluded." +
+"Any path added to excluded list would be removed from
include paths (if present)." +
+"Note that this value can be overridden with a system
property " +
+"'oak.documentstore.fullGCExcludes' where paths " +
+"are separated with '::'. Example:
-Doak.documentstore.fullGCExcludes=/content::/var")
+String[] fullGCExcludes() default {};
Review Comment:
fixed in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/55f310e2d65cfe3208ff7688ce93787eccb0cfba
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
+"Any path which is added to both include & exclude paths,
" +
+"would be removed from included paths." +
+"Note that this value can be overridden with a system
property " +
+"'oak.documentstore.fullGCIncludes' where paths " +
+"are separated with '::'. Example:
-Doak.documentstore.fullGCIncludes=/content::/var")
Review Comment:
fixed in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/55f310e2d65cfe3208ff7688ce93787eccb0cfba
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650825750
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
+"Any path which is added to both include & exclude paths,
" +
+"would be removed from included paths." +
+"Note that this value can be overridden with a system
property " +
+"'oak.documentstore.fullGCIncludes' where paths " +
+"are separated with '::'. Example:
-Doak.documentstore.fullGCIncludes=/content::/var")
Review Comment:
This magic happens here:
https://github.com/apache/jackrabbit-oak/blob/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfiguration.java#L123
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10748: Improve statistics to collect which type of garbage is sent/deleted [jackrabbit-oak]
stefan-egli commented on PR #1543: URL: https://github.com/apache/jackrabbit-oak/pull/1543#issuecomment-2186287106 actually, there are compilation errors: ``` [INFO] - Error: COMPILATION ERROR : [INFO] - Error: /home/runner/work/jackrabbit-oak/jackrabbit-oak/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:[367,11] setStatisticsProvider(org.apache.jackrabbit.oak.stats.StatisticsProvider) is not public in org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector; cannot be accessed from outside package Error: /home/runner/work/jackrabbit-oak/jackrabbit-oak/oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:[538,11] setStatisticsProvider(org.apache.jackrabbit.oak.stats.StatisticsProvider) is not public in org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector; cannot be accessed from outside package [INFO] 2 errors ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10748: Improve statistics to collect which type of garbage is sent/deleted [jackrabbit-oak]
stefan-egli commented on code in PR #1543:
URL: https://github.com/apache/jackrabbit-oak/pull/1543#discussion_r1650820289
##
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##
@@ -535,8 +544,8 @@ private void collectDocument(RevisionsOptions options,
Closer closer, String pat
}
gc.collectGarbageOnDocument(documentNodeStore, workingDocument,
options.isVerbose());
-//TODO: Probably we should output some details of fullGCStats. Could
be done after OAK-10378
-//gc.getFullGCStats();
+System.out.println("Full GC Stats:");
+System.out.println(gc.getFullGCStatsReport());
Review Comment:
just curious : above the report is indented with 4 spaces but not here - is
this on purpose?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
stefan-egli commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650776449
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
Review Comment:
I find this potentially confusing.
Currently when no include path is specified, it results in "include
everything" - so specifying an "include everything" explicitly wouldn't be
necessary. But if we wanted to support this, then I think it shouldn't be an
empty string, but rather eg `"/"` as that is more explicit (so if someone would
specify an empty string, we could rather log a warn and skip that entry).
wdyt?
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
Review Comment:
formatting is broken as some spaces are missing after dot
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
+"Any path which is added to both include & exclude paths,
" +
+"would be removed from included paths." +
+"Note that this value can be overridden with a system
property " +
+"'oak.documentstore.fullGCIncludes' where paths " +
+"are separated with '::'. Example:
-Doak.documentstore.fullGCIncludes=/content::/var")
Review Comment:
I see we sometimes state it can be overwritten via `-Doak.mongo` and
sometimes `-Doak.documentstore` ... does it have this magic indeed or is one or
the other wrong?
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Configuration.java:
##
@@ -260,6 +260,27 @@
"are separated with '::'. Example:
-Doak.documentstore.persistentCacheIncludes=/content::/var")
String[] persistentCacheIncludes() default {"/"};
+@AttributeDefinition(
+name = "Full GC Include Paths",
+description = "Paths which should be included in full garbage
collection." +
+"Empty value means all paths are included. " +
+"Any path which is added to both include & exclude paths,
" +
+"would be removed from included paths." +
Review Comment:
Not sure we need to point out someone configuring duplicate paths
specifically.
What about making this comment more generic into something like "Include and
exclude paths can overlap. Exclude paths will take precedence" ?
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java:
##
@@ -499,6 +499,8 @@ private void configureBuilder(DocumentNodeStoreBuilder
builder) {
setDocStoreEmbeddedVerificationFeature(docStoreEmbeddedVerificationFeature).
setThrottlingEnabled(config.throttlingEnabled()).
setFullGCEnabled(config.fullGCEnabled()).
+setFullGCIncludePaths(config.fullGCIncludes()).
Review Comment:
Configuring the identical path in both include and exclude seems weird - but
the code does behaves correctly, namely exclude has precedence, so it's "as if
that path wasn't there". But I don't think we need to add an extra warn for
this ..
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBuilder.java:
##
@@ -170,6 +174,8 @@ public class DocumentNodeStoreBuilder> {
private boolean clusterInvisible;
private boolean throttlingEnabled;
private boolean fullGCEnabled;
+private Set fullGCIncludePaths = of();
+private Set fullGCExcludePaths = of();
Review Comment:
btw, just a side-comment, I
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user rishabhdaim (triggered by rishabhdaim). Head commit for run: 167b46c5eaa518d39bad952853ac55bed2e0b7a5 / Rishabh Kumar OAK-10912 : unit cases for invalid paths Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9643283014 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user rishabhdaim (triggered by rishabhdaim). Head commit for run: 772c8559d4fdfa7e9fd8d251272f18606325ca04 / Rishabh Kumar OAK-10912 : removed the inValid paths Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9643206692 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
stefan-egli commented on PR #1539: URL: https://github.com/apache/jackrabbit-oak/pull/1539#issuecomment-2186194768 PS: I think we should adjust jira/pr titles to talk about osgi config instead of feature toggle (to avoid future confusion) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
stefan-egli commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1650729569
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -692,7 +694,7 @@ public int getMemory() {
diffCache = builder.getDiffCache(this.clusterId);
// check if root node exists
-NodeDocument rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
+NodeDocument rootDoc = store.find(NODES, getIdFromPath(ROOT));
Review Comment:
I think we should try to avoid making unrelated changes like these, they
make the PR change surface unnecessarily larger (and in this particular case I
don't think it is an improvement - but I guess that is a controversial topic)
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -716,7 +718,7 @@ public int getMemory() {
setRoot(head);
// make sure _lastRev is written back to store
backgroundWrite();
-rootDoc = store.find(NODES, Utils.getIdFromPath(ROOT));
+rootDoc = store.find(NODES, getIdFromPath(ROOT));
Review Comment:
(same as above)
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java:
##
@@ -238,6 +239,8 @@ static DocumentStoreType fromString(String type) {
private Feature cancelInvalidationFeature;
private Feature docStoreFullGCFeature;
private Feature docStoreEmbeddedVerificationFeature;
+private Feature docStoreFullGCModeGapOrphansFeature;
+private Feature docStoreFullGCModeEmptyPropertiesFeature;
Review Comment:
I think we aimed to remove the features here?
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitBuilderTest.java:
##
@@ -83,8 +83,8 @@ public void buildWithNullRevision() {
CommitBuilder builder = new CommitBuilder(ns, null);
try {
builder.build(null);
-expectNPE();
-} catch (NullPointerException e) {
+expectIllegalArgumentException();
+} catch (IllegalArgumentException e) {
Review Comment:
it fails with a NPE for me though
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java:
##
@@ -531,6 +531,57 @@ public void testResetFullGCDryRunMode() throws Exception {
// OAK-10370 END
+// OAK-10896
+
+@Test
+public void testVersionGCLoadGCModeConfigurationNotApplicable() throws
Exception {
+int fullGcModeNotAllowedValue = 5;
+int fullGcModeGapOrphans = 2;
+
+// set fullGcMode to allowed value that is different than NONE
+VersionGarbageCollector.setFullGcMode(fullGcModeGapOrphans);
+
+// reinitialize VersionGarbageCollector with not allowed value
+VersionGarbageCollector gc = new VersionGarbageCollector(
+ns, new VersionGCSupport(store), true, false, false,
+fullGcModeNotAllowedValue);
+
+assertEquals("Starting VersionGarbageCollector with not applicable /
not allowed value" +
+"will set fullGcMode to default NONE", gc.getFullGcMode(),
VersionGarbageCollector.FullGCMode.NONE);
Review Comment:
```suggestion
"will set fullGcMode to default NONE",
VersionGarbageCollector.getFullGcMode(),
VersionGarbageCollector.FullGCMode.NONE);
```
static method should be accessed in a static way
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java:
##
@@ -2603,7 +2605,7 @@ private void cleanOrphanedBranches() {
}
private void cleanRootCollisions() {
-String id = Utils.getIdFromPath(ROOT);
+String id = getIdFromPath(ROOT);
Review Comment:
(same as above throughout this class)
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##
@@ -1092,61 +1124,63 @@ public void collectGarbage(final NodeDocument doc,
final GCPhases phases) {
}
if (!isDeletedOrOrphanedNode(traversedState,
greatestExistingAncestorOrSelf, phases, doc)) {
+
// here the node is not orphaned which means that we can reach
the node from root
-switch(fullGcMode) {
-case NONE : {
+switch (fullGcMode) {
+case NONE: {
Review Comment:
still, this in unrelated and we should not increase PR change surface when
that is not necessary or part of the PR
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##
@@ -1226,7 +1260,10 @@ private boolean isDeletedOrOrphanedNode(final NodeState
traversedState, final Pa
[GH] (jackrabbit-oak): Workflow run "SonarCloud" is working again!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has succeeded. Run started by GitHub user shodaaan (triggered by stefan-egli). Head commit for run: 5d64cbaa050493ec2e7492885622ff0870974b50 / shodaaan - made requested PR changes - modified setFullGCMode method to set fullGCMode to default value if a value outside of the approved values is received through OSGI configuration - added log warn to RDBDocumentNodeStoreBuilder Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9613169104 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650721462
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentNodeStoreBuilderTest.java:
##
@@ -67,6 +68,20 @@ public void fullGCDisabled() {
assertFalse(builder.isFullGCEnabled());
}
+@Test
Review Comment:
added in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/167b46c5eaa518d39bad952853ac55bed2e0b7a5
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentNodeStoreBuilderTest.java:
##
@@ -60,6 +62,22 @@ public void fullGCFeatureToggleDisabled() {
assertNull(builder.getDocStoreFullGCFeature());
}
+@Test
Review Comment:
added in
https://github.com/apache/jackrabbit-oak/pull/1551/commits/167b46c5eaa518d39bad952853ac55bed2e0b7a5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650713278
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java:
##
@@ -115,6 +118,22 @@ public void fullGCEnabled() throws Exception {
assertEquals(fullGCDocStore, config.fullGCEnabled());
}
+@Test
Review Comment:
Check for invalid paths is added here:
https://github.com/apache/jackrabbit-oak/pull/1551/commits/772c8559d4fdfa7e9fd8d251272f18606325ca04
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650697389
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java:
##
@@ -115,6 +118,22 @@ public void fullGCEnabled() throws Exception {
assertEquals(fullGCDocStore, config.fullGCEnabled());
}
+@Test
Review Comment:
> I believe both lists should be different, with even one common path,
nothing gets fetched from DB. cc @stefan-egli
my above is wrong, only common path is excluded not all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650695069
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java:
##
@@ -499,6 +499,8 @@ private void configureBuilder(DocumentNodeStoreBuilder
builder) {
setDocStoreEmbeddedVerificationFeature(docStoreEmbeddedVerificationFeature).
setThrottlingEnabled(config.throttlingEnabled()).
setFullGCEnabled(config.fullGCEnabled()).
+setFullGCIncludePaths(config.fullGCIncludes()).
Review Comment:
I have tested on local and in case a path is present in both `include` &
`exclude`, it won't be selected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650412061
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java:
##
@@ -499,6 +499,8 @@ private void configureBuilder(DocumentNodeStoreBuilder
builder) {
setDocStoreEmbeddedVerificationFeature(docStoreEmbeddedVerificationFeature).
setThrottlingEnabled(config.throttlingEnabled()).
setFullGCEnabled(config.fullGCEnabled()).
+setFullGCIncludePaths(config.fullGCIncludes()).
Review Comment:
That's a good catch. I have tested with a path in both the `include` &
`exclude` list and it turns out that in that case, we won't be fetching any
document from db.
cc @stefan-egli
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] add service principal support while deleting container in oak-run-commons [jackrabbit-oak]
t-rana commented on code in PR #1542:
URL: https://github.com/apache/jackrabbit-oak/pull/1542#discussion_r1650371659
##
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/fixture/DataStoreUtils.java:
##
@@ -146,26 +150,52 @@ public static void deleteBucket(String bucket,
Map map, Date date) th
}
public static void deleteAzureContainer(Map config, String
containerName) throws Exception {
+if (config == null) {
+return;
Review Comment:
thanks for pointing this out, changes done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user Joscorbe (triggered by Joscorbe). Head commit for run: 9c1b96d2698c2b9ad1e76d0f74d36736f9cf2437 / José Andrés Cordero Benítez Merge branch 'trunk' into OAK-10748 Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9641724200 With regards, GitHub Actions via GitBox
[GH] (jackrabbit-oak): Workflow run "SonarCloud" failed!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has failed. Run started by GitHub user thomasmueller (triggered by thomasmueller). Head commit for run: 6e2e0791e23e924b72e03f8cc6dd846a7ed4a576 / Thomas Mueller OAK-10913 SQL-2 grammar: remove documentation for distinct Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9641432806 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10742 : add support for defining include/exclude paths for fullGC [jackrabbit-oak]
stefan-egli commented on code in PR #1547:
URL: https://github.com/apache/jackrabbit-oak/pull/1547#discussion_r1650518213
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoVersionGCSupport.java:
##
@@ -132,6 +133,56 @@ public CloseableIterable
getPossiblyDeletedDocs(final long fromMod
input -> store.convertFromDBObject(NODES, input)));
}
+/**
+ * Calculate the bson representing including only the provided
+ * include path prefixes and/or excluding the provided
+ * exclude path prefixes - if any are provided - AND the provided
+ * query.
+ * Please note that at the moment the includes do not
+ * take long paths into account. That is, if a long path was
+ * supposed to be included via an include, it is not.
+ * Reason for this is that long paths would require
+ * the mongo query to include a '_path' condition - which disallows
+ * mongo from using the '_modified_id' index. IOW long paths
+ * would result in full scans - which results in bad performance.
+ * @param includes set of path prefixes which should only be considered
+ * @param excludes set of path prefixes which should be excluded.
+ * if these overlap with includes, then exclude has precedence.
+ * @param query the query with which to do an AND
+ * @return the combined bson with include/exclude path prefixes
+ * AND the provided query
+ */
+private Bson withIncludeExcludes(Set includes, Set
excludes, Bson query) {
+Bson inclExcl = null;
+if (includes != null && !includes.isEmpty()) {
+final List ors = new ArrayList<>(includes.size());
+for (String incl : includes) {
+ors.add(Filters.regex(ID, ":" + incl));
+}
+inclExcl = or(ors);
+}
+if (excludes != null && !excludes.isEmpty()) {
+final List ands = new ArrayList<>(excludes.size());
+for (String excl : excludes) {
+ands.add(not(Filters.regex(ID, ":" + excl)));
+}
Review Comment:
Great, thanks for the heads-up @nfsantos ! I'll need to see how to fix this
(worst case would I guess be to disallow exclude paths until that's improved)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] OAK-10913 SQL-2 grammar: remove documentation for distinct [jackrabbit-oak]
thomasmueller opened a new pull request, #1552: URL: https://github.com/apache/jackrabbit-oak/pull/1552 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] OAK-10896: - Add feature toggle for removal of deleted properties and orphaned nodes [jackrabbit-oak]
shodaaan commented on code in PR #1539:
URL: https://github.com/apache/jackrabbit-oak/pull/1539#discussion_r1650451430
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCTest.java:
##
@@ -531,6 +531,51 @@ public void testResetFullGCDryRunMode() throws Exception {
// OAK-10370 END
+// OAK-10896
+
+@Test
+public void testVersionGCLoadGCModeConfigurationNotApplicable() throws
Exception {
+int FULL_GC_MODE_NOT_ALLOWED_VALUE = 5;
Review Comment:
Fixed, thank you for the comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GH] (jackrabbit-oak): Workflow run "SonarCloud" is working again!
The GitHub Actions job "SonarCloud" on jackrabbit-oak.git has succeeded. Run started by GitHub user nfsantos (triggered by nfsantos). Head commit for run: 32ef714ab78c4e6d141c1bc80d12b83e75c864cc / Nuno Santos Merge remote-tracking branch 'upstream/trunk' into OAK-10903 Report URL: https://github.com/apache/jackrabbit-oak/actions/runs/9640930049 With regards, GitHub Actions via GitBox
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650412889
##
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreServiceConfigurationTest.java:
##
@@ -115,6 +118,22 @@ public void fullGCEnabled() throws Exception {
assertEquals(fullGCDocStore, config.fullGCEnabled());
}
+@Test
Review Comment:
I believe both lists should be different, with even one common path, nothing
gets fetched from DB.
cc @stefan-egli
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] OAK-10912 : make fullGc include/exclude paths configurable via OSGI config [jackrabbit-oak]
rishabhdaim commented on code in PR #1551:
URL: https://github.com/apache/jackrabbit-oak/pull/1551#discussion_r1650412061
##
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java:
##
@@ -499,6 +499,8 @@ private void configureBuilder(DocumentNodeStoreBuilder
builder) {
setDocStoreEmbeddedVerificationFeature(docStoreEmbeddedVerificationFeature).
setThrottlingEnabled(config.throttlingEnabled()).
setFullGCEnabled(config.fullGCEnabled()).
+setFullGCIncludePaths(config.fullGCIncludes()).
Review Comment:
That's a good catch. I have tested with a path in both the `include` &
`exclude` list and it turns out that in that case, we won't be fetching any
document from db.
cc @stefan-egli
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
