You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by nf...@apache.org on 2024/02/06 08:11:11 UTC
(jackrabbit-oak) branch trunk updated: OAK-10637 - If downloading the root directory, do not add a filter to the Mongo query. (#1293)
This is an automated email from the ASF dual-hosted git repository.
nfsantos pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 1cb0962e1e OAK-10637 - If downloading the root directory, do not add a filter to the Mongo query. (#1293)
1cb0962e1e is described below
commit 1cb0962e1e21b2f3a7a962c871d2da401afa9526
Author: Nuno Santos <ns...@adobe.com>
AuthorDate: Tue Feb 6 09:11:04 2024 +0100
OAK-10637 - If downloading the root directory, do not add a filter to the Mongo query. (#1293)
---
.../pipelined/PipelinedMongoDownloadTask.java | 36 ++++++----
.../document/flatfile/pipelined/PipelinedIT.java | 19 +++++
.../pipelined/PipelinedMongoDownloadTaskTest.java | 82 ++++++++++++++++++++++
3 files changed, 122 insertions(+), 15 deletions(-)
diff --git a/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTask.java b/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTask.java
index 03f741902a..aeac8ba348 100644
--- a/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTask.java
+++ b/oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTask.java
@@ -151,21 +151,22 @@ public class PipelinedMongoDownloadTask implements Callable<PipelinedMongoDownlo
* @return The filter to be used in the Mongo query, or null if no filter is required
*/
static Bson computeMongoQueryFilter(@NotNull MongoFilterPaths mongoFilterPaths, String customExcludeEntriesRegex, boolean queryUsesIndexTraversal) {
- var filters = new ArrayList<Bson>(4);
- if (mongoFilterPaths != MongoFilterPaths.DOWNLOAD_ALL) {
- filters.add(descendantsFilter(mongoFilterPaths.included, queryUsesIndexTraversal));
- if (!mongoFilterPaths.excluded.isEmpty()) {
- // The Mongo filter returned here will download the top level path of each excluded subtree, which in theory
- // should be excluded. That is, if the tree /a/b/c is excluded, the filter will download /a/b/c but none of
- // its descendants.
- // This is done because excluding also the top level path would add extra complexity to the filter and
- // would not have any measurable impact on performance because it only downloads a few extra documents, one
- // for each excluded subtree. The transform stage will anyway filter out these paths.
- Bson excludedFilter = descendantsFilter(mongoFilterPaths.excluded, queryUsesIndexTraversal);
- if (excludedFilter != null) {
- filters.add(Filters.nor(excludedFilter));
- }
- }
+ var filters = new ArrayList<Bson>();
+
+ Bson includedFilter = descendantsFilter(mongoFilterPaths.included, queryUsesIndexTraversal);
+ if (includedFilter != null) {
+ filters.add(includedFilter);
+ }
+
+ // The Mongo filter returned here will download the top level path of each excluded subtree, which in theory
+ // should be excluded. That is, if the tree /a/b/c is excluded, the filter will download /a/b/c but none of
+ // its descendants.
+ // This is done because excluding also the top level path would add extra complexity to the filter and
+ // would not have any measurable impact on performance because it only downloads a few extra documents, one
+ // for each excluded subtree. The transform stage will anyway filter out these paths.
+ Bson excludedFilter = descendantsFilter(mongoFilterPaths.excluded, queryUsesIndexTraversal);
+ if (excludedFilter != null) {
+ filters.add(Filters.nor(excludedFilter));
}
// Custom regex filter to exclude paths
@@ -173,6 +174,7 @@ public class PipelinedMongoDownloadTask implements Callable<PipelinedMongoDownlo
if (customExcludedPathsFilter != null) {
filters.add(customExcludedPathsFilter);
}
+
if (filters.isEmpty()) {
return null;
} else if (filters.size() == 1) {
@@ -198,6 +200,10 @@ public class PipelinedMongoDownloadTask implements Callable<PipelinedMongoDownlo
if (paths.isEmpty()) {
return null;
}
+ if (paths.size() == 1 && paths.get(0).equals("/")) {
+ return null;
+ }
+
// The filter for descendants of a list of paths is a series of or conditions. For each path, we have to build
// two conditions in two different fields of the documents:
// _ _id - for non-long paths - In this case, the _id is of the form "2:/foo/bar"
diff --git a/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedIT.java b/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedIT.java
index 582d7de729..b7f0f7811d 100644
--- a/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedIT.java
+++ b/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedIT.java
@@ -237,6 +237,25 @@ public class PipelinedIT {
));
}
+ @Test
+ public void createFFS_mongoFiltering_include_excludes_retryOnConnectionErrors() throws Exception {
+ System.setProperty(OAK_INDEXER_PIPELINED_RETRY_ON_CONNECTION_ERRORS, "true");
+ System.setProperty(OAK_INDEXER_PIPELINED_MONGO_REGEX_PATH_FILTERING, "true");
+
+ Predicate<String> pathPredicate = s -> true;
+
+ List<PathFilter> pathFilters = List.of(new PathFilter(List.of("/"), List.of("/content/dam", "/etc", "/home", "/jcr:system")));
+
+ testSuccessfulDownload(pathPredicate, pathFilters, List.of(
+ "/|{}",
+ "/content|{}",
+ "/content/dam|{}",
+ "/etc|{}",
+ "/home|{}",
+ "/jcr:system|{}"
+ ));
+ }
+
@Test
public void createFFS_mongoFiltering_include_excludes4() throws Exception {
System.setProperty(OAK_INDEXER_PIPELINED_RETRY_ON_CONNECTION_ERRORS, "false");
diff --git a/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTaskTest.java b/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTaskTest.java
index 488986caba..46060b7985 100644
--- a/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTaskTest.java
+++ b/oak-run-commons/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/pipelined/PipelinedMongoDownloadTaskTest.java
@@ -327,6 +327,32 @@ public class PipelinedMongoDownloadTaskTest {
);
}
+ @Test
+ public void computeMongoQueryFilterOptimizeWhenIncludedPathIsRoot() {
+ // Path filter but no exclude filter
+ var actual = PipelinedMongoDownloadTask.computeMongoQueryFilter(
+ new MongoFilterPaths(List.of("/"), List.of("/excluded1", "/content/excluded2")),
+ null,
+ true
+ );
+ // The generated filter should not include any condition to include the descendants of /
+ var expected = Filters.nor(
+ Filters.or(
+ Filters.in(NodeDocument.ID,
+ Pattern.compile("^[0-9]{1,3}:" + Pattern.quote("/excluded1/") + ".*$"),
+ Pattern.compile("^[0-9]{1,3}:" + Pattern.quote("/content/excluded2/") + ".*$")),
+ Filters.and(
+ Filters.regex(NodeDocument.ID, PipelinedMongoDownloadTask.LONG_PATH_ID_PATTERN),
+ Filters.in(NodeDocument.PATH,
+ Pattern.compile("^" + Pattern.quote("/excluded1/") + ".*$"),
+ Pattern.compile("^" + Pattern.quote("/content/excluded2/") + ".*$"))
+ )
+ )
+ );
+ assertBsonEquals(expected, actual);
+ }
+
+
@Test
public void computeMongoQueryFilterWithPathFilterNoExcludeFilter() {
// Path filter but no exclude filter
@@ -417,6 +443,62 @@ public class PipelinedMongoDownloadTaskTest {
assertBsonEquals(expected, actual);
}
+ @Test
+ public void computeMongoQueryFilterWithPathFilterWithExcludeFilterAndNaturalColumnTraversal() {
+ var actual = PipelinedMongoDownloadTask.computeMongoQueryFilter(
+ new MongoFilterPaths(List.of("/"), List.of("/excluded")),
+ "^[0-9]{1,3}:/a/b.*$",
+ false
+ );
+
+ Pattern excludePattern = Pattern.compile("^[0-9]{1,3}:/a/b.*$");
+ var expected =
+ Filters.and(
+ Filters.nor(
+ Filters.or(
+ Filters.in(NodeDocument.ID, Pattern.compile("^[0-9]{1,3}:" + Pattern.quote("/excluded/") + ".*$")),
+ Filters.in(NodeDocument.PATH, Pattern.compile("^" + Pattern.quote("/excluded/") + ".*$"))
+ )
+ ),
+ Filters.nor(
+ Filters.regex(NodeDocument.ID, excludePattern),
+ Filters.in(NodeDocument.PATH, excludePattern)
+ )
+ );
+ assertBsonEquals(expected, actual);
+ }
+
+ @Test
+ public void computeMongoQueryFilterWithPathFilterWithExcludeFilterAndNaturalIndexTraversal() {
+ var actual = PipelinedMongoDownloadTask.computeMongoQueryFilter(
+ new MongoFilterPaths(List.of("/"), List.of("/excluded")),
+ "^[0-9]{1,3}:/a/b.*$",
+ true
+ );
+
+ Pattern excludePattern = Pattern.compile("^[0-9]{1,3}:/a/b.*$");
+ var expected =
+ Filters.and(
+ Filters.nor(
+ Filters.or(
+ Filters.in(NodeDocument.ID, Pattern.compile("^[0-9]{1,3}:" + Pattern.quote("/excluded/") + ".*$")),
+ Filters.and(
+ Filters.regex(NodeDocument.ID, PipelinedMongoDownloadTask.LONG_PATH_ID_PATTERN),
+ Filters.in(NodeDocument.PATH, Pattern.compile("^" + Pattern.quote("/excluded/") + ".*$"))
+ )
+ )
+ ),
+ Filters.nor(
+ Filters.regex(NodeDocument.ID, excludePattern),
+ Filters.and(
+ Filters.regex(NodeDocument.ID, PipelinedMongoDownloadTask.LONG_PATH_ID_PATTERN),
+ Filters.in(NodeDocument.PATH, excludePattern)
+ )
+ )
+ );
+ assertBsonEquals(expected, actual);
+ }
+
private void assertBsonEquals(Bson actual, Bson expected) {
if (actual == null && expected == null) {
return;