[
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16836296#comment-16836296
]
Thomas Mueller edited comment on SLING-8407 at 5/9/19 1:17 PM:
---------------------------------------------------------------
Patch for sling-org-apache-sling-event:
{noformat}
diff --git a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
index 279d5ca..7faf515 100644
--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,7 @@ public class JobManagerImpl
buf.append(Job.PROPERTY_JOB_CREATED);
buf.append(" ascending");
}
+ buf.append(" option(traversal fail)");
final Iterator<Resource> iter =
resolver.findResources(buf.toString(), "xpath");
long count = 0;
@@ -535,7 +536,11 @@ public class JobManagerImpl
}
}
} catch (final QuerySyntaxException qse) {
- logger.warn("Query syntax wrong " + buf.toString(), qse);
+ // no index is available
+ String message = "Query " + buf.toString() + " failed: " +
qse.getMessage();
+ logger.debug(message, qse);
+ logger.info(message);
+ throw new IllegalStateException(message);
} finally {
resolver.close();
}
diff --git
a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
index c51ecd9..42af6e8 100644
--- a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
+++ b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
@@ -98,8 +98,8 @@ public abstract class AbstractJobHandlingTest {
String localRepo = System.getProperty("maven.repo.local", "");
- final String jackrabbitVersion = "2.13.1";
- final String oakVersion = "1.5.7";
+ final String jackrabbitVersion = "2.14.6";
+ final String oakVersion = "1.6.0";
final String slingHome = new File(buildDir + File.separatorChar +
"sling_" + System.currentTimeMillis()).getAbsolutePath();
@@ -230,6 +230,7 @@ public abstract class AbstractJobHandlingTest {
mavenBundle("com.google.guava", "guava", "15.0"),
mavenBundle("org.apache.jackrabbit", "jackrabbit-api",
jackrabbitVersion),
+ mavenBundle("org.apache.jackrabbit", "jackrabbit-data",
jackrabbitVersion),
mavenBundle("org.apache.jackrabbit", "jackrabbit-jcr-commons",
jackrabbitVersion),
mavenBundle("org.apache.jackrabbit", "jackrabbit-spi",
jackrabbitVersion),
mavenBundle("org.apache.jackrabbit", "jackrabbit-spi-commons",
jackrabbitVersion),
{noformat}
* Added "option(traversal fail)".
* If there is no index, then QuerySyntaxException is thrown (Sling converts all
kinds of query exceptions to QuerySyntaxException, even if the syntax is fine -
this is slightly confusing, but fixing that would be a backward compatibility
problem).
* This exception is logged at info level, and then converted to
IllegalStateException (a RuntimeException). So if there is no index, the
error.log will show what's going on - just that this is no longer a warning,
just an information.
* The AbstractJobHandlingTest now uses a stable Oak version instead of an
unstable one as before. This is needed because "option(traversal fail)" was
implemented slightly after Oak 1.5.7.
* The Jackrabbit version is updated as well, as that's the version needed by
the newer Oak version
* Adding "jackrabbit-data" is needed as that was added slightly after Oak 1.5.7
(part of modularization).
was (Author: tmueller):
Patch for sling-org-apache-sling-event:
{noformat}
diff --git a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
index 279d5ca..7faf515 100644
--- a/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
+++ b/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
@@ -520,6 +520,7 @@ public class JobManagerImpl
buf.append(Job.PROPERTY_JOB_CREATED);
buf.append(" ascending");
}
+ buf.append(" option(traversal fail)");
final Iterator<Resource> iter =
resolver.findResources(buf.toString(), "xpath");
long count = 0;
@@ -535,7 +536,11 @@ public class JobManagerImpl
}
}
} catch (final QuerySyntaxException qse) {
- logger.warn("Query syntax wrong " + buf.toString(), qse);
+ // no index is available
+ String message = "Query " + buf.toString() + " failed: " +
qse.getMessage();
+ logger.debug(message, qse);
+ logger.info(message);
+ throw new IllegalStateException();
} finally {
resolver.close();
}
diff --git
a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
index c51ecd9..42af6e8 100644
--- a/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
+++ b/src/test/java/org/apache/sling/event/it/AbstractJobHandlingTest.java
@@ -98,8 +98,8 @@ public abstract class AbstractJobHandlingTest {
String localRepo = System.getProperty("maven.repo.local", "");
- final String jackrabbitVersion = "2.13.1";
- final String oakVersion = "1.5.7";
+ final String jackrabbitVersion = "2.14.6";
+ final String oakVersion = "1.6.0";
final String slingHome = new File(buildDir + File.separatorChar +
"sling_" + System.currentTimeMillis()).getAbsolutePath();
@@ -230,6 +230,7 @@ public abstract class AbstractJobHandlingTest {
mavenBundle("com.google.guava", "guava", "15.0"),
mavenBundle("org.apache.jackrabbit", "jackrabbit-api",
jackrabbitVersion),
+ mavenBundle("org.apache.jackrabbit", "jackrabbit-data",
jackrabbitVersion),
mavenBundle("org.apache.jackrabbit", "jackrabbit-jcr-commons",
jackrabbitVersion),
mavenBundle("org.apache.jackrabbit", "jackrabbit-spi",
jackrabbitVersion),
mavenBundle("org.apache.jackrabbit", "jackrabbit-spi-commons",
jackrabbitVersion),
{noformat}
* Added "option(traversal fail)".
* If there is no index, then QuerySyntaxException is thrown (Sling converts all
kinds of query exceptions to QuerySyntaxException, even if the syntax is fine -
this is slightly confusing, but fixing that would be a backward compatibility
problem).
* This exception is logged at info level, and then converted to
IllegalStateException (a RuntimeException). So if there is no index, the
error.log will show what's going on - just that this is no longer a warning,
just an information.
* The AbstractJobHandlingTest now uses a stable Oak version instead of an
unstable one as before. This is needed because "option(traversal fail)" was
implemented slightly after Oak 1.5.7.
* The Jackrabbit version is updated as well, as that's the version needed by
the newer Oak version
* Adding "jackrabbit-data" is needed as that was added slightly after Oak 1.5.7
(part of modularization).
> JobManagerImpl.findJobs should prevent traversal
> ------------------------------------------------
>
> Key: SLING-8407
> URL: https://issues.apache.org/jira/browse/SLING-8407
> Project: Sling
> Issue Type: Improvement
> Components: Event
> Reporter: Thomas Mueller
> Priority: Major
>
> The method
> [JobManagerImpl.findJobs|https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java#L373]
> runs a JCR query to find all jobs for a topic.
> It is possible that such a query is running while the repository isn't
> initialized yet, meaning while the index isn't available yet. What is
> happening in this case is that the query is traversing all nodes below that
> path, triggering a warning that the query doesn't use an index. It is
> sometimes happening when a health check is running before the repository is
> initialized (ReplicationQueueHealthCheck and DistributionQueueHealthCheck).
> It doesn't make sense that the query traverses the nodes. It should use an
> index. If the index isn't available yet, it should fail. Therefore, the query
> should use "option(traversal fail)". That would result in an exception that
> can be caught. I will log a related issue to change the health checks to
> process this exception and return HEALTH_CHECK_ERROR for this case.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)