-
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] only start bulk uploader once #31307
Conversation
Pinging @elastic/stack-monitoring |
cc @tsullivan |
💔 Build Failed |
} else { | ||
this._fetchAndUpload(filterThem(collectorSet)); // initial fetch | ||
} | ||
|
||
this._timer = setInterval(() => { | ||
this._fetchAndUpload(filterThem(collectorSet)); | ||
}, this._interval); |
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'll likely want to wait for completion of _fetchAndUpload before restarting the interval. this may make many requests, and could result in a fair amount of back pressure
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.
Yes, and Cloud already suffered from this once: #30058
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.
👍 opened #31498. quick fix for patching and we can address that separately.
I'm going to test these changes later today, but code-wise, it seems to make sense, but @elastic/kibana-platform or @tsullivan would know more than me if this might not work for some edge case reason. |
retest |
💔 Build Failed |
💚 Build Succeeded |
@elastic/kibana-operations a little different :) anyone available for a review? |
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
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
hypothesis: cluster becomes unavailable, license check fails, bulk check started again?
kibana/x-pack/plugins/monitoring/init.js
Line 65 in 10e88ab