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

[asfimport]

That's why https://issues.apache.org/jira/browse/LUCENE-7701?focusedCommentId=17084365&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17084365

---

Migrated from LUCENE-9328 by Mikhail Khludnev (@mkhludnev), updated Sep 05 2020
Attachments: LUCENE-9328.patch (versions: 12)
Linked issues:

• false positive match BlockJoinSelector[SortedDV] when child value is absent [LUCENE-7871] #8922
• Refactor grouping collectors [LUCENE-7701] #8752

Sub-tasks:

• AssertingSorted(Set|Numeric)DocValues should be unwrappable [LUCENE-9357] #10397
• might be NEEDED. ToParentDocValues uses advanceExact() of underneath DocValues [LUCENE-9360] #10400

Pull requests: apache/lucene-solr#1462, apache/lucene-solr#1625

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Can we try and avoid making any changes to FieldComparator and friends?  I think we can do this entirely within AllGroupHeadsCollector, by wrapping incoming LeafReaders with a FilterLeafReader that caches the various DocValues iterators.  We may need to make some changes to allow a FilterLeafReader to keep its inner reader's context though, so that the comparators see the correct docBase.

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

Good idea, @romseygeek. Let me try.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
			Prechecks
-1	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			master Compile Tests
+1	compile	1m 36s	master passed
			Patch Compile Tests
+1	compile	1m 15s	the patch passed
+1	javac	1m 15s	the patch passed
+1	Release audit (RAT)	1m 12s	the patch passed
+1	Check forbidden APIs	1m 8s	the patch passed
+1	Validate source patterns	1m 8s	the patch passed
			Other Tests
-1	unit	6m 36s	core in the patch failed.
+1	unit	0m 25s	grouping in the patch passed.
		11m 59s

Reason	Tests
Failed junit tests	lucene.search.TestSortRandom
	lucene.index.TestIndexSorting

Subsystem	Report/Notes
JIRA Issue	LUCENE-9328
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/13000386/LUCENE-9328.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene2-us-west.apache.org 4.4.0-170-generic #199-Ubuntu SMP Thu Nov 14 01:45:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / f5d9139
ant	version: Apache Ant(TM) version 1.9.6 compiled on July 20 2018
Default Java	LTS
unit	https://builds.apache.org/job/PreCommit-LUCENE-Build/263/artifact/out/patch-unit-lucene_core.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/263/testReport/
modules	C: lucene/core lucene/grouping U: lucene
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/263/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

It's a little bit awkward to wrap LeafReaderContext. Attaching a little bit rough patch.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
			Prechecks
-1	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			master Compile Tests
+1	compile	0m 23s	master passed
			Patch Compile Tests
+1	compile	0m 18s	the patch passed
+1	javac	0m 18s	the patch passed
+1	Release audit (RAT)	0m 18s	the patch passed
+1	Check forbidden APIs	0m 18s	the patch passed
+1	Validate source patterns	0m 18s	the patch passed
			Other Tests
+1	unit	0m 29s	grouping in the patch passed.
		4m 6s

Subsystem	Report/Notes
JIRA Issue	LUCENE-9328
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/13000889/LUCENE-9328.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene2-us-west.apache.org 4.4.0-170-generic #199-Ubuntu SMP Thu Nov 14 01:45:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / e43b179
ant	version: Apache Ant(TM) version 1.9.6 compiled on July 20 2018
Default Java	LTS
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/264/testReport/
modules	C: lucene/grouping U: lucene/grouping
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/264/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

✔ +1 overall

Vote	Subsystem	Runtime	Comment
			Prechecks
+1	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
			master Compile Tests
+1	compile	0m 18s	master passed
			Patch Compile Tests
+1	compile	0m 18s	the patch passed
+1	javac	0m 18s	the patch passed
+1	Release audit (RAT)	0m 18s	the patch passed
+1	Check forbidden APIs	0m 18s	the patch passed
+1	Validate source patterns	0m 18s	the patch passed
			Other Tests
+1	unit	0m 10s	grouping in the patch passed.
		2m 26s

Subsystem	Report/Notes
JIRA Issue	LUCENE-9328
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/13001169/LUCENE-9328.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene2-us-west.apache.org 4.4.0-170-generic #199-Ubuntu SMP Thu Nov 14 01:45:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / ecc98e8
ant	version: Apache Ant(TM) version 1.9.6 compiled on July 20 2018
Default Java	LTS
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/265/testReport/
modules	C: lucene/grouping U: lucene/grouping
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/265/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Would you be able to open this as a github pull request? It's much easier to review than a patch.

