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

changing defaults #164957

Merged
merged 4 commits into from
Sep 4, 2023
Merged

changing defaults #164957

merged 4 commits into from
Sep 4, 2023

Conversation

ppisljar
Copy link
Member

@ppisljar ppisljar commented Aug 28, 2023

Summary

resolves #157837

the defaults were updated:

  • wait_for_completion is increased to 200ms (from 100ms). this gives es more time to provide initial response before starting the pooling. With this change most responses in our sample dashboards are actually returned in this first response, which improves time to data from 1.2seconds to 0.3seconds in my local tests
  • poolling logic was updated so it does faster polling in the firs 2s after request was sent. Rather than waiting 1s before pools it waits only 300ms, which leads to actual pools sent out every 500ms (wait_for_completion time + poll interval)

to test i used shard delay aggregation to simulate slower responses
(add data.search.aggs.shardDelay.enabled: true to kibana.dev.yml` and use legacy visualizations and add a chart split with aggregation shard delay before adding anything else to the chart)

@ppisljar ppisljar added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Aug 28, 2023
@ppisljar ppisljar force-pushed the async/backoff branch 3 times, most recently from 83450ea to 65c1f99 Compare August 28, 2023 12:21
@ppisljar ppisljar marked this pull request as ready for review August 28, 2023 12:24
@ppisljar ppisljar requested review from a team as code owners August 28, 2023 12:24
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@ppisljar ppisljar added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.11.0 labels Aug 28, 2023
Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

The code looks fine 👍

I've tried to test both locally and in remote CCS and could not see any particular worsening of the performance. Perhaps this could be better measured in a deployed scenario.

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

These changes seem reasonable to me but I'm not super familiar with this code overall. WDYT @lukasolson?

@davismcphee davismcphee requested a review from a team August 29, 2023 23:09
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 408.3KB 408.3KB +23.0B

History

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

@ppisljar ppisljar merged commit 9d74315 into elastic:main Sep 4, 2023
28 checks passed
sphilipse pushed a commit to sphilipse/kibana that referenced this pull request Sep 4, 2023
bryce-b pushed a commit to bryce-b/kibana that referenced this pull request Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Search] Async search backing off strategy makes dashboards slow when searches take less than 1s to be served
6 participants