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

[Profiling] Auto-abort requests #142750

Merged
merged 5 commits into from
Oct 6, 2022
Merged

Conversation

dgieselaar
Copy link
Member

@dgieselaar dgieselaar commented Oct 5, 2022

Auto-abort requests when the user navigates away from the page or a new request is started.

@dgieselaar dgieselaar added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes v8.5.0 labels Oct 5, 2022
@dgieselaar dgieselaar requested a review from a team as a code owner October 5, 2022 13:38
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
profiling 235.4KB 235.9KB +455.0B

Page load bundle

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

id before after diff
profiling 12.5KB 12.6KB +145.0B

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

@rockdaboot
Copy link
Contributor

I'll test tomorrow morning locally.
Are you sure that the Merge branch main commit isn't accidentally backported to 8.5 ?

@dgieselaar
Copy link
Member Author

dgieselaar commented Oct 5, 2022

@rockdaboot yeah, that's no problem. The commit gets squashed and all that is left is the diff. That one gets backported. (Sorry other Tim R 🥲)

@timroes
Copy link
Contributor

timroes commented Oct 5, 2022

Nevertheless I find this a great enhancement. Let's not waste those resources for the query if the user already doesn't care about the response! Approve 🎉

Copy link
Contributor

@jbcrail jbcrail left a comment

Choose a reason for hiding this comment

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

Looks good and works as expected!

I tested these use cases:

  • load tested using 10 parallel runs and then canceled after a few seconds
  • switched back-and-forth between flamegraph and differential flamegraph multiple times for a large time range
  • switched back-and-forth between TopN functions and differential TopN functions multiple times for a large time range

For all cases, the server requests were canceled and the "Request aborted" was logged. I did not see the server errors about aborted requests as I had seen previously.

It was difficult to test the TopN stacktraces since the requests completed so quickly.

@dgieselaar
Copy link
Member Author

@timroes 😄 hope you're well!

@dgieselaar dgieselaar merged commit c05190e into elastic:main Oct 6, 2022
@dgieselaar dgieselaar deleted the auto-abort-requests branch October 6, 2022 07:17
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 6, 2022
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.5

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 6, 2022
(cherry picked from commit c05190e)

Co-authored-by: Dario Gieselaar <[email protected]>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
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 release_note:skip Skip the PR/issue when compiling release notes v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants