Refactor grouping collectors [LUCENE-7701]

[asfimport]

Grouping currently works via abstract collectors, which need to be overridden for each way of defining a group - currently we have two, 'term' (based on SortedDocValues) and 'function' (based on ValueSources). These collectors all have a lot of repeated code, and means that if you want to implement your own group definitions, you need to override four or five different classes.

This would be easier to deal with if instead the 'group selection' code was abstracted out into a single interface, and the various collectors were changed to concrete implementations.

---

Migrated from LUCENE-7701 by Alan Woodward (@romseygeek), resolved Apr 10 2017
Attachments: LUCENE-7701.patch (versions: 2)
Linked issues:

• Sorting by DocValues while grouping is slower than old good FieldCache [LUCENE-9328] #10368

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Here's a patch implementing the idea. Group definitions are delegated to a GroupSelector class, with the following methods:

• void setNextReader(LeafReaderContext ctx) - called per-segment
• State advanceTo(int doc) - advance the selector to a given doc. State can be one of VALUE or SKIP
• T currentValue() - get the current group value
• T copyValue() - get a persistent copy of the current group value
• void setGroups(Collection<SearchGroup<T>> groups) - called to change the selector to 'second-pass' mode. advanceTo(int) will return SKIP if the currentValue is not in one of these groups.

There are two implementations of this, TermGroupSelector and ValueSourceGroupSelector, replacing the current term and function package Collectors.

The patch converts FirstPass, AllGroups, AllGroupHeads and SecondPassCollectors. I haven't yet worked on the DistinctValues or GroupFacet collectors, but they should be convertible in the same way.

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

This patch also refactors the second-pass collector into a concrete class that accepts a GroupReducer. GroupReducers return a Collector instance for each group that is to be reduced, so for example the TopGroupsCollector will create a TopDocsCollector for each group.

Now if you want to create a new type of group (a set of ranges over a DoubleValuesSource, for example) you just need to create a GroupSelector implementation; and if you want to create a new type of group summarizer (say some statistics over a group), then you create a GroupReducer implementation.

There's a failing test in Solr that I still need to track down, but I think this is a much nicer grouping interface. It is not at all backwards compatible, so I'd be targeting this at 7.0. cc @martijnvg

[asfimport]

Martijn van Groningen (@martijnvg) (migrated from JIRA)

Sorry for the late reply @romseygeek! I general I agree with this refactoring. It is a better design. I'll look more closely at this patch next week.

[asfimport]

Martijn van Groningen (@martijnvg) (migrated from JIRA)

+1 I think this is a good change. I agree it should 7.0 only.

[asfimport]

ASF subversion and git services (migrated from JIRA)

Commit caf3297 in lucene-solr's branch refs/heads/master from @romseygeek
https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=caf3297

LUCENE-7701: Refactor grouping collectors

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Thanks for the review Martijn!

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

Hi, @romseygeek, would you mind if I followup here? Turns out, if: 

1. group.truncate=true

2. group.sort=docvalues_enabled_field asc
   The following hotspot pops up:
   {code}
   "stackTrace":["org.apache.lucene.store.ByteBufferIndexInput.slice(ByteBufferIndexInput.java:268)",
   "org.apache.lucene.store.ByteBufferIndexInput$SingleBufferImpl.slice(ByteBufferIndexInput.java:347)",
   "org.apache.lucene.store.IndexInput.randomAccessSlice(IndexInput.java:122)",
   "org.apache.lucene.codecs.lucene80.Lucene80DocValuesProducer$TermsDict.<init>(Lucene80DocValuesProducer.java:943)",

      "org.apache.lucene.codecs.lucene80.Lucene80DocValuesProducer.getSorted(Lucene80DocValuesProducer.java:750)",
      "org.apache.lucene.codecs.perfield.PerFieldDocValuesFormat$FieldsReader.getSorted(PerFieldDocValuesFormat.java:329)",
   
      "org.apache.lucene.index.DocValues.getSorted(DocValues.java:367)",
      "org.apache.lucene.search.FieldComparator$TermOrdValComparator.getSortedDocValues(FieldComparator.java:709)",
      "org.apache.lucene.search.FieldComparator$TermOrdValComparator.getLeafComparator(FieldComparator.java:714)",
      "org.apache.lucene.search.grouping.AllGroupHeadsCollector$SortingGroupHead.<init>(AllGroupHeadsCollector.java:266)",
      "org.apache.lucene.search.grouping.AllGroupHeadsCollector$SortingGroupHeadsCollector.newGroupHead(AllGroupHeadsCollector.java:250)",
      "org.apache.lucene.search.grouping.AllGroupHeadsCollector.collect(AllGroupHeadsCollector.java:133)",
   
      "org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:440)",
   
      "org.apache.solr.search.grouping.CommandHandler.execute(CommandHandler.java:158)",
      "org.apache.solr.handler.component.QueryComponent.doProcessGroupedDistributedSearchSecondPhase(QueryComponent.java:1399)",
      "org.apache.solr.handler.component.QueryComponent.process(QueryComponent.java:386)",
   

{code}
I read it as follows, when group collector encounter new group it creates field compactor for this value that opens DocValues that turns out to be a way more expensive to open rather than old good FieldCache. I think DocValues should somehow to be reused between groups. WDYT?
 

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Sure, go ahead!

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

Spawned #10368