We need to deal with SortedNumeric and SortedSet iterators, because they also have the notion of a within-document iterator that will need to be independent between all the different consumers. I'm also a bit worried about one consumer being able to advance the underlying values iterator too far. Can we make the ReusingReader control advancing the iterators?

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

@romseygeek, you are welcome apache/lucene-solr#1462

At the patch all type docvalues pooled already. I'm not sure I understand what you mean in control advancing the iterators. Now there's no control. It's not so scary as it looked at the first glance. Collector scores docs in-order (o'rlly?), every doc just kicks a certain GroupHead, which pull a value(s) from it. And that's it. I don't even think they ever call dv.advanceExact() for the current dv.docID. I think I'll add a test covering grouping/sorting over sortedset dvs. 

[asfimport]

Alan Woodward (@romseygeek) (migrated from JIRA)

Thanks @mkhludnev, I'll have a proper look tomorrow.

The GroupHeads don't call advanceExact(), but they use FieldComparators which do; and some of them do some very funky stuff with their iterators, eg see ToParentBlockJoinSortField#getXXXComparator

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

Ok. @romseygeek, thanks for the clue.
Sorry for noise. WIP.

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

It's better than I expected.

• Asserting DVs and sigletonDVs, spawned subtask.
• Need to migrate ToParentDocValues to advanceExact() will spawn one.
• there's a runtime check in one class, I commented it and will discuss it separately.
• curiously, it's hard to put a test which covering grouping over blockjoin in a proper module, so far it's landed in solr/test.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
-1	patch	0m 8s	LUCENE-9328 does not apply to master. Rebase required? Wrong Branch? See https://wiki.apache.org/lucene-java/HowToContribute#Contributing_your_work for help.

Subsystem	Report/Notes
JIRA Issue	LUCENE-9328
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/13001957/LUCENE-9328.patch
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/266/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
-1	patch	0m 8s	LUCENE-9328 does not apply to master. Rebase required? Wrong Branch? See https://wiki.apache.org/lucene-java/HowToContribute#Contributing_your_work for help.

Subsystem	Report/Notes
JIRA Issue	LUCENE-9328
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/13001971/LUCENE-9328.patch
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/267/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.

[asfimport]

Mikhail Khludnev (@mkhludnev) (migrated from JIRA)

Attaching a proper git patch.

[asfimport]

Lucene/Solr QA (migrated from JIRA)

❌ -1 overall

Vote	Subsystem	Runtime	Comment
			Prechecks
+1	test4tests	0m 0s	The patch appears to include 3 new or modified test files.
			master Compile Tests
+1	compile	2m 20s	master passed
			Patch Compile Tests
+1	compile	3m 17s	the patch passed
-1	javac	0m 58s	lucene_grouping generated 1 new + 100 unchanged - 0 fixed = 101 total (was 100)
+1	Release audit (RAT)	1m 9s	the patch passed
+1	Check forbidden APIs	0m 58s	the patch passed
+1	Validate source patterns	0m 58s	the patch passed
			Other Tests
+1	unit	0m 19s	grouping in the patch passed.
+1	unit	0m 18s	join in the patch passed.
+1	unit	0m 32s	test-framework in the patch passed.
-1	unit	71m 47s	core in the patch failed.
		83m 5s

Reason	Tests
Failed junit tests	solr.search.grouping.AllGroupHeadsCollectorTest
	solr.TestGroupingSearch

Subsystem	Report/Notes
JIRA Issue	LUCENE-9328
JIRA Patch URL	https://issues.apache.org/jira/secure/attachment/13002026/LUCENE-9328.patch
Optional Tests	compile javac unit ratsources checkforbiddenapis validatesourcepatterns
uname	Linux lucene2-us-west.apache.org 4.4.0-170-generic #199-Ubuntu SMP Thu Nov 14 01:45:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	ant
Personality	/home/jenkins/jenkins-slave/workspace/PreCommit-LUCENE-Build/sourcedir/dev-tools/test-patch/lucene-solr-yetus-personality.sh
git revision	master / 6f775bf
ant	version: Apache Ant(TM) version 1.9.6 compiled on July 20 2018
Default Java	LTS
javac	https://builds.apache.org/job/PreCommit-LUCENE-Build/268/artifact/out/diff-compile-javac-lucene_grouping.txt
unit	https://builds.apache.org/job/PreCommit-LUCENE-Build/268/artifact/out/patch-unit-solr_core.txt
Test Results	https://builds.apache.org/job/PreCommit-LUCENE-Build/268/testReport/
modules	C: lucene/grouping lucene/join lucene/test-framework solr/core U: .
Console output	https://builds.apache.org/job/PreCommit-LUCENE-Build/268/console
Powered by	Apache Yetus 0.7.0 http://yetus.apache.org

This message was automatically generated.