# Highlighters Executed Twice

**URL:** <https://discuss.elastic.co/t/highlighters-executed-twice/100080>\
**Category:** Elasticsearch\
**Created:** [September 11, 2017, 3:17pm UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080 "2017-09-11T15:17:27Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![mkrauklis](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/mkrauklis/32/17839_2.png) [@mkrauklis](https://discuss.elastic.co/u/mkrauklis)\
**Post date:** [September 11, 2017, 3:17pm UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080/1 "2017-09-11T15:17:27Z")

</div>

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](https://github.com/elastic/elasticsearch/blob/master/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorPlugin.java)

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

```

I can see this in FetchPhase.fetchSubPhases:  
 ![image](https://us1.discourse-cdn.com/elastic/original/3X/e/4/e408d54bd25f415114b5063f398f244cb644c45b.png)

FetchPhase.execute iterates over each fetchSubPhase and calls hitExecute:

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

```

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

```java
    @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 hit **s** Execute on each sub-phase:

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

```

PercolatorHighlightSubFetchPhase does override hit **s** Execute and the first thing it does is verify the query is some sort of percolate query:  
[PercolatorHighlightSubFetchPhase.java](https://github.com/elastic/elasticsearch/blob/master/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java)

```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;
        }
...
    }

```

```java
    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.

---

<div class="post-metadata">

**Author:** ![jimczi](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/jimczi/32/47985_2.png) [@jimczi](https://discuss.elastic.co/u/jimczi)\
**Post date:** [September 13, 2017, 9:35am UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080/2 "2017-09-13T09:35:51Z")

</div>

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:

> <https://github.com/elastic/elasticsearch/pull/26622>

---

<div class="post-metadata">

**Author:** ![mkrauklis](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/mkrauklis/32/17839_2.png) [@mkrauklis](https://discuss.elastic.co/u/mkrauklis)\
**Post date:** [September 13, 2017, 2:05pm UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080/3 "2017-09-13T14:05:48Z")

</div>

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?

---

<div class="post-metadata">

**Author:** ![jimczi](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/jimczi/32/47985_2.png) [@jimczi](https://discuss.elastic.co/u/jimczi)\
**Post date:** [September 13, 2017, 2:20pm UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080/4 "2017-09-13T14:20:29Z")

</div>

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](https://github.com/elastic/elasticsearch/pull/26622) in the plugin. Then you can remove the percolator module and use your plugin instead.

---

<div class="post-metadata">

**Author:** ![mkrauklis](https://sea2.discourse-cdn.com/elastic/user_avatar/discuss.elastic.co/mkrauklis/32/17839_2.png) [@mkrauklis](https://discuss.elastic.co/u/mkrauklis)\
**Post date:** [September 13, 2017, 3:16pm UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080/5 "2017-09-13T15:16:21Z")

</div>

I figured not but had to ask. Thanks again, Jim 👍

---

<div class="post-metadata">

**Author:** ![system](https://us1.discourse-cdn.com/elastic/original/3X/1/a/1ac57faf039f6b580b3f104ef42a2a89e41014de.png) [@system](https://discuss.elastic.co/u/system)\
**Post date:** [October 11, 2017, 3:16pm UTC](https://discuss.elastic.co/t/highlighters-executed-twice/100080/6 "2017-10-11T15:16:39Z")

</div>

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