nfsantos commented on code in PR #639:
URL: https://github.com/apache/jackrabbit-oak/pull/639#discussion_r929866300
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResultRowAsyncIterator.java:
##########
@@ -111,6 +111,21 @@ public boolean hasNext() {
throw new IllegalStateException("Error reading next result
from Elastic", e);
}
}
+
+ // Check if there are any Throwable filled from onFailure Callback in
the errorReference
+ // Any exception (such as ParseException) during the prefetch (init
scanner) via the async call to ES would be available here
+ // when the cursor is actually being traversed.
+ // This is being done so that we can log the caller stack trace in
case of any exception from ES and not just the trace of the async query thread.
+
+ Throwable error = errorRef.get();
Review Comment:
The `errorRef` is not cleared by this call to `get`. Is there the risk that
this method will be called again and it will log the same exception several
times? Maybe use `getAndSet(null)` to ensure we do not log more than once.
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResultRowAsyncIterator.java:
##########
@@ -323,7 +338,8 @@ public void onSuccess(SearchResponse<ObjectNode>
searchResponse) {
public void onFailure(Throwable t) {
metricHandler.measureFailedQuery(indexNode.getDefinition().getIndexPath(),
System.currentTimeMillis() - searchStartTime);
- LOG.error("Error retrieving data from Elastic: closing scanner,
notifying listeners", t);
+ errorRef.set(t);
Review Comment:
This is not checking if errorRef is already set. In that case, the exception
stored there will be lost. If we assume that onFailure is called only once,
then it's fine (although we may want to log a warning in case errorRef is
already set, `getAndSet()` could once again be useful here). But if the
assumption is that it can be called multiple times, then this can lead to
losing exceptions.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
