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.

Sign up for GitHub

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

Jump to bottom

Debounce Homepage health overview refresh #515

Merged
merged 1 commit into from
May 11, 2022
Merged

Debounce Homepage health overview refresh #515

merged 1 commit into from
May 11, 2022

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented May 11, 2022 •
edited
Loading

e2e tests in our pipeline are failing on the dashboard.js test. Seems to be flaky.

A possible cause might be the fact that the dashboard gets refreshed whenever any of the following things happen

HOST_REGISTERED
CLUSTER_REGISTERED
DATABASE_REGISTERED
SAP_SYSTEM_REGISTERED
HEARTBEAT_FAILED
HEARTBEAT_SUCCEDED
DATABASE_HEALTH_CHANGED
SAP_SYSTEM_HEALTH_CHANGED
CLUSTER_HEALTH_CHANGED

In addition, when we run photofinish loading the healthy-27-node-SAP-cluster a lot of those thing happen, triggering something like ~40 refreshes, hence ~40 api calls that stack one after another.

By adding debouncing in that scenario we reduced the API calls to ~5.

Not sure it fixes, yet might be anyway helpful 😄

@nelsonkopliku nelsonkopliku force-pushed the debounce_health_overview_refresh branch from 9ccfca5 to a905d0f Compare May 11, 2022 08:00
stefanotorresi
stefanotorresi approved these changes May 11, 2022
View reviewed changes
Copy link
Member

@stefanotorresi stefanotorresi left a comment

Choose a reason for hiding this comment

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

LGTM.

dottorblaster
dottorblaster approved these changes May 11, 2022
View reviewed changes
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

Impressive, LGTM

@dottorblaster dottorblaster merged commit 79e5940 into main May 11, 2022
@dottorblaster dottorblaster deleted the debounce_health_overview_refresh branch May 11, 2022 08:14
@nelsonkopliku nelsonkopliku mentioned this pull request May 11, 2022
Merged
@nelsonkopliku nelsonkopliku added the enhancement New feature or request label May 11, 2022
@nelsonkopliku nelsonkopliku self-assigned this May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dottorblaster dottorblaster dottorblaster approved these changes

@stefanotorresi stefanotorresi stefanotorresi approved these changes

@fabriziosestito fabriziosestito Awaiting requested review from fabriziosestito

@arbulu89 arbulu89 Awaiting requested review from arbulu89

@rtorrero rtorrero Awaiting requested review from rtorrero

Assignees

@nelsonkopliku nelsonkopliku

Labels
enhancement New feature or request
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nelsonkopliku @dottorblaster @stefanotorresi