[ 
https://issues.apache.org/jira/browse/SLING-8407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16836935#comment-16836935
 ] 

Thomas Mueller commented on SLING-8407:
---------------------------------------

> therefore there is no point in throwing an exception if the query is not 
> correct back to he caller. 

This is a very surprising view. Is that approach used a lot in the code you 
wrote? 

So you are saying, if the implementation is buggy, then it's fine to swallow 
the exception, and return an empty result (in this case)? This is very 
problematic: it hides problem. People don't typically read the log file, so 
logging a warning in case of coding errors results in nobody noticing the bug 
for a long, long time (as we have seen here).

The right approach, in my view, is to throw a runtime exception. This ensures 
coding errors are detected early on, not hidden, and don't simply result in 
weird, buggy behavior.

> 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)

Reply via email to