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

[Bugfix/Feature] Fixed slice render staggering on dashboard first load #3478

Merged

Conversation

Mogball
Copy link
Contributor

@Mogball Mogball commented Sep 17, 2017

  • Initial dashboard loading set a random delay (usually 0 to 5 seconds) on slice rendering
  • Triggering a dashboard refresh by changing filters or clicking "Refresh" did not do this
  • If a slice is indicated in timed_refresh_immune_slices it will not load upon initial rendering

Refreshing the dashboard now will either stagger or not stagger slice rendering, regardless of how it is triggered, and there is the option to disable this and set the stagger interval (in milliseconds) in the dashboard's JSON Metadata: { ... "stagger_refresh": false, "stagger_time": 15000 }

Default value is true and 5 seconds. Staggering is also more deterministic.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.142% when pulling 1c9bbefacc6b9245ba6bec6bab6d20d8ccd71f07 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.142% when pulling 1c9bbefacc6b9245ba6bec6bab6d20d8ccd71f07 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.142% when pulling 1c9bbefacc6b9245ba6bec6bab6d20d8ccd71f07 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.142% when pulling 1c9bbefacc6b9245ba6bec6bab6d20d8ccd71f07 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage remained the same at 69.142% when pulling 0d9a97e3ddbccdf1ab07ae406e0dd98d05ced96f on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.142% when pulling 0f84378cd4f88de2c75d0fa2ccf37a0cb7ca7786 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.142% when pulling 0f84378cd4f88de2c75d0fa2ccf37a0cb7ca7786 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage remained the same at 69.142% when pulling 0f84378cd4f88de2c75d0fa2ccf37a0cb7ca7786 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@Mogball Mogball force-pushed the mogball/feature/dashboard_loading branch from 0f84378 to c622f95 Compare September 17, 2017 23:42
@coveralls
Copy link

coveralls commented Sep 17, 2017

Coverage Status

Coverage remained the same at 69.142% when pulling c622f9580abbbb85cd4feda85f1f733a44a34a60 on Mogball:mogball/feature/dashboard_loading into e22aecb on apache:master.

@mistercrunch
Copy link
Member

These 2 new configuration elements (stagger_refresh, stagger_refresh) need to be documented. Currently the best place for that is faq.rst

@Mogball Mogball force-pushed the mogball/feature/dashboard_loading branch from 9d9a9d1 to 963b9cb Compare September 18, 2017 17:33
@Mogball
Copy link
Contributor Author

Mogball commented Sep 18, 2017

@mistercrunch Added to docs

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 69.442% when pulling 963b9cb on Mogball:mogball/feature/dashboard_loading into 6fe93e1 on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 69.442% when pulling 963b9cb on Mogball:mogball/feature/dashboard_loading into 6fe93e1 on apache:master.

@coveralls
Copy link

coveralls commented Sep 18, 2017

Coverage Status

Coverage increased (+0.3%) to 69.442% when pulling 963b9cb on Mogball:mogball/feature/dashboard_loading into 6fe93e1 on apache:master.

@mistercrunch
Copy link
Member

Wait I didn't realize that there was a 5 * random sec delay by default on first load. I thought [community contributed] startPeriodicRender only applied when auto-refresh mode was on.

We need to remove that delay on the first load, the idea with the delay was to randomize the subsequent refreshes so that if you set it to refresh every 5 minutes, it won't go all reload at once, it's nicer if the slices go randomly over that period.

Do you want take this on as part of this PR since you're already deep into it?

@Mogball
Copy link
Contributor Author

Mogball commented Sep 20, 2017

Alrighty. I'll make changes so that

  • Dashboard instantly refreshes: on initial load, when changing dashboard filters, when force refreshing the dashboard
  • stagger_refresh and stagger_time will only affect the periodic refreshing

@Mogball Mogball force-pushed the mogball/feature/dashboard_loading branch from 5b225d9 to 1b71821 Compare September 20, 2017 23:59
@Mogball Mogball force-pushed the mogball/feature/dashboard_loading branch from 1b71821 to 268804e Compare September 21, 2017 00:01
@coveralls
Copy link

coveralls commented Sep 21, 2017

Coverage Status

Coverage remained the same at 69.535% when pulling 268804e on Mogball:mogball/feature/dashboard_loading into 9af34ba on apache:master.

@coveralls
Copy link

coveralls commented Sep 21, 2017

Coverage Status

Coverage remained the same at 69.535% when pulling 268804e on Mogball:mogball/feature/dashboard_loading into 9af34ba on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling 268804e on Mogball:mogball/feature/dashboard_loading into 9af34ba on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.535% when pulling 268804e on Mogball:mogball/feature/dashboard_loading into 9af34ba on apache:master.

@coveralls
Copy link

coveralls commented Sep 21, 2017

Coverage Status

Coverage remained the same at 69.535% when pulling 268804e on Mogball:mogball/feature/dashboard_loading into 9af34ba on apache:master.

@Mogball
Copy link
Contributor Author

Mogball commented Sep 21, 2017

@mistercrunch Done

@Mogball Mogball changed the title Feature: option to disable dashboard refresh staggering [Bugfix/Feature] Fixed slice render staggering on dashboard first load Oct 4, 2017
@mistercrunch mistercrunch merged commit e95132d into apache:master Oct 4, 2017
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
apache#3478)

* Feature: disable dashboard refresh staggering

* Removed refresh staggering everywhere except during periodic render
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.2 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants