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] org.elasticsearch.percolator.CandidateQueryTests.testDuel failure #50305

Closed
martijnvg opened this issue Dec 18, 2019 · 3 comments · Fixed by #50669
Closed

[CI] org.elasticsearch.percolator.CandidateQueryTests.testDuel failure #50305

martijnvg opened this issue Dec 18, 2019 · 3 comments · Fixed by #50669
Assignees
@romseygeek
Labels
:Search/Percolator Reverse search: find queries that match a document >test-failure Triaged test failures from CI

Comments

@martijnvg
Copy link
Member 

REPRODUCE WITH: ./gradlew ':modules:percolator:test' --tests "org.elasticsearch.percolator.CandidateQueryTests.testDuel" -Dtests.seed=90ED18A4A0A763FF -Dtests.security.manager=true -Dtests.locale=el-CY -Dtests.timezone=Etc/GMT-13 -Dcompiler.java=13

This failure does reproduce locally for me.

2> java.lang.AssertionError: 
    Expected: <835L>
         but: was <834L>
        at __randomizedtesting.SeedInfo.seed([90ED18A4A0A763FF:DC6DA2A4FE2FFBED]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:956)
        at org.junit.Assert.assertThat(Assert.java:923)
        at org.elasticsearch.percolator.CandidateQueryTests.duelRun(CandidateQueryTests.java:1056)
        at org.elasticsearch.percolator.CandidateQueryTests.testDuel(CandidateQueryTests.java:269)

Build urls:
@martijnvg martijnvg added >test-failure Triaged test failures from CI :Search/Percolator Reverse search: find queries that match a document labels Dec 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Percolator)

@martijnvg
Copy link
Member Author

This is a slightly different instance of this failure, which I expect to be related:

org.elasticsearch.percolator.CandidateQueryTests > testDuel FAILED
02:46:44     java.lang.AssertionError: 
02:46:44     Expected: <838L>
02:46:44          but: was <837L>
02:46:44         at __randomizedtesting.SeedInfo.seed([DC3224DB2B29D634:90B29EDB75A14E26]:0)
02:46:44         at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
02:46:44         at org.junit.Assert.assertThat(Assert.java:956)
02:46:44         at org.junit.Assert.assertThat(Assert.java:923)
02:46:44         at org.elasticsearch.percolator.CandidateQueryTests.duelRun(CandidateQueryTests.java:1062)
02:46:44         at org.elasticsearch.percolator.CandidateQueryTests.testDuel(CandidateQueryTests.java:269)

Build urls:

This also reproduces for me:

./gradlew ':modules:percolator:test' --tests "org.elasticsearch.percolator.CandidateQueryTests.testDuel"   -Dtests.seed=DC3224DB2B29D634

@martijnvg
Copy link
Member Author

Other instances of this failure:

@romseygeek romseygeek self-assigned this Dec 18, 2019
@romseygeek romseygeek mentioned this issue Jan 6, 2020
Merged
romseygeek added a commit that referenced this issue Jan 7, 2020
@romseygeek
Correctly handle MSM for nested disjunctions (#50669)
0bbae8a 
With the rewrite of the percolator's QueryAnalyzer to use lucene's QueryVisitor API,
term queries that are direct children of a boolean query are handled separately from
other children. This works fine for conjunctions, but for disjunctions we need to
treat the extracted terms from these direct descendents along with extractions from
more deeply nested children to ensure that minimum-should-match requirements
are met correctly.

This commit changes the logic in QueryAnalyzer#getResult() to bundle child term
results with all other results before handling them.

Fixes #50305
romseygeek added a commit that referenced this issue Jan 7, 2020
@romseygeek
Correctly handle MSM for nested disjunctions (#50669)
a3ab7eb 
With the rewrite of the percolator's QueryAnalyzer to use lucene's QueryVisitor API,
term queries that are direct children of a boolean query are handled separately from
other children. This works fine for conjunctions, but for disjunctions we need to
treat the extracted terms from these direct descendents along with extractions from
more deeply nested children to ensure that minimum-should-match requirements
are met correctly.

This commit changes the logic in QueryAnalyzer#getResult() to bundle child term
results with all other results before handling them.

Fixes #50305
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this issue Jan 23, 2020
@romseygeek @SivagurunathanV
Correctly handle MSM for nested disjunctions (elastic#50669)
45e5244 
With the rewrite of the percolator's QueryAnalyzer to use lucene's QueryVisitor API,
term queries that are direct children of a boolean query are handled separately from
other children. This works fine for conjunctions, but for disjunctions we need to
treat the extracted terms from these direct descendents along with extractions from
more deeply nested children to ensure that minimum-should-match requirements
are met correctly.

This commit changes the logic in QueryAnalyzer#getResult() to bundle child term
results with all other results before handling them.

Fixes elastic#50305
This was referenced Feb 3, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@romseygeek romseygeek

Labels
:Search/Percolator Reverse search: find queries that match a document >test-failure Triaged test failures from CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correctly handle MSM for nested disjunctions romseygeek/elasticsearch
3 participants
@martijnvg @romseygeek @elasticmachine