nfsantos commented on code in PR #2442:
URL: https://github.com/apache/jackrabbit-oak/pull/2442#discussion_r2275585732
##########
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/facets/ElasticStatisticalFacetAsyncProvider.java:
##########
@@ -96,87 +106,120 @@ public class ElasticStatisticalFacetAsyncProvider
implements ElasticFacetProvide
)
);
- LOG.trace("Kicking search query with random sampling {}",
searchRequest);
- CompletableFuture<SearchResponse<ObjectNode>> searchFuture =
- connection.getAsyncClient().search(searchRequest,
ObjectNode.class);
-
- searchFuture.whenCompleteAsync((searchResponse, throwable) -> {
- try {
- if (throwable != null) {
- LOG.error("Error while retrieving sample documents. Search
request: {}", searchRequest, throwable);
- } else {
- List<Hit<ObjectNode>> searchHits =
searchResponse.hits().hits();
- this.sampled = searchHits != null ? searchHits.size() : 0;
- if (sampled > 0) {
- this.totalHits = searchResponse.hits().total().value();
- processAggregations(searchResponse.aggregations());
- searchResponse.hits().hits().forEach(this::processHit);
- computeStatisticalFacets();
- }
- }
- } finally {
- latch.countDown();
- }
- });
+ this.queryStartTimeNanos = System.nanoTime();
+ LOG.trace("Kicking search query with random sampling {}",
searchRequest, new Throwable());
+ this.searchFuture = connection.getAsyncClient()
+ .search(searchRequest, ObjectNode.class)
+ .thenApplyAsync(this::computeFacets);
}
@Override
public List<FulltextIndex.Facet> getFacets(int numberOfFacets, String
columnName) {
- LOG.trace("Requested facets for {} - Latch count: {}", columnName,
latch.getCount());
- try {
- boolean completed = latch.await(facetsEvaluationTimeoutMs,
TimeUnit.MILLISECONDS);
- if (!completed) {
- LOG.error("Timed out while waiting for facets. Search request:
{}", searchRequest);
- throw new IllegalStateException("Timed out while waiting for
facets");
+ // TODO: In case of failure, we log an exception and return null. This
is likely not the ideal behavior, as the
+ // caller has no way to distinguish between a failure and empty
results. But in this PR I'm leaving this
+ // behavior as is to not introduce further changes. We should revise
this behavior once the queries for facets
+ // are decoupled from the query for results, as this will make it
easier to better handle errors
+ if (!searchFuture.isDone()) {
+ try {
+ LOG.trace("Requested facets for {}. Waiting up to: {}",
columnName, facetsEvaluationTimeoutMs);
+ long start = System.nanoTime();
+ facets = searchFuture.get(facetsEvaluationTimeoutMs,
TimeUnit.MILLISECONDS);
+ LOG.trace("Facets computed in {}.",
TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start));
+ } catch (ExecutionException e) {
+ LOG.error("Error evaluating facets", e);
+ } catch (TimeoutException e) {
+ searchFuture.cancel(true);
+ LOG.error("Timed out while waiting for facets. Search request:
{}. {}", searchRequest, timingsToString());
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt(); // restore interrupt
status
+ throw new IllegalStateException("Error while waiting for
facets", e);
}
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt(); // restore interrupt status
- throw new IllegalStateException("Error while waiting for facets",
e);
}
LOG.trace("Reading facets for {} from {}", columnName, facets);
String field =
ElasticIndexUtils.fieldName(FulltextIndex.parseFacetField(columnName));
return facets != null ? facets.get(field) : null;
}
- private void processHit(Hit<ObjectNode> searchHit) {
- final String path = elasticResponseHandler.getPath(searchHit);
- if (path != null && isAccessible.test(path)) {
- for (String field : facetFields) {
- JsonNode value = searchHit.source().get(field);
- if (value != null) {
- accessibleFacetCounts.compute(field, (column, facetValues)
-> {
- if (facetValues == null) {
- Map<String, MutableInt> values = new HashMap<>();
- values.put(value.asText(), new MutableInt(1));
- return values;
- } else {
- facetValues.compute(value.asText(), (k, v) -> {
- if (v == null) {
- return new MutableInt(1);
- } else {
- v.increment();
- return v;
- }
- });
- return facetValues;
- }
- });
- }
+ private Map<String, List<FulltextIndex.Facet>>
computeFacets(SearchResponse<ObjectNode> searchResponse) {
+ LOG.info("SearchResponse: {}", searchResponse);
Review Comment:
Set level to TRACE.
--
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]
