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

[Infrastructure UI] Refactor the use of reduces for the Metric Threshold Rule in 7.17 #126508

Closed
simianhacker opened this issue Feb 28, 2022 · 6 comments
Assignees
Labels
Feature:Metrics UI Metrics UI feature Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.2.0

Comments

@simianhacker
Copy link
Member

As a follow up to #121904 we need to refactor the execution code to remove the reduces in 7.17 since the original PR only targets the 8.0 branch. For this ticket, we are ONLY refactoring the three places referred in the original PR to eliminate the event loop blocking. We shouldn't add the new configuration settings or anything beyond the scope of removing the reduces.

@simianhacker simianhacker added Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.2.0 labels Feb 28, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@weltenwort
Copy link
Member

Are you sure it's the reduce that's causing it? Or is it the fact that we're creating a new array in each iteration? Would mutating the accumulator yield the same speed-ups?

@simianhacker
Copy link
Member Author

@weltenwort You're right, it's the spread... I just tested it between the for/of and reduce but modified the accumulator instead of using the spread and the performance gains are the same. I will update this PR to keep the reduces but eliminate the spread.

@simianhacker
Copy link
Member Author

Although... I kind of prefer the for/of code because it's more concises.

@weltenwort
Copy link
Member

Thanks for investigating further ❤️ reduce vs for is mostly a matter of taste and depends on whether a sequence of statement or a single expression is more appropriate. 🤷 Good to know we can choose either as long as we pay attention to the array creation.

@simianhacker
Copy link
Member Author

Closed with #126545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.2.0
Projects
None yet
Development

No branches or pull requests

3 participants