Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CI] TopHitsIT#testTopHitsInNestedSimple failure #26738

Closed
martijnvg opened this issue Sep 21, 2017 · 1 comment
Closed

[CI] TopHitsIT#testTopHitsInNestedSimple failure #26738

martijnvg opened this issue Sep 21, 2017 · 1 comment
Assignees
@martijnvg
Labels
>test Issues or PRs that are addressing/adding tests

Comments

@martijnvg
Copy link
Member

Reproduce line: gradle :core:integTest -Dtests.seed=F4BA8811B299D761 -Dtests.class=org.elasticsearch.search.aggregations.metrics.TopHitsIT -Dtests.method="testTopHitsInNestedSimple" -Dtests.security.manager=true -Dtests.locale=sk-SK -Dtests.timezone=Pacific/Pago_Pago

Doesn't reproduce consistently.

Actual error:

 2> WARNING: Uncaught exception in thread: Thread[elasticsearch[node_sd1][search][T#1],5,TGRP-TopHitsIT]
  2> java.lang.AssertionError
  2> 	at __randomizedtesting.SeedInfo.seed([F4BA8811B299D761]:0)
  2> 	at org.apache.lucene.index.AssertingLeafReader$AssertingSortedNumericDocValues.advanceExact(AssertingLeafReader.java:730)
  2> 	at org.apache.lucene.search.SortedNumericSelector$MinValue.advanceExact(SortedNumericSelector.java:136)
  2> 	at org.apache.lucene.search.FieldComparator$LongComparator.getValueForDoc(FieldComparator.java:363)
  2> 	at org.apache.lucene.search.FieldComparator$LongComparator.copy(FieldComparator.java:382)
  2> 	at org.apache.lucene.search.TopFieldCollector$SimpleFieldCollector$1.collect(TopFieldCollector.java:141)
  2> 	at org.elasticsearch.search.aggregations.metrics.tophits.TopHitsAggregator$1.collect(TopHitsAggregator.java:138)
  2> 	at org.elasticsearch.search.aggregations.bucket.BestBucketsDeferringCollector.prepareSelectedBuckets(BestBucketsDeferringCollector.java:178)
  2> 	at org.elasticsearch.search.aggregations.bucket.DeferringBucketCollector.replay(DeferringBucketCollector.java:44)
  2> 	at org.elasticsearch.search.aggregations.bucket.DeferableBucketAggregator.runDeferredCollections(DeferableBucketAggregator.java:91)
  2> 	at org.elasticsearch.search.aggregations.bucket.terms.GlobalOrdinalsStringTermsAggregator.buildAggregation(GlobalOrdinalsStringTermsAggregator.java:225)
  2> 	at org.elasticsearch.search.aggregations.AggregatorFactory$MultiBucketAggregatorWrapper.buildAggregation(AggregatorFactory.java:147)
  2> 	at org.elasticsearch.search.aggregations.bucket.BucketsAggregator.bucketAggregations(BucketsAggregator.java:113)
  2> 	at org.elasticsearch.search.aggregations.bucket.nested.NestedAggregator.buildAggregation(NestedAggregator.java:116)
  2> 	at org.elasticsearch.search.aggregations.AggregationPhase.execute(AggregationPhase.java:129)
  2> 	at org.elasticsearch.search.query.QueryPhase.execute(QueryPhase.java:116)
  2> 	at org.elasticsearch.search.SearchService.loadOrExecuteQueryPhase(SearchService.java:302)
  2> 	at org.elasticsearch.search.SearchService.executeQueryPhase(SearchService.java:335)
  2> 	at org.elasticsearch.search.SearchService$2.onResponse(SearchService.java:311)
  2> 	at org.elasticsearch.search.SearchService$2.onResponse(SearchService.java:307)
  2> 	at org.elasticsearch.search.SearchService$3.doRun(SearchService.java:997)
  2> 	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:638)
  2> 	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)
  2> 	at org.elasticsearch.common.util.concurrent.TimedRunnable.run(TimedRunnable.java:41)
  2> 	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
  2> 	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
  2> 	at java.lang.Thread.run(Thread.java:745)

Is related to this change: #26683
If top_hits aggregation gets wrapped inside nested aggregation and execution mode is breath first then sometimes the leaf collectors don't get created properly in TopHitsAggregator. I'm thinking about how to best fix this.
@martijnvg martijnvg added the >test Issues or PRs that are addressing/adding tests label Sep 21, 2017
@martijnvg martijnvg self-assigned this Sep 21, 2017
@martijnvg
Copy link
Member Author

Muted this test for now in master (fda8f8b) and 6.x (369a899).

@martijnvg martijnvg mentioned this issue Sep 22, 2017
Merged
martijnvg added a commit that referenced this issue Sep 22, 2017
@martijnvg
aggs: Changed how top_hits initialises leaf collectors
a1d049c 
Both TopDocsCollector and LeafCollector were being kept around at the aggregator level. In case the nested aggregator would do a post collection then this could cause pushing down docids to top hits child aggregators that already moved the next LeafCollector (causing assertions to trip and incorrect results).

By keeping track of the LeafCollector in a seperate map at the leaf bucket level this problem can simply not happen any more as the place holding LeafCollector is no longer shared.

Also LeafCollector instances for TopDocsCollectors are no longer pre-created as the beginning a new segment is evaluated. There is no guarantee that TopHitsAggregator encounters a document for a particular bucket and there has to be logic to create LeafCollector instances which have not been seen before.

Closes #26738
rahulanishetty pushed a commit to rahulanishetty/elasticsearch that referenced this issue Jul 26, 2018
@martijnvg
aggs: Changed how top_hits initialises leaf collectors
e5ea7f4 
Both TopDocsCollector and LeafCollector were being kept around at the aggregator level. In case the nested aggregator would do a post collection then this could cause pushing down docids to top hits child aggregators that already moved the next LeafCollector (causing assertions to trip and incorrect results).

By keeping track of the LeafCollector in a seperate map at the leaf bucket level this problem can simply not happen any more as the place holding LeafCollector is no longer shared.

Also LeafCollector instances for TopDocsCollectors are no longer pre-created as the beginning a new segment is evaluated. There is no guarantee that TopHitsAggregator encounters a document for a particular bucket and there has to be logic to create LeafCollector instances which have not been seen before.

Closes elastic#26738

(cherry picked from commit a1d049c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@martijnvg martijnvg

Labels
>test Issues or PRs that are addressing/adding tests
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@martijnvg