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] EqlCancellationIT#testCancellation failure #62530

Closed
martijnvg opened this issue Sep 17, 2020 · 4 comments · Fixed by #62577
Closed

[CI] EqlCancellationIT#testCancellation failure #62530

martijnvg opened this issue Sep 17, 2020 · 4 comments · Fixed by #62577
Assignees
@imotov
Labels
:Analytics/EQL EQL querying Team:QL (Deprecated) Meta label for query languages team >test-failure Triaged test failures from CI

Comments

@martijnvg
Copy link
Member

Build scan: https://gradle-enterprise.elastic.co/s/gehzubbmkv22c/tests/:x-pack:plugin:eql:internalClusterTest/org.elasticsearch.xpack.eql.action.EqlCancellationIT/testCancellation#1

Repro line:

./gradlew ':x-pack:plugin:eql:internalClusterTest' --tests "org.elasticsearch.xpack.eql.action.EqlCancellationIT.testCancellation" -Dtests.seed=CED58D289E2F2711 -Dtests.security.manager=true -Dtests.locale=it-IT -Dtests.timezone=GB -Druntime.java=8

Reproduces locally?: No

Applicable branches: 7.x

Failure history:

Failed twice in the last 7 days, in 7.x and a pr build for master.

https://gradle-enterprise.elastic.co/scans/tests?search.buildToolTypes=gradle&search.buildToolTypes=maven&search.relativeStartTime=P7D&search.timeZoneId=Europe/Amsterdam&tests.container=org.elasticsearch.xpack.eql.action.EqlCancellationIT&tests.sortField=FAILED&tests.test=testCancellation&tests.unstableOnly=true

Failure excerpt:

junit.framework.AssertionFailedError: Expected exception Exception but no exception was thrown	
at __randomizedtesting.SeedInfo.seed([CED58D289E2F2711:18E2D25E00901097]:0)	
at org.apache.lucene.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2744)	
at org.apache.lucene.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2734)	
at org.elasticsearch.xpack.eql.action.EqlCancellationIT.testCancellation(EqlCancellationIT.java:78)
@martijnvg martijnvg added >test-failure Triaged test failures from CI :Analytics/EQL EQL querying labels Sep 17, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (:Query Languages/EQL)

@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Sep 17, 2020
@costin
Copy link
Member

costin commented Sep 17, 2020

Igor can you please take a look?

@andreidan
Copy link
Contributor

Another occurrence https://gradle-enterprise.elastic.co/s/fhwzd3ekix2wo

@imotov
Copy link
Contributor

imotov commented Sep 17, 2020

This is triggered by the change in 63afc61#diff-9c70930068120c515204efa560512fd9R96

Basically, in this test the shortcut is triggered by before we ever check if task is cancelled causing the task to proceed. I am working on a fix.

@imotov imotov mentioned this issue Sep 17, 2020
Merged
imotov added a commit that referenced this issue Sep 17, 2020
@imotov
Add an additional cancellation check to the fetch phase (#62577)
1b030e7 
In #62357 we introduced an additional optimization that allows us to skip the
most of the fetch phase early if no results are found. This change caused
some cancellation test failures that were relying on definitive cancellation
during the fetch phase. This commit adds an additional quick cancellation
check at the very beginning of the fetch phase to make cancellation process
more deterministic.

Fixes #62530
@imotov imotov mentioned this issue Sep 17, 2020
Merged
imotov added a commit that referenced this issue Sep 18, 2020
@imotov
Add an additional cancellation check to the fetch phase (#62577) (#62587
260c11d 
)

In #62357 we introduced an additional optimization that allows us to skip the
most of the fetch phase early if no results are found. This change caused
some cancellation test failures that were relying on definitive cancellation
during the fetch phase. This commit adds an additional quick cancellation
check at the very beginning of the fetch phase to make cancellation process
more deterministic.

Fixes #62530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@imotov imotov

Labels
:Analytics/EQL EQL querying Team:QL (Deprecated) Meta label for query languages team >test-failure Triaged test failures from CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add an additional cancellation check to the fetch phase imotov/elasticsearch
5 participants
@costin @martijnvg @imotov @andreidan @elasticmachine