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

[7.16] SQL: fix use of requestTimeout and pageTimeout query parameters (#79360) #79915

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

Luegg
Copy link
Contributor

@Luegg Luegg commented Oct 27, 2021

Backports the following commits to 7.16:

…tic#79360)

Resolves elastic#72151 The _sql endpoint offers a `page_timeout` parameter for
customizing how long scroll contexts should be kept open (if needed) and
a `request_timeout` parameter which the docs describe as "Timeout before
the request fails.". Currently, the value of the `page_timeout`
parameter is used as the `timeout` in subsequent _search requests and
not as the timeout in the `scroll` configuration. For the `scroll`
configuration, SQL uses the `request_timeout` parameter. This PR
addresses the issue by swapping the uses of `page_timeout` and
`request_timeout` in querier. Additionally, the PR removes some unused
artifacts that might have caused some confusion: - The `timeout` and
`keepAlive` fields in `Querier`. Instead, `Querier` directly uses the
according fields in `SqlConfiguration`. - The `SqlConfiguration`
parameter from `ScrollCursor.clear`, it's not used but required an
instance of `SqlConfiguration` with all default values. - One overloaded
constructor of `SqlConfiguration` that was only used for calling
`ScrollCursor.clear` (and some tests) and used default values for an
(arbitrary?) subset of the fields. - The fields related to async
requests in `SqlConfiguration`. I'm a bit unsure about this one but the
fields are never read and it does not seem like an SQL specific concern.
The whole creation of the async tasks is handled in
`TransportSqlQueryAction` and the downstream components do not require
the information.
@Luegg Luegg added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport labels Oct 27, 2021
@elasticsearchmachine elasticsearchmachine merged commit 9bf3a32 into elastic:7.16 Oct 27, 2021
@Luegg Luegg deleted the backport/7.16/pr-79360 branch October 27, 2021 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) backport v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants