-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Monitoring] Stop a new request when one is inflight #27253
[Monitoring] Stop a new request when one is inflight #27253
Conversation
…e to stop a new request when one is inflight
Pinging @elastic/stack-monitoring |
Could you possibly split this so that the Logstash monitoring UI changes are in a separate PR? It looks like the meat of the core change here is quite small. |
💔 Build Failed |
@pickypg Done! |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
retest |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm most concerned about base_controller.js
's updates. And those LGTM, but I think could be improved a little.
this.updateData = () => { | ||
if (this.updatingData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could simplify this and remove this.updatingData
altogether by simply checking this.updateDataPromise
, then just set this.updateDataPromise = null;
, which we should probably do regardless, instead of this.updatingData = false;
.
@pickypg Updated! |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Convert all pages to use the base controlller, then add logic in there to stop a new request when one is inflight * Reuse the promise * Undo logstash changes * Update in catch too * Add unit test * Fix cluster name showing up * Update broken test * Just use updateDataPromise
* Convert all pages to use the base controlller, then add logic in there to stop a new request when one is inflight * Reuse the promise * Undo logstash changes * Update in catch too * Add unit test * Fix cluster name showing up * Update broken test * Just use updateDataPromise
Backport: 6.x: 6d2b61f |
Nice improvement for "Killbana" mitigation. ++ |
Resolves #24082
This PR adds code to prevent an additional API request if there is one in-flight. However, in order to make this work properly for all monitoring UI pages, we need to update the legacy ones to properly use the base controller. This PR includes that work as well.
Testing
NOTE: This is just how I tested it and feel free to find another way if this doesn't work for you.
3) In the URL address bar, change the interval to 1s (or 1000ms):
4) In the browser dev tools, turn on throttling: