Highlighters Executed Twice


(Michael Krauklis) #1

We're running ES 5.1.1 and are using a custom highlighter, although we've tested and this effects the plain highlighter (and I would imagine any highlighter) as well.

In short, it would appear our highlighter is being called once by HighlightPhase.hitExecute and once by PercolatorHighlightSubFetchPhase(HighlightPhase).hitExecute, even though we're not doing any Percolate queries (nor do we have any registered).

Detail
The PercolatorPlugin registers the percolatorHighlightSubFetchPhase on ES initialization:
PercolatorPlugin.java

    @Override
    public List<FetchSubPhase> getFetchSubPhases(FetchPhaseConstructionContext context) {
        return singletonList(new PercolatorHighlightSubFetchPhase(settings, context.getHighlighters()));
    }

I can see this in FetchPhase.fetchSubPhases:
image

FetchPhase.execute iterates over each fetchSubPhase and calls hitExecute:

            for (FetchSubPhase fetchSubPhase : fetchSubPhases) {
                fetchSubPhase.hitExecute(context, hitContext);
            }

For both HighlightPhase and PercolatorHighlightSubFetchPhase (which does not override HighlightPhase.hitExecute) the highlighter is executed:

    @Override
    public void hitExecute(SearchContext context, HitContext hitContext) {
        if (context.highlight() == null) {
            return;
        }
...
                HighlightField highlightField = highlighter.highlight(highlighterContext);
                if (highlightField != null) {
                    highlightFields.put(highlightField.name(), highlightField);
                }
            }
        }
        hitContext.hit().highlightFields(highlightFields);
    }

Finally FetchPhase.execute calls hitsExecute on each sub-phase:

        for (FetchSubPhase fetchSubPhase : fetchSubPhases) {
            fetchSubPhase.hitsExecute(context, hits);
        }

PercolatorHighlightSubFetchPhase does override hitsExecute and the first thing it does is verify the query is some sort of percolate query:
PercolatorHighlightSubFetchPhase.java

    boolean hitsExecutionNeeded(SearchContext context) { // for testing
        return context.highlight() != null && locatePercolatorQuery(context.query()).isEmpty() == false;
    }

    @Override
    public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOException {
        if (hitsExecutionNeeded(context) == false) {
            return;
        }
...
    }
    static List<PercolateQuery> locatePercolatorQuery(Query query) {
        if (query instanceof PercolateQuery) {
            return Collections.singletonList((PercolateQuery) query);
        } else if (query instanceof BooleanQuery) {
            List<PercolateQuery> percolateQueries = new ArrayList<>();
            for (BooleanClause clause : ((BooleanQuery) query).clauses()) {
                List<PercolateQuery> result = locatePercolatorQuery(clause.getQuery());
                if (result.isEmpty() == false) {
                    percolateQueries.addAll(result);
                }
            }
            return percolateQueries;
        } else if (query instanceof DisjunctionMaxQuery) {
            List<PercolateQuery> percolateQueries = new ArrayList<>();
            for (Query disjunct : ((DisjunctionMaxQuery) query).getDisjuncts()) {
                List<PercolateQuery> result = locatePercolatorQuery(disjunct);
                if (result.isEmpty() == false) {
                    percolateQueries.addAll(result);
                }
            }
            return  percolateQueries;
        } else if (query instanceof ConstantScoreQuery) {
            return locatePercolatorQuery(((ConstantScoreQuery) query).getQuery());
        } else if (query instanceof BoostQuery) {
            return locatePercolatorQuery(((BoostQuery) query).getQuery());
        } else if (query instanceof FunctionScoreQuery) {
            return locatePercolatorQuery(((FunctionScoreQuery) query).getSubQuery());
        }
        return Collections.emptyList();
    }

Ultimately InternalSearchHit.highlightFields gets overwritten so I don't believe there's any functional impact, but we notice a performance hit.

I'm hoping I'm missing something here or there's a way to turn off PercolatorHighlightSubFetchPhase that we're not seeing.

Edit: Temporary Solution
We've just removed the percolator module (es/modules/percolator) such that the PercolatorPlugin (and subsequently the relevant Fetch-Sub-Phase) is never registered. Temporary in that we can no longer run percolate-queries.

Thanks,
Mike K.


(Jimferenczi) #2

Thanks for the great reporting. This is indeed a bug, introduced in 5.0, I opened a PR to fix this since you cannot just disable the PercolatorHighlightSubFetchPhase:


(Michael Krauklis) #3

Thanks Jim, I appreciate the quick turnaround. There isn't going to be a 5.1.3 release, by chance, that might include this fix is there?


(Jimferenczi) #4

Nope the next possible release for 5.x are 5.5.4 and 5.6.1. The other workaround is to copy the module from es in a separate plugin and to apply this fix https://github.com/elastic/elasticsearch/pull/26622 in the plugin. Then you can remove the percolator module and use your plugin instead.


(Michael Krauklis) #5

I figured not but had to ask. Thanks again, Jim :+1:


(system) #6

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.