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.

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

SQL: stabilize SqlSearchPageTimeoutIT #79928

Merged

Conversation

Luegg
Copy link
Contributor

@Luegg Luegg commented Oct 27, 2021

#79360 introduced a flaky test.

Increasing the page timeout used in the initial query should ensure that line 44 should always see the search context (I suspect that it has been removed before getNumberOfSearchContexts() reads the search contexts).

I've run the test with a 1000 iterations locally on both master and 7.16 and it didn't fail.

@Luegg Luegg added the :Analytics/SQL SQL querying label Oct 27, 2021
@Luegg Luegg requested a review from bpintea October 27, 2021 16:21
@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Oct 27, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

Copy link
Contributor

@bpintea bpintea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Luegg Luegg added auto-backport-and-merge auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Oct 27, 2021
@Luegg
Copy link
Contributor Author

Luegg commented Oct 27, 2021

@elasticmachine run elasticsearch-ci/part-2

elasticsearchmachine pushed a commit that referenced this pull request Oct 27, 2021
since #79928 is blocked by other failing tests I'm just muting it for
now.
Luegg pushed a commit to Luegg/elasticsearch that referenced this pull request Oct 27, 2021
since elastic#79928 is blocked by other failing tests I'm just muting it for
now.
Luegg pushed a commit to Luegg/elasticsearch that referenced this pull request Oct 27, 2021
since elastic#79928 is blocked by other failing tests I'm just muting it for
now.
Luegg pushed a commit that referenced this pull request Oct 27, 2021
since #79928 is blocked by other failing tests I'm just muting it for
now.
Luegg pushed a commit that referenced this pull request Oct 27, 2021
since #79928 is blocked by other failing tests I'm just muting it for
now.
@Luegg Luegg removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge labels Oct 28, 2021
@Luegg Luegg added auto-backport-and-merge auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) v7.16.0 and removed v7.16.1 labels Oct 28, 2021
@elasticsearchmachine elasticsearchmachine merged commit 9113ff1 into elastic:master Oct 28, 2021
Luegg pushed a commit to Luegg/elasticsearch that referenced this pull request Oct 28, 2021
elastic#79360 introduced a [flaky
test](https://gradle-enterprise.elastic.co/scans/tests?search.relativeStartTime=P7D&search.timeZoneId=Europe/Zurich&tests.container=org.elasticsearch.xpack.sql.action.SqlSearchPageTimeoutIT&tests.sortField=FAILED&tests.test=testSearchContextIsCleanedUpAfterPageTimeoutForHitsQueries&tests.unstableOnly=true).
Increasing the page timeout used in the initial query should ensure that
line 44 should always see the search context (I suspect that it has been
removed before `getNumberOfSearchContexts()` reads the search contexts).
I've run the test with a 1000 iterations locally on both master and 7.16
and it didn't fail.
Luegg pushed a commit to Luegg/elasticsearch that referenced this pull request Oct 28, 2021
elastic#79360 introduced a [flaky
test](https://gradle-enterprise.elastic.co/scans/tests?search.relativeStartTime=P7D&search.timeZoneId=Europe/Zurich&tests.container=org.elasticsearch.xpack.sql.action.SqlSearchPageTimeoutIT&tests.sortField=FAILED&tests.test=testSearchContextIsCleanedUpAfterPageTimeoutForHitsQueries&tests.unstableOnly=true).
Increasing the page timeout used in the initial query should ensure that
line 44 should always see the search context (I suspect that it has been
removed before `getNumberOfSearchContexts()` reads the search contexts).
I've run the test with a 1000 iterations locally on both master and 7.16
and it didn't fail.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.0
7.16

elasticsearchmachine pushed a commit that referenced this pull request Oct 28, 2021
#79360 introduced a [flaky
test](https://gradle-enterprise.elastic.co/scans/tests?search.relativeStartTime=P7D&search.timeZoneId=Europe/Zurich&tests.container=org.elasticsearch.xpack.sql.action.SqlSearchPageTimeoutIT&tests.sortField=FAILED&tests.test=testSearchContextIsCleanedUpAfterPageTimeoutForHitsQueries&tests.unstableOnly=true).
Increasing the page timeout used in the initial query should ensure that
line 44 should always see the search context (I suspect that it has been
removed before `getNumberOfSearchContexts()` reads the search contexts).
I've run the test with a 1000 iterations locally on both master and 7.16
and it didn't fail.
elasticsearchmachine pushed a commit that referenced this pull request Oct 28, 2021
#79360 introduced a [flaky
test](https://gradle-enterprise.elastic.co/scans/tests?search.relativeStartTime=P7D&search.timeZoneId=Europe/Zurich&tests.container=org.elasticsearch.xpack.sql.action.SqlSearchPageTimeoutIT&tests.sortField=FAILED&tests.test=testSearchContextIsCleanedUpAfterPageTimeoutForHitsQueries&tests.unstableOnly=true).
Increasing the page timeout used in the initial query should ensure that
line 44 should always see the search context (I suspect that it has been
removed before `getNumberOfSearchContexts()` reads the search contexts).
I've run the test with a 1000 iterations locally on both master and 7.16
and it didn't fail.

Co-authored-by: Elastic Machine <[email protected]>
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 28, 2021
…formance

* upstream/master: (153 commits)
  [ML] update truncation default & adding field output when input is truncated (elastic#79942)
  [ML] stop using isAllowedByLicense for model license checks (elastic#79908)
  [ML] Retain built-in ML roles granting Kibana privileges (elastic#80014)
  [Transform] remove old mixed cluster BWC layers, not required for 8x (elastic#79927)
  Increase test timeout for CoordinatorTests testAllSearchesExecuted
  [Transform] add rolling upgrade tests for upgrade endpoint (elastic#79721)
  [ML] Update trained model docs for truncate parameter for bert tokenization (elastic#79652)
  `CoordinatorTests` sometimes needs three term bumps (elastic#79574)
  [ML] Account for service being triggered twice in tests (elastic#80000)
  SearchContext: remove unused variable (elastic#79917)
  Revert "Deprecate resolution loss on date field (elastic#78921)" (elastic#79914)
  Re-enable GeoIpDownloaderIT#testStartWithNoDatabases() (elastic#79907)
  Fix SnapshotBasedIndexRecoveryIT#testSeqNoBasedRecoveryIsUsedAfterPrimaryFailOver (elastic#79469)
  Fix RecoverySourceHandlerTests (elastic#79546)
  SQL: stabilize SqlSearchPageTimeoutIT (elastic#79928)
  Wait 3 seconds for the server to reload trust (elastic#79778)
  Skip automatically preserved request headers when rewriting (elastic#79973)
  Check whether stdout is a real console (elastic#79882)
  Convert remote license checker to use LicensedFeature (elastic#79876)
  Miscellaneous fixes for LDAP SDK v6 upgrade (elastic#79891)
  ...

# Conflicts:
#	libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPath.java
#	libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathGeneratorFilteringTests.java
#	libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathTests.java
@danhermann danhermann added the >test Issues or PRs that are addressing/adding tests label Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/SQL SQL querying auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) Team:QL (Deprecated) Meta label for query languages team >test Issues or PRs that are addressing/adding tests v7.16.0 v8.0.0-beta1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants