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

[Reporting/CSV] Resolve max_concurrent_shard_requests issue #182536

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented May 2, 2024

Summary

There has been a consistent failure in a Discover-related test set in the kibana-ES-serverless verification job, meaning that ES-Kibana compatibility has drifted.

Error details:

 + "Encountered an unknown error: status_exception
 + 	Root causes:
 + 		status_exception: Parameter validation failed for [/_search]: The http parameter [max_concurrent_shard_requests] (with value [5]) is not permitted when running in serverless mode"
 + "Encountered an error with the number of CSV rows generated fromthe search: expected rows were indeterminable, received 0."
       at Context.<anonymous> (reporting.ts:182:33)
       at processTicksAndRejections (node:internal/process/task_queues:95:5)
       at Object.apply (wrap_function.js:73:16)

This tracked back to a feature added for reporting awhile back, which created a config schema field for the max_concurrent_shard_requests parameter in the search queries: https://github.com/elastic/kibana/pull/170344/files#diff-7bceb37eef3761e1161cf04f41668dd9195bfac9fea36e734a230b5ed878a974

Most of the changes in this PR are in test code. I created "Test" which extend protected methods in the original classes. This was done to remove the @ts-expect-errors lines of code.

@tsullivan tsullivan force-pushed the reporting/csv-serverless-max-concurrent-shards branch from 2e0e352 to 18b1541 Compare May 2, 2024 22:35
@@ -37,7 +37,7 @@ export class SearchCursorPit extends SearchCursor {
this.cursorId = await this.openPointInTime();
}

private async openPointInTime() {
Copy link
Member Author

@tsullivan tsullivan May 2, 2024

Choose a reason for hiding this comment

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

A lot of changes in this PR are to use Test classes (TestSearchCursorPit and TestSearchCursorScroll) that extend protected methods in the original classes. This was done to remove the @ts-expect-errors lines of code that were in the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

These test additions are awesome 🎉

@tsullivan tsullivan requested a review from davismcphee May 2, 2024 22:40
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@tsullivan tsullivan marked this pull request as ready for review May 3, 2024 01:17
@tsullivan tsullivan requested review from a team as code owners May 3, 2024 01:17
@tsullivan tsullivan added bug Fixes for quality problems that affect the customer experience Feature:Reporting Reporting (PDF, CSV, ..) feature release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) labels May 3, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@@ -37,7 +37,7 @@ export class SearchCursorPit extends SearchCursor {
this.cursorId = await this.openPointInTime();
}

private async openPointInTime() {
Copy link
Contributor

Choose a reason for hiding this comment

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

These test additions are awesome 🎉

@tsullivan tsullivan merged commit f51c5c9 into elastic:main May 3, 2024
26 checks passed
@tsullivan tsullivan deleted the reporting/csv-serverless-max-concurrent-shards branch May 3, 2024 14:30
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels May 3, 2024
@kertal
Copy link
Member

kertal commented May 6, 2024

I'm having a look at 3 8.14 serverless failures with the described error message status_exception: Parameter validation failed for [/_search]: The http parameter [max_concurrent_shard_requests] (with value [5]) is not permitted when running in serverless mode"

#182599
#182601
#182603

I assume this would be resolved when this PR is back ported?

@tsullivan
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.14

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

tsullivan added a commit to tsullivan/kibana that referenced this pull request May 6, 2024
…182536)

## Summary

There has been a consistent failure in a Discover-related test set in
the kibana-ES-serverless verification job, meaning that ES-Kibana
compatibility has drifted.

Error details:
```
 + "Encountered an unknown error: status_exception
 + 	Root causes:
 + 		status_exception: Parameter validation failed for [/_search]: The http parameter [max_concurrent_shard_requests] (with value [5]) is not permitted when running in serverless mode"
 + "Encountered an error with the number of CSV rows generated fromthe search: expected rows were indeterminable, received 0."
       at Context.<anonymous> (reporting.ts:182:33)
       at processTicksAndRejections (node:internal/process/task_queues:95:5)
       at Object.apply (wrap_function.js:73:16)
```

This tracked back to a feature added for reporting awhile back, which
created a config schema field for the `max_concurrent_shard_requests`
parameter in the search queries:
https://github.com/elastic/kibana/pull/170344/files#diff-7bceb37eef3761e1161cf04f41668dd9195bfac9fea36e734a230b5ed878a974

Most of the changes in this PR are in test code. I created "Test" which
extend protected methods in the original classes. This was done to
remove the `@ts-expect-errors` lines of code.

(cherry picked from commit f51c5c9)
@tsullivan
Copy link
Member Author

@kertal I have opened a backport of this fix to 8.14 to silence the noise created from these failure.

@tsullivan tsullivan removed the backport:skip This commit does not require backporting label May 7, 2024
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 7, 2024
@tsullivan tsullivan added v8.14.0 and removed backport:skip This commit does not require backporting labels May 7, 2024
tsullivan added a commit that referenced this pull request May 7, 2024
…182536) (#182742)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[Reporting/CSV] Resolve max_concurrent_shard_requests issue
(#182536)](#182536)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-05-03T14:30:45Z","message":"[Reporting/CSV]
Resolve max_concurrent_shard_requests issue (#182536)\n\n##
Summary\r\n\r\nThere has been a consistent failure in a Discover-related
test set in\r\nthe kibana-ES-serverless verification job, meaning that
ES-Kibana\r\ncompatibility has drifted.\r\n\r\nError details:\r\n```\r\n
+ \"Encountered an unknown error: status_exception\r\n + \tRoot
causes:\r\n + \t\tstatus_exception: Parameter validation failed for
[/_search]: The http parameter [max_concurrent_shard_requests] (with
value [5]) is not permitted when running in serverless mode\"\r\n +
\"Encountered an error with the number of CSV rows generated fromthe
search: expected rows were indeterminable, received 0.\"\r\n at
Context.<anonymous> (reporting.ts:182:33)\r\n at
processTicksAndRejections (node:internal/process/task_queues:95:5)\r\n
at Object.apply (wrap_function.js:73:16)\r\n```\r\n\r\nThis tracked back
to a feature added for reporting awhile back, which\r\ncreated a config
schema field for the `max_concurrent_shard_requests`\r\nparameter in the
search
queries:\r\nhttps://github.com//pull/170344/files#diff-7bceb37eef3761e1161cf04f41668dd9195bfac9fea36e734a230b5ed878a974\r\n\r\nMost
of the changes in this PR are in test code. I created \"Test\"
which\r\nextend protected methods in the original classes. This was done
to\r\nremove the `@ts-expect-errors` lines of
code.","sha":"f51c5c92bcc13686119ddf215eaa083ffac8e517","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Feature:Reporting","release_note:skip","backport:skip","Team:SharedUX","v8.15.0"],"number":182536,"url":"#182536
Resolve max_concurrent_shard_requests issue (#182536)\n\n##
Summary\r\n\r\nThere has been a consistent failure in a Discover-related
test set in\r\nthe kibana-ES-serverless verification job, meaning that
ES-Kibana\r\ncompatibility has drifted.\r\n\r\nError details:\r\n```\r\n
+ \"Encountered an unknown error: status_exception\r\n + \tRoot
causes:\r\n + \t\tstatus_exception: Parameter validation failed for
[/_search]: The http parameter [max_concurrent_shard_requests] (with
value [5]) is not permitted when running in serverless mode\"\r\n +
\"Encountered an error with the number of CSV rows generated fromthe
search: expected rows were indeterminable, received 0.\"\r\n at
Context.<anonymous> (reporting.ts:182:33)\r\n at
processTicksAndRejections (node:internal/process/task_queues:95:5)\r\n
at Object.apply (wrap_function.js:73:16)\r\n```\r\n\r\nThis tracked back
to a feature added for reporting awhile back, which\r\ncreated a config
schema field for the `max_concurrent_shard_requests`\r\nparameter in the
search
queries:\r\nhttps://github.com//pull/170344/files#diff-7bceb37eef3761e1161cf04f41668dd9195bfac9fea36e734a230b5ed878a974\r\n\r\nMost
of the changes in this PR are in test code. I created \"Test\"
which\r\nextend protected methods in the original classes. This was done
to\r\nremove the `@ts-expect-errors` lines of
code.","sha":"f51c5c92bcc13686119ddf215eaa083ffac8e517"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.15.0","labelRegex":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#182536
Resolve max_concurrent_shard_requests issue (#182536)\n\n##
Summary\r\n\r\nThere has been a consistent failure in a Discover-related
test set in\r\nthe kibana-ES-serverless verification job, meaning that
ES-Kibana\r\ncompatibility has drifted.\r\n\r\nError details:\r\n```\r\n
+ \"Encountered an unknown error: status_exception\r\n + \tRoot
causes:\r\n + \t\tstatus_exception: Parameter validation failed for
[/_search]: The http parameter [max_concurrent_shard_requests] (with
value [5]) is not permitted when running in serverless mode\"\r\n +
\"Encountered an error with the number of CSV rows generated fromthe
search: expected rows were indeterminable, received 0.\"\r\n at
Context.<anonymous> (reporting.ts:182:33)\r\n at
processTicksAndRejections (node:internal/process/task_queues:95:5)\r\n
at Object.apply (wrap_function.js:73:16)\r\n```\r\n\r\nThis tracked back
to a feature added for reporting awhile back, which\r\ncreated a config
schema field for the `max_concurrent_shard_requests`\r\nparameter in the
search
queries:\r\nhttps://github.com//pull/170344/files#diff-7bceb37eef3761e1161cf04f41668dd9195bfac9fea36e734a230b5ed878a974\r\n\r\nMost
of the changes in this PR are in test code. I created \"Test\"
which\r\nextend protected methods in the original classes. This was done
to\r\nremove the `@ts-expect-errors` lines of
code.","sha":"f51c5c92bcc13686119ddf215eaa083ffac8e517"}}]}] BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Reporting Reporting (PDF, CSV, ..) feature release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.14.0 v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants